threads / patch / 40802

v2, 2 partscompletion: add support for completing email aliases

Subject: [PATCH v2 2/2] completion: add support for completing email aliases

## tl;dr

14 messages between Nov 15, 2015 and Nov 17, 2015. Diffs are folded; open one to read it.

replies: 13people: 4as markdown or json

Jacob Keller· Nov 15, 2015, 20:22 UTC · lore

[PATCH v2 1/2] sendemail: teach git-send-email to list aliases

From: Jacob Keller <jacob.keller@gmail.com>

Add an option "list-aliases" which changes the default behavior of git-send-email. This mode will simply read the alias files configured by sendemail.aliasesfile and sendemail.aliasfiletype and print a list of all known aliases. The intended usecase for this option is the bash-completion script which will use it to autocomplete aliases on the options which take addresses.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
 Documentation/git-send-email.txt | 10 ++++++++++
 git-send-email.perl              | 11 +++++++++++
 2 files changed, 21 insertions(+)
Show changes to 2 files +21 −0

Documentation/git-send-email.txt, git-send-email.perl

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index b9134d234f53..481770d72e13 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -10,6 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git send-email' [options] <file|directory|rev-list options>...
+'git send-email' --list-aliases
 
 
 DESCRIPTION
@@ -387,6 +388,15 @@ default to '--validate'.
 	Send emails even if safety checks would prevent it.
 
 
+Information
+~~~~~~~~~~~
+
+--list-aliases::
+	Instead of the standard operation, list all aliases found in the
+	configured alias file(s), and then exit. See 'sendemail.aliasesfile'
+	for more information about aliases.
+
+
 CONFIGURATION
 -------------
 
diff --git a/git-send-email.perl b/git-send-email.perl
index e907e0eacf31..ee14894b172b 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -46,6 +46,7 @@ package main;
 sub usage {
 	print <<EOT;
 git send-email [options] <file | directory | rev-list options >
+git send-email --list-aliases
 
   Composing:
     --from                  <str>  * Email From:
@@ -101,6 +102,9 @@ git send-email [options] <file | directory | rev-list options >
                                      `git format-patch` ones.
     --force                        * Send even if safety checks would prevent it.
 
+  Information:
+    --list-aliases                 * read the aliases from configured alias files
+
 EOT
 	exit(1);
 }
@@ -180,6 +184,7 @@ my ($quiet, $dry_run) = (0, 0);
 my $format_patch;
 my $compose_filename;
 my $force = 0;
+my $list_aliases = 0;
 
 # Handle interactive edition of files.
 my $multiedit;
@@ -344,6 +349,7 @@ my $rc = GetOptions("h" => \$help,
 		    "force" => \$force,
 		    "xmailer!" => \$use_xmailer,
 		    "no-xmailer" => sub {$use_xmailer = 0},
+                    "list-aliases" => \$list_aliases,
 	 );
 
 usage() if $help;
@@ -551,6 +557,11 @@ if (@alias_files and $aliasfiletype and defined $parse_alias{$aliasfiletype}) {
 	}
 }
 
+if ($list_aliases) {
+    print $_,"\n" for (keys %aliases);
+    exit(0);
+}
+
 # is_format_patch_arg($f) returns 0 if $f names a patch, or 1 if
 # $f is a revision list specification to be passed to format-patch.
 sub is_format_patch_arg {

-- 
2.6.3.491.g3e3f6ce
Jacob Keller· Nov 15, 2015, 20:22 UTC · re: Jacob Keller · lore
From: Jacob Keller <jacob.keller@gmail.com>

Using the new --list-aliases option from git-send-email, add completion for --to, --cc, and --bcc with the available configured aliases.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
Notes:
    - v2
    * reuse parsing from git-send-email via --list-aliases
 contrib/completion/git-completion.bash | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
Show changes to contrib/completion/git-completion.bash +16 −0
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 482ca84b451b..fde246e2943e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -10,6 +10,7 @@
 #    *) local and remote tag names
 #    *) .git/remotes file names
 #    *) git 'subcommands'
+#    *) git email aliases for git-send-email
 #    *) tree paths within 'ref:path/to/file' expressions
 #    *) file paths within current working directory and index
 #    *) common --long-options
@@ -1711,6 +1712,15 @@ __git_send_email_suppresscc_options="author self cc bodycc sob cccmd body all"
 
 _git_send_email ()
 {
+	case "$prev" in
+	--to|--cc|--bcc)
+		__gitcomp "
+		$(git --git-dir="$(__gitdir)" send-email --list-aliases 2>/dev/null)
+		" "" ""
+		return
+		;;
+	esac
+
 	case "$cur" in
 	--confirm=*)
 		__gitcomp "
