All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH RFC] completion: add support for completing email aliases
@ 2015-11-14  0:55 SZEDER Gábor
  2015-11-14  8:36 ` Jacob Keller
  0 siblings, 1 reply; 4+ messages in thread
From: SZEDER Gábor @ 2015-11-14  0:55 UTC (permalink / raw)
  To: Jacob Keller
  Cc: git, Junio C Hamano, Shawn O . Pearce, Felipe Contreras,
	Lee Marlow, Jacob Keller


Hi,

Quoting Jacob Keller <jacob.e.keller@intel.com>:

> From: Jacob Keller <jacob.keller@gmail.com>
>
> Extract email aliases from the sendemail.aliasesfile according to the
> known types. Implementation only extracts the alias name and does not
> attempt to complete email addresses.
>
> Add a few tests for simple layouts of the currently supported alias
> filetypes.
>
> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> ---
>
> Labeled this RFC because I have only been able to test the mutt format
> as this is what I use locally. I have a few (probably brittle) test
> cases for the files, but they are not "real" configuration files as per
> the upstream tools, so they are essentially made to work with the simple
> extractors that I have now. I'd like some review on this to see if it's
> valuable, but it definitely helps me type out aliases and see what is
> available by just using TAB.

I think it's definitely valuable, but I'm unsure about parsing the
different alias file formats in shell (well, in awk...), though some
of the parses are much simpler than I expected.
However, 'git send-email' already knows how to parse these files, so
how about letting it do all the work, i.e. teach it a new 'git
send-email --list-aliases' option that would only read the config,
parse the alias file, print all the aliases, i.e. the keys from the
'aliases' associative array and exit.  That way we wouldn't have to
duplicate parts of an already working and tested parsing into a
different language, the completion script would be simpler, because we
wouldn't need __git_email_aliases() helper function, it would
immediately benefit from future bug fixes to 'send-email', and new
alias file formats would Just Work.

> contrib/completion/git-completion.bash | 69 ++++++++++++++++++++++++++
> t/t9902-completion.sh                  | 90
> ++++++++++++++++++++++++++++++++++
> 2 files changed, 159 insertions(+)
>
> diff --git a/contrib/completion/git-completion.bash
> b/contrib/completion/git-completion.bash
> index 482ca84b451b..9b786bb390ba 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
> @@ -785,6 +786,56 @@ __git_aliased_command ()
> 	done
> }
>
> +# Print aliases for email addresses from sendemail.aliasesfile
> +__git_email_aliases ()
> +{
> +	local file="$(git --git-dir="$(__gitdir)" config --path
> sendemail.aliasesfile)"
> +	local filetype="$(git --git-dir="$(__gitdir)" config
> sendemail.aliasfiletype)"

Using $(__gitdir) is good.  Running it twice not so much.  fork()ing a
subshell and fork()+exec()ing a git command is expensive on some
platforms, most importantly Windows.
Please run it only once, store the returned path in a variable, and
pass that to the 'git config' commands.

Sidenote, just wondering: why are these config keys named
'aliasfiletype' and 'alias*es*file'?!

> +	# Only run awk if we find an actual file
> +	if ! [ -f $file ]; then
> +		return
> +	fi

According to the docs multiple alias files can be configured, but this
would only work with one.
That 'git send-email --list-aliases' would take care of this, too.

> +
> +	case "$filetype" in
> +		# Each file type needs to be parsed differently.
> +		mutt|mailrc)
> +			# Mutt and mailrc are simple and just put the alias in
> +			# the 2nd field of the file.
> +			awk '{print $2}' $file
> +			return
> +			;;
> +		sendmail)
> +			# Skip new lines, lines without fields, and lines
> +			# ending in '\' then print the name minus the final :
> +			awk 'NF && $1!~/^#/ && !/\\$/ {sub(/:$/, "", $1); print $1 }' $file
> +			return
> +			;;
> +		pine)
> +			# According to spec, line continuations are any line
> +			# which starts with whitespace, otherwise we can just
> +			# use the normal separator and print the first field.
> +			awk '/^\S/ {print $1}' "$file"
> +			return
> +			;;
> +		elm)
> +			# Elm doesn't appear to allow newlines, and
> +			# git-send-email only accepts one alias per line, so
> +			# just print the first field.
> +			awk '{print $1}' "$file"
> +			return
> +			;;
> +		gnus)
> +			# The gnus format has the alias quoted, so we just use
> +			# gsub to extract the alias from the quotes
> +			awk '/define-mail-alias/ {gsub(/"/, "", $2); print $2}' $file
> +			return
> +			;;
> +		*)
> +			return;;
> +	esac
> +}

Since there is nothing after the case statement in this function, the
return statements in each branch are unnecessary, as is the last
catch-all branch.

> +
> # __git_find_on_cmdline requires 1 argument
> __git_find_on_cmdline ()
> {
> @@ -1735,6 +1786,24 @@ _git_send_email ()
> 			" "" "${cur##--thread=}"
> 		return
> 		;;
> +	--to=*)
> +		__gitcomp "
> +		$(__git_email_aliases)
> +		" "" "${cur##--to=}"
> +		return
> +		;;
> +	--cc=*)
> +		__gitcomp "
> +		$(__git_email_aliases)
> +		" "" "${cur##--cc=}"
> +		return
> +		;;
> +	--bcc=*)
> +		__gitcomp "
> +		$(__git_email_aliases)
> +		" "" "${cur##--bcc=}"
> +		return
> +		;;

These case branches look essentially the same except what has to be
removed from the word being completed.  And what gets removed is
essentially everything before and including the first '='.
So how about squashing these three cases into a single (untested) one
like this:

      --to=*|--cc=*|--bcc=*)
        __gitcomp "$(__git_email_aliases)" "" "${cur#*=}"
        return
        ;;

> 	--*)
> 		__gitcomp "--annotate --bcc --cc --cc-cmd --chain-reply-to
> 			--compose --confirm= --dry-run --envelope-sender
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 2ba62fbc178e..0549f75e6e7c 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -404,6 +404,96 @@ test_expect_success '__git_aliases' '
> 	test_cmp expect actual
> '
>
> +test_expect_success '__git_email_aliases (mutt)' '
> +	cat >aliases <<-EOF &&
> +	alias user1 Some User <user1@example.org>
> +	alias user2 random-user-foo@foo.garbage
> +	EOF
> +	cat >expect <<-EOF &&
> +	user1
> +	user2
> +	EOF
> +	test_config sendemail.aliasesfile aliases &&
> +	test_config sendemail.aliasfiletype mutt &&
> +	__git_email_aliases >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success '__git_email_aliases (mailrc)' '
> +	cat >aliases <<-EOF &&
> +	alias user1 Some User <user1@example.org>
> +	alias user2 random-user-foo@foo.garbage
> +	EOF
> +	cat >expect <<-EOF &&
> +	user1
> +	user2
> +	EOF
> +	test_config sendemail.aliasesfile aliases &&
> +	test_config sendemail.aliasfiletype mailrc &&
> +	__git_email_aliases >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success '__git_email_aliases (sendmail)' '
> +	cat >aliases <<-EOF &&
> +	user1: Some User <user1@example.org>
> +	user2: random-user-foo@foo.garbage
> +	EOF
> +	cat >expect <<-EOF &&
> +	user1
> +	user2
> +	EOF
> +	test_config sendemail.aliasesfile aliases &&
> +	test_config sendemail.aliasfiletype sendmail &&
> +	__git_email_aliases >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success '__git_email_aliases (pine)' '
> +	cat >aliases <<-EOF &&
> +	user1	Some User	user1@example.org>
> +	user2	random-user-foo@foo.garbage
> +	EOF
> +	cat >expect <<-EOF &&
> +	user1
> +	user2
> +	EOF
> +	test_config sendemail.aliasesfile aliases &&
> +	test_config sendemail.aliasfiletype pine &&
> +	__git_email_aliases >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success '__git_email_aliases (elm)' '
> +	cat >aliases <<-EOF &&
> +	user1 = User; Someone = user1@example.org
> +	user2 = = user2@garbage.foo
> +	EOF
> +	cat >expect <<-EOF &&
> +	user1
> +	user2
> +	EOF
> +	test_config sendemail.aliasesfile aliases &&
> +	test_config sendemail.aliasfiletype elm &&
> +	__git_email_aliases >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success '__git_email_aliases (gnus)' '
> +	cat >aliases <<-EOF &&
> +	define-mail-alias "user1" "user1@example.org"
> +	define-mail-alias "user2" "user2@arbitrary.foo"
> +	EOF
> +	cat >expect <<-EOF &&
> +	user1
> +	user2
> +	EOF
> +	test_config sendemail.aliasesfile aliases &&
> +	test_config sendemail.aliasfiletype gnus &&
> +	__git_email_aliases >actual &&
> +	test_cmp expect actual
> +'

I didn't check how the contents of these aliases files conform to each
format, but other than that these tests look good.
But then again, with a 'git send-email --list-aliases' option we
wouldn't these tests at all.


Thanks,
Gábor

> +
> test_expect_success 'basic' '
> 	run_completion "git " &&
> 	# built-in
> --
> 2.6.1.264.gbab76a9

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

* Re: [PATCH RFC] completion: add support for completing email aliases
  2015-11-14  0:55 [PATCH RFC] completion: add support for completing email aliases SZEDER Gábor
@ 2015-11-14  8:36 ` Jacob Keller
  2015-11-14  9:44   ` SZEDER Gábor
  0 siblings, 1 reply; 4+ messages in thread
From: Jacob Keller @ 2015-11-14  8:36 UTC (permalink / raw)
  To: SZEDER Gábor
  Cc: Jacob Keller, git, Junio C Hamano, Shawn O . Pearce,
	Felipe Contreras, Lee Marlow

On Fri, Nov 13, 2015 at 4:55 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>
> Hi,
>
> Quoting Jacob Keller <jacob.e.keller@intel.com>:
>
>> From: Jacob Keller <jacob.keller@gmail.com>
>>
>> Extract email aliases from the sendemail.aliasesfile according to the
>> known types. Implementation only extracts the alias name and does not
>> attempt to complete email addresses.
>>
>> Add a few tests for simple layouts of the currently supported alias
>> filetypes.
>>
>> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
>> ---
>>
>> Labeled this RFC because I have only been able to test the mutt format
>> as this is what I use locally. I have a few (probably brittle) test
>> cases for the files, but they are not "real" configuration files as per
>> the upstream tools, so they are essentially made to work with the simple
>> extractors that I have now. I'd like some review on this to see if it's
>> valuable, but it definitely helps me type out aliases and see what is
>> available by just using TAB.
>
>
> I think it's definitely valuable, but I'm unsure about parsing the
> different alias file formats in shell (well, in awk...), though some
> of the parses are much simpler than I expected.
> However, 'git send-email' already knows how to parse these files, so
> how about letting it do all the work, i.e. teach it a new 'git
> send-email --list-aliases' option that would only read the config,
> parse the alias file, print all the aliases, i.e. the keys from the
> 'aliases' associative array and exit.  That way we wouldn't have to
> duplicate parts of an already working and tested parsing into a
> different language, the completion script would be simpler, because we
> wouldn't need __git_email_aliases() helper function, it would
> immediately benefit from future bug fixes to 'send-email', and new
> alias file formats would Just Work.
>

Agreed. I hadn't thought about it this way. We could also add email
addresses to the complete list, if that was a reasonable addition?
Maybe not worth it though.

I'll respin a version like this in the next few days.

>
>> contrib/completion/git-completion.bash | 69 ++++++++++++++++++++++++++
>> t/t9902-completion.sh                  | 90
>> ++++++++++++++++++++++++++++++++++
>> 2 files changed, 159 insertions(+)
>>
>> diff --git a/contrib/completion/git-completion.bash
>> b/contrib/completion/git-completion.bash
>> index 482ca84b451b..9b786bb390ba 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
>> @@ -785,6 +786,56 @@ __git_aliased_command ()
>>         done
>> }
>>
>> +# Print aliases for email addresses from sendemail.aliasesfile
>> +__git_email_aliases ()
>> +{
>> +       local file="$(git --git-dir="$(__gitdir)" config --path
>> sendemail.aliasesfile)"
>> +       local filetype="$(git --git-dir="$(__gitdir)" config
>> sendemail.aliasfiletype)"
>
>
> Using $(__gitdir) is good.  Running it twice not so much.  fork()ing a
> subshell and fork()+exec()ing a git command is expensive on some
> platforms, most importantly Windows.
> Please run it only once, store the returned path in a variable, and
> pass that to the 'git config' commands.
>

Yep. We won't even need this once we just exec git send-email once, anwyays.

> Sidenote, just wondering: why are these config keys named
> 'aliasfiletype' and 'alias*es*file'?!
>

No idea, but we can't really fix it now.

>> +       # Only run awk if we find an actual file
>> +       if ! [ -f $file ]; then
>> +               return
>> +       fi
>
>
> According to the docs multiple alias files can be configured, but this
> would only work with one.
> That 'git send-email --list-aliases' would take care of this, too.
>

Agreed, yep. The --list-aliases sounds like a much stronger approach.

>
>> +
>> +       case "$filetype" in
>> +               # Each file type needs to be parsed differently.
>> +               mutt|mailrc)
>> +                       # Mutt and mailrc are simple and just put the
>> alias in
>> +                       # the 2nd field of the file.
>> +                       awk '{print $2}' $file
>> +                       return
>> +                       ;;
>> +               sendmail)
>> +                       # Skip new lines, lines without fields, and lines
>> +                       # ending in '\' then print the name minus the
>> final :
>> +                       awk 'NF && $1!~/^#/ && !/\\$/ {sub(/:$/, "", $1);
>> print $1 }' $file
>> +                       return
>> +                       ;;
>> +               pine)
>> +                       # According to spec, line continuations are any
>> line
>> +                       # which starts with whitespace, otherwise we can
>> just
>> +                       # use the normal separator and print the first
>> field.
>> +                       awk '/^\S/ {print $1}' "$file"
>> +                       return
>> +                       ;;
>> +               elm)
>> +                       # Elm doesn't appear to allow newlines, and
>> +                       # git-send-email only accepts one alias per line,
>> so
>> +                       # just print the first field.
>> +                       awk '{print $1}' "$file"
>> +                       return
>> +                       ;;
>> +               gnus)
>> +                       # The gnus format has the alias quoted, so we just
>> use
>> +                       # gsub to extract the alias from the quotes
>> +                       awk '/define-mail-alias/ {gsub(/"/, "", $2); print
>> $2}' $file
>> +                       return
>> +                       ;;
>> +               *)
>> +                       return;;
>> +       esac
>> +}
>
>
> Since there is nothing after the case statement in this function, the
> return statements in each branch are unnecessary, as is the last
> catch-all branch.
>

Sure, though we'd drop this anyways. I'm sensing a wonderful pattern here.

>> +
>> # __git_find_on_cmdline requires 1 argument
>> __git_find_on_cmdline ()
>> {
>> @@ -1735,6 +1786,24 @@ _git_send_email ()
>>                         " "" "${cur##--thread=}"
>>                 return
>>                 ;;
>> +       --to=*)
>> +               __gitcomp "
>> +               $(__git_email_aliases)
>> +               " "" "${cur##--to=}"
>> +               return
>> +               ;;
>> +       --cc=*)
>> +               __gitcomp "
>> +               $(__git_email_aliases)
>> +               " "" "${cur##--cc=}"
>> +               return
>> +               ;;
>> +       --bcc=*)
>> +               __gitcomp "
>> +               $(__git_email_aliases)
>> +               " "" "${cur##--bcc=}"
>> +               return
>> +               ;;
>
>
> These case branches look essentially the same except what has to be
> removed from the word being completed.  And what gets removed is
> essentially everything before and including the first '='.
> So how about squashing these three cases into a single (untested) one
> like this:

I tried this, but I wasn't sure if it was reasonable or not inccase
there was an equals later in the section? since glob will match the
longest one?

>
>      --to=*|--cc=*|--bcc=*)
>        __gitcomp "$(__git_email_aliases)" "" "${cur#*=}"
>        return
>
>        ;;
>
>>         --*)
>>                 __gitcomp "--annotate --bcc --cc --cc-cmd --chain-reply-to
>>                         --compose --confirm= --dry-run --envelope-sender
>> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
>> index 2ba62fbc178e..0549f75e6e7c 100755
>> --- a/t/t9902-completion.sh
>> +++ b/t/t9902-completion.sh
>> @@ -404,6 +404,96 @@ test_expect_success '__git_aliases' '
>>         test_cmp expect actual
>> '
>>
>> +test_expect_success '__git_email_aliases (mutt)' '
>> +       cat >aliases <<-EOF &&
>> +       alias user1 Some User <user1@example.org>
>> +       alias user2 random-user-foo@foo.garbage
>> +       EOF
>> +       cat >expect <<-EOF &&
>> +       user1
>> +       user2
>> +       EOF
>> +       test_config sendemail.aliasesfile aliases &&
>> +       test_config sendemail.aliasfiletype mutt &&
>> +       __git_email_aliases >actual &&
>> +       test_cmp expect actual
>> +'
>> +
>> +test_expect_success '__git_email_aliases (mailrc)' '
>> +       cat >aliases <<-EOF &&
>> +       alias user1 Some User <user1@example.org>
>> +       alias user2 random-user-foo@foo.garbage
>> +       EOF
>> +       cat >expect <<-EOF &&
>> +       user1
>> +       user2
>> +       EOF
>> +       test_config sendemail.aliasesfile aliases &&
>> +       test_config sendemail.aliasfiletype mailrc &&
>> +       __git_email_aliases >actual &&
>> +       test_cmp expect actual
>> +'
>> +
>> +test_expect_success '__git_email_aliases (sendmail)' '
>> +       cat >aliases <<-EOF &&
>> +       user1: Some User <user1@example.org>
>> +       user2: random-user-foo@foo.garbage
>> +       EOF
>> +       cat >expect <<-EOF &&
>> +       user1
>> +       user2
>> +       EOF
>> +       test_config sendemail.aliasesfile aliases &&
>> +       test_config sendemail.aliasfiletype sendmail &&
>> +       __git_email_aliases >actual &&
>> +       test_cmp expect actual
>> +'
>> +
>> +test_expect_success '__git_email_aliases (pine)' '
>> +       cat >aliases <<-EOF &&
>> +       user1   Some User       user1@example.org>
>> +       user2   random-user-foo@foo.garbage
>> +       EOF
>> +       cat >expect <<-EOF &&
>> +       user1
>> +       user2
>> +       EOF
>> +       test_config sendemail.aliasesfile aliases &&
>> +       test_config sendemail.aliasfiletype pine &&
>> +       __git_email_aliases >actual &&
>> +       test_cmp expect actual
>> +'
>> +
>> +test_expect_success '__git_email_aliases (elm)' '
>> +       cat >aliases <<-EOF &&
>> +       user1 = User; Someone = user1@example.org
>> +       user2 = = user2@garbage.foo
>> +       EOF
>> +       cat >expect <<-EOF &&
>> +       user1
>> +       user2
>> +       EOF
>> +       test_config sendemail.aliasesfile aliases &&
>> +       test_config sendemail.aliasfiletype elm &&
>> +       __git_email_aliases >actual &&
>> +       test_cmp expect actual
>> +'
>> +
>> +test_expect_success '__git_email_aliases (gnus)' '
>> +       cat >aliases <<-EOF &&
>> +       define-mail-alias "user1" "user1@example.org"
>> +       define-mail-alias "user2" "user2@arbitrary.foo"
>> +       EOF
>> +       cat >expect <<-EOF &&
>> +       user1
>> +       user2
>> +       EOF
>> +       test_config sendemail.aliasesfile aliases &&
>> +       test_config sendemail.aliasfiletype gnus &&
>> +       __git_email_aliases >actual &&
>> +       test_cmp expect actual
>> +'
>
>
> I didn't check how the contents of these aliases files conform to each
> format, but other than that these tests look good.
> But then again, with a 'git send-email --list-aliases' option we
> wouldn't these tests at all.
>

Agreed, we can extend tests for list-aliases but that is even easier.
I really like that approach a lot more.

Regards,
Jake

>
> Thanks,
> Gábor
>
>
>> +
>> test_expect_success 'basic' '
>>         run_completion "git " &&
>>         # built-in
>> --
>> 2.6.1.264.gbab76a9
>
>

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

* Re: [PATCH RFC] completion: add support for completing email aliases
  2015-11-14  8:36 ` Jacob Keller
