All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] sendemail: teach git-send-email to list aliases
@ 2015-11-15 20:22 Jacob Keller
  2015-11-15 20:22 ` [PATCH v2 2/2] completion: add support for completing email aliases Jacob Keller
  2015-11-16 23:30 ` [PATCH v2 1/2] sendemail: teach git-send-email to list aliases Eric Sunshine
  0 siblings, 2 replies; 14+ messages in thread
From: Jacob Keller @ 2015-11-15 20:22 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor, Shawn O . Pearce,
	Felipe Contreras, Lee Marlow, Jacob Keller

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(+)

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/2] completion: add support for completing email aliases
  2015-11-15 20:22 [PATCH v2 1/2] sendemail: teach git-send-email to list aliases Jacob Keller
@ 2015-11-15 20:22 ` Jacob Keller
  2015-11-16 23:33   ` Eric Sunshine
  2015-11-16 23:30 ` [PATCH v2 1/2] sendemail: teach git-send-email to list aliases Eric Sunshine
  1 sibling, 1 reply; 14+ messages in thread
From: Jacob Keller @ 2015-11-15 20:22 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor, Shawn O . Pearce,
	Felipe Contreras, Lee Marlow, Jacob Keller

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(+)

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases
  2015-11-15 20:22 [PATCH v2 1/2] sendemail: teach git-send-email to list aliases Jacob Keller
  2015-11-15 20:22 ` [PATCH v2 2/2] completion: add support for completing email aliases Jacob Keller
@ 2015-11-16 23:30 ` Eric Sunshine
  2015-11-16 23:40   ` Keller, Jacob E
  1 sibling, 1 reply; 14+ messages in thread
From: Eric Sunshine @ 2015-11-16 23:30 UTC (permalink / raw)
  To: Jacob Keller
  Cc: Git List, Junio C Hamano, SZEDER Gábor, Shawn O . Pearce,
	Felipe Contreras, Lee Marlow, Jacob Keller

On Sun, Nov 15, 2015 at 3:22 PM, Jacob Keller <jacob.e.keller@intel.com> 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.

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.

> 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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/2] completion: add support for completing email aliases
  2015-11-15 20:22 ` [PATCH v2 2/2] completion: add support for completing email aliases Jacob Keller
@ 2015-11-16 23:33   ` Eric Sunshine
  2015-11-16 23:40     ` Keller, Jacob E
  0 siblings, 1 reply; 14+ messages in thread
From: Eric Sunshine @ 2015-11-16 23:33 UTC (permalink / raw)
  To: Jacob Keller
  Cc: Git List, Junio C Hamano, SZEDER Gábor, Shawn O . Pearce,
	Felipe Contreras, Lee Marlow, Jacob Keller

On Sun, Nov 15, 2015 at 3:22 PM, Jacob Keller <jacob.e.keller@intel.com> 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?

> +               __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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases
  2015-11-16 23:30 ` [PATCH v2 1/2] sendemail: teach git-send-email to list aliases Eric Sunshine
@ 2015-11-16 23:40   ` Keller, Jacob E
  2015-11-16 23:50     ` Eric Sunshine
  0 siblings, 1 reply; 14+ messages in thread
From: Keller, Jacob E @ 2015-11-16 23:40 UTC (permalink / raw)
  To: sunshine
  Cc: git, gitster, spearce, felipe.contreras, szeder, jacob.keller,
	lee.marlow

On Mon, 2015-11-16 at 18:30 -0500, Eric Sunshine wrote:
> 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.

> 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.

> > 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.

> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/2] completion: add support for completing email aliases
  2015-11-16 23:33   ` Eric Sunshine
@ 2015-11-16 23:40     ` Keller, Jacob E
  0 siblings, 0 replies; 14+ messages in thread
From: Keller, Jacob E @ 2015-11-16 23:40 UTC (permalink / raw)
  To: sunshine
  Cc: git, gitster, spearce, felipe.contreras, szeder, jacob.keller,
	lee.marlow

On Mon, 2015-11-16 at 18:33 -0500, Eric Sunshine wrote:
> 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.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases
  2015-11-16 23:40   ` Keller, Jacob E
@ 2015-11-16 23:50     ` Eric Sunshine
  2015-11-16 23:56       ` Eric Sunshine
  2015-11-17  0:10       ` Keller, Jacob E
  0 siblings, 2 replies; 14+ messages in thread
From: Eric Sunshine @ 2015-11-16 23:50 UTC (permalink / raw)
  To: Keller, Jacob E
  Cc: git, gitster, spearce, felipe.contreras, szeder, jacob.keller,
	lee.marlow

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