@@ -1735,6 +1745,12 @@ _git_send_email ()
 			" "" "${cur##--thread=}"
 		return
 		;;
+	--to=*|--cc=*|--bcc=*)
+		__gitcomp "
+		$(git --git-dir="$(__gitdir)" send-email --list-aliases 2>/dev/null)
+		" "" "${cur#--*=}"
+		return
+		;;
 	--*)
 		__gitcomp "--annotate --bcc --cc --cc-cmd --chain-reply-to
 			--compose --confirm= --dry-run --envelope-sender

-- 
2.6.3.491.g3e3f6ce
Eric Sunshine· Nov 16, 2015, 23:33 UTC · re: Jacob Keller · lore

Re: [PATCH v2 2/2] completion: add support for completing email aliases

On Sun, Nov 15, 2015 at 3:22 PM, Jacob Keller <jacob.e.keller@intel.com> wrote:
Show 12 quoted lines
> Using the new --list-aliases option from git-send-email, add completion
> for --to, --cc, and --bcc with the available configured aliases.
>
> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> ---
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> @@ -1711,6 +1712,15 @@ __git_send_email_suppresscc_options="author self cc bodycc sob cccmd body all"
>
>  _git_send_email ()
>  {
> +       case "$prev" in
> +       --to|--cc|--bcc)
What about --from, which also undergoes alias expansion?
Show 25 quoted lines
> +               __gitcomp "
> +               $(git --git-dir="$(__gitdir)" send-email --list-aliases 2>/dev/null)
> +               " "" ""
> +               return
> +               ;;
> +       esac
> +
>         case "$cur" in
>         --confirm=*)
>                 __gitcomp "
> @@ -1735,6 +1745,12 @@ _git_send_email ()
>                         " "" "${cur##--thread=}"
>                 return
>                 ;;
> +       --to=*|--cc=*|--bcc=*)
> +               __gitcomp "
> +               $(git --git-dir="$(__gitdir)" send-email --list-aliases 2>/dev/null)
> +               " "" "${cur#--*=}"
> +               return
> +               ;;
>         --*)
>                 __gitcomp "--annotate --bcc --cc --cc-cmd --chain-reply-to
>                         --compose --confirm= --dry-run --envelope-sender
> --
> 2.6.3.491.g3e3f6ce
Keller, Jacob E· Nov 16, 2015, 23:40 UTC · re: Eric Sunshine · lore

Re: [PATCH v2 2/2] completion: add support for completing email aliases

On Mon, 2015-11-16 at 18:33 -0500, Eric Sunshine wrote:
Show 20 quoted lines
> On Sun, Nov 15, 2015 at 3:22 PM, Jacob Keller <jacob.e.keller@intel.c
> om> wrote:
> > Using the new --list-aliases option from git-send-email, add
> > completion
> > for --to, --cc, and --bcc with the available configured aliases.
> > 
> > Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> > ---
> > diff --git a/contrib/completion/git-completion.bash
> > b/contrib/completion/git-completion.bash
> > @@ -1711,6 +1712,15 @@ __git_send_email_suppresscc_options="author
> > self cc bodycc sob cccmd body all"
> > 
> >  _git_send_email ()
> >  {
> > +       case "$prev" in
> > +       --to|--cc|--bcc)
> 
> What about --from, which also undergoes alias expansion?
> 
Makes sense, yep.
Eric Sunshine· Nov 16, 2015, 23:30 UTC · re: Jacob Keller · lore

Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases

On Sun, Nov 15, 2015 at 3:22 PM, Jacob Keller <jacob.e.keller@intel.com> wrote:
Show 6 quoted lines
> Add an option "list-aliases" which changes the default behavior of
> git-send-email. This mode will simply read the alias files configured by
> sendemail.aliasesfile and sendemail.aliasfiletype and print a list of
> all known aliases. The intended usecase for this option is the
> bash-completion script which will use it to autocomplete aliases on the
> options which take addresses.