@ 2015-11-14  9:44   ` SZEDER Gábor
  0 siblings, 0 replies; 4+ messages in thread
From: SZEDER Gábor @ 2015-11-14  9:44 UTC (permalink / raw)
  To: Jacob Keller
  Cc: Jacob Keller, git, Junio C Hamano, Shawn O . Pearce,
	Felipe Contreras, Lee Marlow


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

> On Fri, Nov 13, 2015 at 4:55 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:

>> However, 'git send-email' already knows how to parse these files, so
>> how about letting it do all the work, i.e. teach it a new 'git
>> send-email --list-aliases' option that would only read the config,
>> parse the alias file, print all the aliases, i.e. the keys from the
>> 'aliases' associative array and exit.  That way we wouldn't have to
>> duplicate parts of an already working and tested parsing into a
>> different language, the completion script would be simpler, because we
>> wouldn't need __git_email_aliases() helper function, it would
>> immediately benefit from future bug fixes to 'send-email', and new
>> alias file formats would Just Work.
>>
>
> Agreed. I hadn't thought about it this way. We could also add email
> addresses to the complete list, if that was a reasonable addition?
> Maybe not worth it though.

I don't really see the use case of listing email addresses as well.

> I'll respin a version like this in the next few days.

Great, thank you!


>>> @@ -1735,6 +1786,24 @@ _git_send_email ()
>>>                         " "" "${cur##--thread=}"
>>>                 return
>>>                 ;;
>>> +       --to=*)
>>> +               __gitcomp "
>>> +               $(__git_email_aliases)
>>> +               " "" "${cur##--to=}"
>>> +               return
>>> +               ;;
>>> +       --cc=*)
>>> +               __gitcomp "
>>> +               $(__git_email_aliases)
>>> +               " "" "${cur##--cc=}"
>>> +               return
>>> +               ;;
>>> +       --bcc=*)
>>> +               __gitcomp "
>>> +               $(__git_email_aliases)
>>> +               " "" "${cur##--bcc=}"
>>> +               return
>>> +               ;;
>>
>>
>> These case branches look essentially the same except what has to be
>> removed from the word being completed.  And what gets removed is
>> essentially everything before and including the first '='.
>> So how about squashing these three cases into a single (untested) one
>> like this:
>
> I tried this, but I wasn't sure if it was reasonable or not inccase
> there was an equals later in the section? since glob will match the
> longest one?

That's why I used a single '#' in the parameter expansion below.  A  
single '#' removes the shortest matching prefix pattern, '##' removes  
the longest:

$ cur="foo=bar=baz"
$ echo "#: ${cur#*=}    ##: ${cur##*=}"
#: bar=baz    ##: baz

(Don't ask why we use '##' without glob everywhere... :)

>>
>>      --to=*|--cc=*|--bcc=*)
>>        __gitcomp "$(__git_email_aliases)" "" "${cur#*=}"
>>        return
>>
>>        ;;
>>

There's an other point: these options have an unstuck variant as well,  
i.e. '--to alice'.  It would be great to support those, too.
Perhaps something like this snippet looking at the previous word on  
the command line could come before the big case statement handling the  
current word:

   case "$prev" in
   --to|--cc|--bcc)
     __gitcomp "$(git --git-dir="$dir" send-email --list-aliases 2>/dev/null)"
     return
     ;;
   esca

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

* [PATCH RFC] completion: add support for completing email aliases
@ 2015-11-13 18:17 Jacob Keller
  0 siblings, 0 replies; 4+ messages in thread
From: Jacob Keller @ 2015-11-13 18:17 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>

Extract email aliases from the sendemail.aliasesfile according to the
known types. Implementation only extracts the alias name and does not
attempt to complete email addresses.

Add a few tests for simple layouts of the currently supported alias
filetypes.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---

Labeled this RFC because I have only been able to test the mutt format
as this is what I use locally. I have a few (probably brittle) test
cases for the files, but they are not "real" configuration files as per
the upstream tools, so they are essentially made to work with the simple
extractors that I have now. I'd like some review on this to see if it's
valuable, but it definitely helps me type out aliases and see what is
available by just using TAB.

 contrib/completion/git-completion.bash | 69 ++++++++++++++++++++++++++
 t/t9902-completion.sh                  | 90 ++++++++++++++++++++++++++++++++++
 2 files changed, 159 insertions(+)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 482ca84b451b..9b786bb390ba 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
@@ -785,6 +786,56 @@ __git_aliased_command ()
 	done
 }
 
+# Print aliases for email addresses from sendemail.aliasesfile
+__git_email_aliases ()
+{
+	local file="$(git --git-dir="$(__gitdir)" config --path sendemail.aliasesfile)"
+	local filetype="$(git --git-dir="$(__gitdir)" config sendemail.aliasfiletype)"
+
+	# Only run awk if we find an actual file
+	if ! [ -f $file ]; then
+		return
+	fi
+
+	case "$filetype" in
+		# Each file type needs to be parsed differently.
+		mutt|mailrc)
+			# Mutt and mailrc are simple and just put the alias in
+			# the 2nd field of the file.
+			awk '{print $2}' $file
+			return
+			;;
+		sendmail)
+			# Skip new lines, lines without fields, and lines
+			# ending in '\' then print the name minus the final :
+			awk 'NF && $1!~/^#/ && !/\\$/ {sub(/:$/, "", $1); print $1 }' $file
+			return
+			;;
+		pine)
+			# According to spec, line continuations are any line
+			# which starts with whitespace, otherwise we can just
+			# use the normal separator and print the first field.
+			awk '/^\S/ {print $1}' "$file"
+			return
+			;;
+		elm)
+			# Elm doesn't appear to allow newlines, and
+			# git-send-email only accepts one alias per line, so
+			# just print the first field.
+			awk '{print $1}' "$file"
+			return
+			;;
+		gnus)
+			# The gnus format has the alias quoted, so we just use
+			# gsub to extract the alias from the quotes
+			awk '/define-mail-alias/ {gsub(/"/, "", $2); print $2}' $file
+			return
+			;;
+		*)
+			return;;
+	esac
+}
+
 # __git_find_on_cmdline requires 1 argument
 __git_find_on_cmdline ()
 {
@@ -1735,6 +1786,24 @@ _git_send_email ()
 			" "" "${cur##--thread=}"
 		return
 		;;
+	--to=*)
+		__gitcomp "
+		$(__git_email_aliases)
+		" "" "${cur##--to=}"
+		return
+		;;
+	--cc=*)
+		__gitcomp "
+		$(__git_email_aliases)
+		" "" "${cur##--cc=}"
+		return
+		;;
+	--bcc=*)
+		__gitcomp "
+		$(__git_email_aliases)
+		" "" "${cur##--bcc=}"
+		return
+		;;
 	--*)
 		__gitcomp "--annotate --bcc --cc --cc-cmd --chain-reply-to
 			--compose --confirm= --dry-run --envelope-sender
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 2ba62fbc178e..0549f75e6e7c 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -404,6 +404,96 @@ test_expect_success '__git_aliases' '
 	test_cmp expect actual
 '
 
+test_expect_success '__git_email_aliases (mutt)' '
+	cat >aliases <<-EOF &&
+	alias user1 Some User <user1@example.org>
+	alias user2 random-user-foo@foo.garbage
+	EOF
+	cat >expect <<-EOF &&
+	user1
+	user2
+	EOF
+	test_config sendemail.aliasesfile aliases &&
+	test_config sendemail.aliasfiletype mutt &&
+	__git_email_aliases >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '__git_email_aliases (mailrc)' '
+	cat >aliases <<-EOF &&
+	alias user1 Some User <user1@example.org>
+	alias user2 random-user-foo@foo.garbage
+	EOF
+	cat >expect <<-EOF &&
+	user1
+	user2
+	EOF
+	test_config sendemail.aliasesfile aliases &&
+	test_config sendemail.aliasfiletype mailrc &&
+	__git_email_aliases >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '__git_email_aliases (sendmail)' '
+	cat >aliases <<-EOF &&
+	user1: Some User <user1@example.org>
+	user2: random-user-foo@foo.garbage
+	EOF
+	cat >expect <<-EOF &&
+	user1
+	user2
+	EOF
+	test_config sendemail.aliasesfile aliases &&
+	test_config sendemail.aliasfiletype sendmail &&
+	__git_email_aliases >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '__git_email_aliases (pine)' '
+	cat >aliases <<-EOF &&
+	user1	Some User	user1@example.org>
+	user2	random-user-foo@foo.garbage
+	EOF
+	cat >expect <<-EOF &&
+	user1
+	user2
+	EOF
+	test_config sendemail.aliasesfile aliases &&
+	test_config sendemail.aliasfiletype pine &&
+	__git_email_aliases >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '__git_email_aliases (elm)' '
+	cat >aliases <<-EOF &&
+	user1 = User; Someone = user1@example.org
+	user2 = = user2@garbage.foo
+	EOF
+	cat >expect <<-EOF &&
+	user1
+	user2
+	EOF
+	test_config sendemail.aliasesfile aliases &&
+	test_config sendemail.aliasfiletype elm &&
+	__git_email_aliases >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '__git_email_aliases (gnus)' '
+	cat >aliases <<-EOF &&
+	define-mail-alias "user1" "user1@example.org"
+	define-mail-alias "user2" "user2@arbitrary.foo"
+	EOF
+	cat >expect <<-EOF &&
+	user1
+	user2
+	EOF
+	test_config sendemail.aliasesfile aliases &&
+	test_config sendemail.aliasfiletype gnus &&
+	__git_email_aliases >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'basic' '
 	run_completion "git " &&
 	# built-in
-- 
2.6.1.264.gbab76a9

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

end of thread, other threads:[~2015-11-14  9:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-14  0:55 [PATCH RFC] completion: add support for completing email aliases SZEDER Gábor
2015-11-14  8:36 ` Jacob Keller
2015-11-14  9:44   ` SZEDER Gábor
  -- strict thread matches above, loose matches on Subject: below --
2015-11-13 18:17 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.