>> 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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases
  2015-11-16 23:50     ` Eric Sunshine
@ 2015-11-16 23:56       ` Eric Sunshine
  2015-11-17  0:09         ` Keller, Jacob E
  2015-11-17  0:10       ` Keller, Jacob E
  1 sibling, 1 reply; 14+ messages in thread
From: Eric Sunshine @ 2015-11-16 23:56 UTC (permalink / raw)
  To: Keller, Jacob E
  Cc: git, gitster, spearce, felipe.contreras, szeder, jacob.keller,
	lee.marlow

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).

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases
  2015-11-16 23:56       ` Eric Sunshine
@ 2015-11-17  0:09         ` Keller, Jacob E
  0 siblings, 0 replies; 14+ messages in thread
From: Keller, Jacob E @ 2015-11-17  0:09 UTC (permalink / raw)
  To: sunshine
  Cc: git, gitster, spearce, lee.marlow, felipe.contreras, szeder,
	jacob.keller

On Mon, 2015-11-16 at 18:56 -0500, Eric Sunshine wrote:
> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases
  2015-11-16 23:50     ` Eric Sunshine
  2015-11-16 23:56       ` Eric Sunshine
@ 2015-11-17  0:10       ` Keller, Jacob E
  2015-11-17  7:20         ` Eric Sunshine
  1 sibling, 1 reply; 14+ messages in thread
From: Keller, Jacob E @ 2015-11-17  0:10 UTC (permalink / raw)
  To: sunshine
  Cc: git, gitster, spearce, lee.marlow, felipe.contreras, szeder,
	jacob.keller

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 :)

> > > 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases
  2015-11-17  0:10       ` Keller, Jacob E
@ 2015-11-17  7:20         ` Eric Sunshine
  2015-11-17  8:25           ` Jacob Keller
  2015-11-17 12:26           ` SZEDER Gábor
  0 siblings, 2 replies; 14+ messages in thread
From: Eric Sunshine @ 2015-11-17  7:20 UTC (permalink / raw)
  To: Keller, Jacob E
  Cc: git, gitster, spearce, lee.marlow, felipe.contreras, szeder,
	jacob.keller

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.
> 
> 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

> > 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< ---
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.

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases
  2015-11-17  7:20         ` Eric Sunshine
@ 2015-11-17  8:25           ` Jacob Keller
  2015-11-17 12:26           ` SZEDER Gábor
  1 sibling, 0 replies; 14+ messages in thread
From: Jacob Keller @ 2015-11-17  8:25 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: Keller, Jacob E, git, gitster, spearce, lee.marlow,
	felipe.contreras, szeder

On Mon, Nov 16, 2015 at 11:20 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases
  2015-11-17  7:20         ` Eric Sunshine
  2015-11-17  8:25           ` Jacob Keller
@ 2015-11-17 12:26           ` SZEDER Gábor
  2015-11-17 16:27             ` Jacob Keller
  1 sibling, 1 reply; 14+ messages in thread
From: SZEDER Gábor @ 2015-11-17 12:26 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: Keller, Jacob E, git, gitster, spearce, lee.marlow,
	felipe.contreras, jacob.keller


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:

   $ 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.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases
  2015-11-17 12:26           ` SZEDER Gábor
@ 2015-11-17 16:27             ` Jacob Keller
  0 siblings, 0 replies; 14+ messages in thread
From: Jacob Keller @ 2015-11-17 16:27 UTC (permalink / raw)
  To: SZEDER Gábor
  Cc: Eric Sunshine, Keller, Jacob E, Git mailing list, Junio C Hamano,
	Shawn Pearce, Lee Marlow, Felipe Contreras

On Tue, Nov 17, 2015 at 4:26 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>
> 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.

>   $ 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.

>
> 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
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-11-17 16:27 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-15 20:22 [PATCH v2 1/2] sendemail: teach git-send-email to list aliases Jacob Keller
2015-11-15 20:22 ` [PATCH v2 2/2] completion: add support for completing email aliases Jacob Keller
2015-11-16 23:33   ` Eric Sunshine
2015-11-16 23:40     ` Keller, Jacob E
2015-11-16 23:30 ` [PATCH v2 1/2] sendemail: teach git-send-email to list aliases Eric Sunshine
2015-11-16 23:40   ` Keller, Jacob E
2015-11-16 23:50     ` Eric Sunshine
2015-11-16 23:56       ` Eric Sunshine
2015-11-17  0:09         ` Keller, Jacob E
2015-11-17  0:10       ` Keller, Jacob E
2015-11-17  7:20         ` Eric Sunshine
2015-11-17  8:25           ` Jacob Keller
2015-11-17 12:26           ` SZEDER Gábor
2015-11-17 16:27             ` Jacob Keller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.