As this is primarily a plumbing option, I wonder if --dump-aliases might be a more suitable name.

Also, is it possible that some consumer down the road might want richer output which includes the expansion of each alias? For instance, it could emit the alias name as the first token on each line and the expansion as the remainder. Consumers interested in only the alias name would grab the first token on the line and ignore everything else.

Show 9 quoted lines
> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> ---
> diff --git a/git-send-email.perl b/git-send-email.perl
> @@ -101,6 +102,9 @@ git send-email [options] <file | directory | rev-list options >
>                                       `git format-patch` ones.
>      --force                        * Send even if safety checks would prevent it.
>
> +  Information:
> +    --list-aliases                 * read the aliases from configured alias files

This description is odd. It seems to imply that aliases will be loaded (and used) only if this option is given, and says nothing about its actual purpose of dumping the aliases.

Also, with one exception, all the other option descriptions are capitalized. This probably ought to follow suit.

> +if ($list_aliases) {
> +    print $_,"\n" for (keys %aliases);
> +    exit(0);
> +}
New test(s) seem to be missing.
Keller, Jacob E· Nov 16, 2015, 23:40 UTC · re: Eric Sunshine · lore

Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases

On Mon, 2015-11-16 at 18:30 -0500, Eric Sunshine wrote:
Show 15 quoted lines
> On Sun, Nov 15, 2015 at 3:22 PM, Jacob Keller <jacob.e.keller@intel.c
> om> wrote:
> > Add an option "list-aliases" which changes the default behavior of
> > git-send-email. This mode will simply read the alias files
> > configured by
> > sendemail.aliasesfile and sendemail.aliasfiletype and print a list
> > of
> > all known aliases. The intended usecase for this option is the
> > bash-completion script which will use it to autocomplete aliases on
> > the
> > options which take addresses.
> 
> As this is primarily a plumbing option, I wonder if --dump-aliases
> might be a more suitable name.
> 
Sure that would be reasonable.
Show 8 quoted lines
> Also, is it possible that some consumer down the road might want
> richer output which includes the expansion of each alias? For
> instance, it could emit the alias name as the first token on each
> line
> and the expansion as the remainder. Consumers interested in only the
> alias name would grab the first token on the line and ignore
> everything else.
>

Maybe? The problem with printing the full address is that it may not be quoted or similar, and it makes the bash completion require an extra parameter.. I am not sure how valuable the alias expansion would be for use? The main concern I have is we'd need to use another process on top to extract only alias names.

Show 18 quoted lines
> > Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> > ---
> > diff --git a/git-send-email.perl b/git-send-email.perl
> > @@ -101,6 +102,9 @@ git send-email [options] <file | directory |
> > rev-list options >
> >                                       `git format-patch` ones.
> >      --force                        * Send even if safety checks
> > would prevent it.
> > 
> > +  Information:
> > +    --list-aliases                 * read the aliases from
> > configured alias files
> 
> This description is odd. It seems to imply that aliases will be
> loaded
> (and used) only if this option is given, and says nothing about its
> actual purpose of dumping the aliases.
> 
I can re-word this.
Show 10 quoted lines
> Also, with one exception, all the other option descriptions are
> capitalized. This probably ought to follow suit.
> 
> > +if ($list_aliases) {
> > +    print $_,"\n" for (keys %aliases);
> > +    exit(0);
> > +}
> 
> New test(s) seem to be missing.
>

I had removed the tests from the old version because they weren't necessary anymore. New ones wouldn't hurt here either, though.. I'll work on that.

Regards, Jake

> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Sunshine· Nov 16, 2015, 23:50 UTC · re: Keller, Jacob E · lore

Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases

On Mon, Nov 16, 2015 at 6:40 PM, Keller, Jacob E <jacob.e.keller@intel.com> wrote:

Show 14 quoted lines
> On Mon, 2015-11-16 at 18:30 -0500, Eric Sunshine wrote:
>> Also, is it possible that some consumer down the road might want
>> richer output which includes the expansion of each alias? For
>> instance, it could emit the alias name as the first token on each
>> line
>> and the expansion as the remainder. Consumers interested in only the
>> alias name would grab the first token on the line and ignore
>> everything else.
>
> Maybe? The problem with printing the full address is that it may not be
> quoted or similar, and it makes the bash completion require an extra
> parameter.. I am not sure how valuable the alias expansion would be for
> use? The main concern I have is we'd need to use another process on top
> to extract only alias names.

