Hi, Quoting Jacob Keller <jacob.e.keller@xxxxxxxxx>:
From: Jacob Keller <jacob.keller@xxxxxxxxx> 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@xxxxxxxxx> --- 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@xxxxxxxxxxx> + 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@xxxxxxxxxxx> + 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@xxxxxxxxxxx> + 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@xxxxxxxxxxx> + 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@xxxxxxxxxxx + user2 = = user2@xxxxxxxxxxx + 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@xxxxxxxxxxx" + define-mail-alias "user2" "user2@xxxxxxxxxxxxx" + 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
-- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html