It should be possible to extract the alias within the shell itself without a separate process. For instance:

    read alias rest

will leave the first token in $alias and the remainder of the line in $rest, and it's all done within the shell process.

Show 5 quoted lines
>> New test(s) seem to be missing.
>
> I had removed the tests from the old version because they weren't
> necessary anymore. New ones wouldn't hurt here either, though.. I'll
> work on that.

I'm not sure which tests you mean, but I was referring to tests to make sure that git-send-email recognizes --list-aliases (or --dump-aliases if you switch to that) and that it produces the expected output in the expected format.

Also, shouldn't --list-aliases (or --dump-aliases) be mutually exclusive with many of the other options? New tests would check such exclusivity as well.

Eric Sunshine· Nov 16, 2015, 23:56 UTC · re: Eric Sunshine · lore

Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases

On Mon, Nov 16, 2015 at 6:50 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> Also, shouldn't --list-aliases (or --dump-aliases) be mutually
> exclusive with many of the other options? New tests would check such
> exclusivity as well.

In fact, while I agree with Szeder that it makes sense to re-use send-email's aliases parsing functionality (and was going to suggest the same, but he beat me to it), this new option is awfully orthogonal to the overall purpose of send-email, thus, doesn't really fit in well and almost cries out for a command of its own which would be used by send-email and bash completion (though I'm not convinced that it's worth going that route for this one minor use-case).

Keller, Jacob E· Nov 17, 2015, 00:09 UTC · re: Eric Sunshine · lore

Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases

On Mon, 2015-11-16 at 18:56 -0500, Eric Sunshine wrote:
Show 16 quoted lines
> On Mon, Nov 16, 2015 at 6:50 PM, Eric Sunshine <sunshine@sunshineco.c
> om> wrote:
> > Also, shouldn't --list-aliases (or --dump-aliases) be mutually
> > exclusive with many of the other options? New tests would check
> > such
> > exclusivity as well.
> 
> In fact, while I agree with Szeder that it makes sense to re-use
> send-email's aliases parsing functionality (and was going to suggest
> the same, but he beat me to it), this new option is awfully
> orthogonal
> to the overall purpose of send-email, thus, doesn't really fit in
> well
> and almost cries out for a command of its own which would be used by
> send-email and bash completion (though I'm not convinced that it's
> worth going that route for this one minor use-case).

I don't think it's worth it at this point, because we'd have to extract out the alias parsing logic from send-email, which is not easy.

The option is pretty orthogonal to git-send-email, but until/unless git-send-email is re-implemented in C, i don't really see value in trying to separate the logic out... That is a lot more effort for very little gain.

Regards, Jake

Keller, Jacob E· Nov 17, 2015, 00:10 UTC · re: Eric Sunshine · lore

Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases

On Mon, 2015-11-16 at 18:50 -0500, Eric Sunshine wrote:
Show 8 quoted lines
> It should be possible to extract the alias within the shell itself
> without a separate process. For instance:
> 
>     read alias rest
> 
> will leave the first token in $alias and the remainder of the line in
> $rest, and it's all done within the shell process.
> 
I'll look into this :)
Show 12 quoted lines
> > > New test(s) seem to be missing.
> > 
> > I had removed the tests from the old version because they weren't
> > necessary anymore. New ones wouldn't hurt here either, though..
> > I'll
> > work on that.
> 
> I'm not sure which tests you mean, but I was referring to tests to
> make sure that git-send-email recognizes --list-aliases (or
> --dump-aliases if you switch to that) and that it produces the
> expected output in the expected format.
> 
Yep, I added some in my respin.
> Also, shouldn't --list-aliases (or --dump-aliases) be mutually
> exclusive with many of the other options? New tests would check such
> exclusivity as well.

I am at a loss for how to do that correctly in the perl. Help would be appreciated here.

Regards, Jak

Eric Sunshine· Nov 17, 2015, 07:20 UTC · re: Keller, Jacob E · lore

Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases

On Tue, Nov 17, 2015 at 12:10:35AM +0000, Keller, Jacob E wrote:
Show 10 quoted lines
> On Mon, 2015-11-16 at 18:50 -0500, Eric Sunshine wrote:
> > It should be possible to extract the alias within the shell itself
> > without a separate process. For instance:
> > 
> >     read alias rest
> > 
> > will leave the first token in $alias and the remainder of the line in
> > $rest, and it's all done within the shell process.
> 
> I'll look into this :)

My reason for asking is concern about scripts possibly breaking if someone comes along and wants to "fix" --dump-aliases to also dump the alias expansions. One possibility is just to punt today and say that when that feature is needed in the future, then that someone can add a --verbose option to complement --dump-aliases which would emit the alias expansions as well. One nice thing about punting at this point is that we don't (today) have to define a format for the output of the expansions. If we did want to think about it, one verbose, non-ambiguous format would be to show the alias name on a line by itself, and each expansion value on a line by itself indented by a tab. For instance:

    managers
        bob
	fred
    devs
        jane
	john
Show 6 quoted lines
> > Also, shouldn't --list-aliases (or --dump-aliases) be mutually
> > exclusive with many of the other options? New tests would check such
> > exclusivity as well.
> 
> I am at a loss for how to do that correctly in the perl. Help would be
> appreciated here.

Since git-send-email.perl already configures GetOpt::Long with the 'pass_through' option, one possibility would be to invoke GetOptions() once for --list-aliases (or --dump-aliases), and then again for the normal options. Doing so may be a bit ugly; on the other hand, it does indicate pretty clearly that --list-aliases is a distinct "mode" of operation. On top of your patch, it might look something like this:

--- 8< ---
Show changes to git-send-email.perl +6 −1
diff --git a/git-send-email.perl b/git-send-email.perl
index ee14894..cada5ea 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -296,6 +296,12 @@ $SIG{INT}  = \&signal_handler;
 
 my $help;
 my $rc = GetOptions("h" => \$help,
+		    "list-aliases" => \$list_aliases);
+usage() unless $rc;
+die "--list-aliases incompatible with other options\n"
+	if !$help and $list_aliases and @ARGV;
+
+$rc = GetOptions(
 		    "sender|from=s" => \$sender,
                     "in-reply-to=s" => \$initial_reply_to,
 		    "subject=s" => \$initial_subject,
@@ -349,7 +355,6 @@ my $rc = GetOptions("h" => \$help,
 		    "force" => \$force,
 		    "xmailer!" => \$use_xmailer,
 		    "no-xmailer" => sub {$use_xmailer = 0},
-                    "list-aliases" => \$list_aliases,
 	 );
 
 usage() if $help;
--- 8< ---

Though, it may be overkill for this minor use-case.
Jacob Keller· Nov 17, 2015, 08:25 UTC · re: Eric Sunshine · lore

Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases

On Mon, Nov 16, 2015 at 11:20 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 37 quoted lines
> Since git-send-email.perl already configures GetOpt::Long with the
> 'pass_through' option, one possibility would be to invoke
> GetOptions() once for --list-aliases (or --dump-aliases), and then
> again for the normal options. Doing so may be a bit ugly; on the
> other hand, it does indicate pretty clearly that --list-aliases is a
> distinct "mode" of operation. On top of your patch, it might look
> something like this:
>
> --- 8< ---
> diff --git a/git-send-email.perl b/git-send-email.perl
> index ee14894..cada5ea 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -296,6 +296,12 @@ $SIG{INT}  = \&signal_handler;
>
>  my $help;
>  my $rc = GetOptions("h" => \$help,
> +                   "list-aliases" => \$list_aliases);
> +usage() unless $rc;
> +die "--list-aliases incompatible with other options\n"
> +       if !$help and $list_aliases and @ARGV;
> +
> +$rc = GetOptions(
>                     "sender|from=s" => \$sender,
>                      "in-reply-to=s" => \$initial_reply_to,
>                     "subject=s" => \$initial_subject,
> @@ -349,7 +355,6 @@ my $rc = GetOptions("h" => \$help,
>                     "force" => \$force,
>                     "xmailer!" => \$use_xmailer,
>                     "no-xmailer" => sub {$use_xmailer = 0},
> -                    "list-aliases" => \$list_aliases,
>          );
>
>  usage() if $help;
> --- 8< ---
>
> Though, it may be overkill for this minor use-case.
I actually really like this approach. I will squash it in.

Regards, Jake

SZEDER Gábor· Nov 17, 2015, 12:26 UTC · re: Eric Sunshine · lore

Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases

Quoting Eric Sunshine <sunshine@sunshineco.com>:
Show 9 quoted lines
> On Tue, Nov 17, 2015 at 12:10:35AM +0000, Keller, Jacob E wrote:
>> On Mon, 2015-11-16 at 18:50 -0500, Eric Sunshine wrote:
>> > It should be possible to extract the alias within the shell itself
>> > without a separate process. For instance:
>> >
>> >     read alias rest
>> >
>> > will leave the first token in $alias and the remainder of the line in
>> > $rest, and it's all done within the shell process.

Actually, putting this read in a while loop and feeding 'git send-email's output into that does fork() a subshell:

   $ echo "outside: $BASH_SUBSHELL" |while read line ; do echo "$line   
inside: $BASH_SUBSHELL" ; done
   outside: 0  inside: 1
Show 10 quoted lines
>> I'll look into this :)
>
> My reason for asking is concern about scripts possibly breaking if
> someone comes along and wants to "fix" --dump-aliases to also dump
> the alias expansions. One possibility is just to punt today and say
> that when that feature is needed in the future, then that someone can
> add a --verbose option to complement --dump-aliases which would emit
> the alias expansions as well. One nice thing about punting at this
> point is that we don't (today) have to define a format for the output
> of the expansions.
I think we should cross the bridge when we get to it.

However, we could still be nice to that brave soul who might want to cross it in the future, and since at this point we are interested in listing only alias names, perhaps we should not appropriate the broader '--dump-alias' option, but go with the more specific '--dump-alias-names' instead.

Gábor
Jacob Keller· Nov 17, 2015, 16:27 UTC · re: SZEDER Gábor · lore

Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases

On Tue, Nov 17, 2015 at 4:26 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:
Show 18 quoted lines
>
> Quoting Eric Sunshine <sunshine@sunshineco.com>:
>
>> On Tue, Nov 17, 2015 at 12:10:35AM +0000, Keller, Jacob E wrote:
>>>
>>> On Mon, 2015-11-16 at 18:50 -0500, Eric Sunshine wrote:
>>> > It should be possible to extract the alias within the shell itself
>>> > without a separate process. For instance:
>>> >
>>> >     read alias rest
>>> >
>>> > will leave the first token in $alias and the remainder of the line in
>>> > $rest, and it's all done within the shell process.
>
>
> Actually, putting this read in a while loop and feeding 'git send-email's
> output into that does fork() a subshell:
>

Yea, I realized this. One extra subshell isn't a big deal but if we can eliminate that it is good.

Show 18 quoted lines
>   $ echo "outside: $BASH_SUBSHELL" |while read line ; do echo "$line
> inside: $BASH_SUBSHELL" ; done
>   outside: 0  inside: 1
>
>>> I'll look into this :)
>>
>>
>> My reason for asking is concern about scripts possibly breaking if
>> someone comes along and wants to "fix" --dump-aliases to also dump
>> the alias expansions. One possibility is just to punt today and say
>> that when that feature is needed in the future, then that someone can
>> add a --verbose option to complement --dump-aliases which would emit
>> the alias expansions as well. One nice thing about punting at this
>> point is that we don't (today) have to define a format for the output
>> of the expansions.
>
>
> I think we should cross the bridge when we get to it.
Agreed.
Show 6 quoted lines
>
> However, we could still be nice to that brave soul who might want to cross
> it in the future, and since at this point we are interested in listing only
> alias names, perhaps we should not appropriate the broader '--dump-alias'
> option, but go with the more specific '--dump-alias-names' instead.
>

Disagree. I think the best approach for expanding --dump-aliases output is turning it into a keyword-option argument such as:

--dump-aliases=format

and we can say "historic reasons the format is always names only", then we can extend the support that way. I don't really think that "--dump-alias-names" is nice since then we'd need separate option if this ever does change, and it's rather long if it never changes.

I suspect it won't be something people need either.

Regards, Jake

>
> Gábor
>

← back to recent threads