| Message ID | pull.1583.git.1694108551683.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Commit | c7153fad2d7e11b28d1cde21db040f8accae1900 |
| Headers | show |
| Series | completion: commit: complete configured trailer tokens | expand |
"Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Philippe Blain <levraiphilippeblain@gmail.com> > > Since 2daae3d1d1 (commit: add --trailer option, 2021-03-23), 'git > commit' can add trailers to commit messages. To make that feature more > pleasant to use at the command line, update the Bash completion code to > offer configured trailer tokens. > > Add a __git_trailer_tokens function to list the configured trailers > tokens, and use it in _git_commit to suggest the configured tokens, > suffixing the completion words with ':' so that the user only has to add > the trailer value. Nice attention to the details. I do not use custom trailers myself, but I can see how this will be useful. The choice of the source of the information (i.e. the configuration variables trailer.*.key) sounds sensible, too. Will queue. Thanks. > Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> > --- > completion: commit: complete configured trailer tokens > > Since 2daae3d1d1 (commit: add --trailer option, 2021-03-23), 'git > commit' can add trailers to commit messages. To make that feature more > pleasant to use at the command line, update the Bash completion code to > offer configured trailer tokens. > > Add a __git_trailer_tokens function to list the configured trailers > tokens, and use it in _git_commit to suggest the configured tokens, > suffixing the completion words with ':' so that the user only has to add > the trailer value. > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1583%2Fphil-blain%2Fcompletion-commit-trailers-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1583/phil-blain/completion-commit-trailers-v1 > Pull-Request: https://github.com/gitgitgadget/git/pull/1583 > > contrib/completion/git-completion.bash | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index 133ec92bfae..b5eb75aadc5 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -1677,6 +1677,11 @@ _git_clone () > > __git_untracked_file_modes="all no normal" > > +__git_trailer_tokens () > +{ > + git config --name-only --get-regexp trailer.\*.key | awk -F. '{print $2}' > +} > + > _git_commit () > { > case "$prev" in > @@ -1701,6 +1706,10 @@ _git_commit () > __gitcomp "$__git_untracked_file_modes" "" "${cur##--untracked-files=}" > return > ;; > + --trailer=*) > + __gitcomp_nl "$(__git_trailer_tokens)" "" "${cur##--trailer=}" ":" > + return > + ;; > --*) > __gitcomp_builtin commit > return > > base-commit: 1fc548b2d6a3596f3e1c1f8b1930d8dbd1e30bf3
Hi Philippe, > Add a __git_trailer_tokens function to list the configured trailers > tokens, and use it in _git_commit to suggest the configured tokens, > suffixing the completion words with ':' so that the user only has to add > the trailer value. Makes sense. I've never dabbled in the completion scripts, so take the following with some salt. > +__git_trailer_tokens () > +{ > + git config --name-only --get-regexp trailer.\*.key | awk -F. '{print $2}' > +} The rest of this script uses `__git config` rather than `git config`. The purpose of `__git` seems to be to respect options given on the command line, so I think we would want to use it here. These "." in "trailer." and ".key" will match any character. We also don't anchor this at beginning and end. Maybe tighten this a bit and use '^trailer\..*\.key$' to behave better in the face of config such as this: [strailer] skeying = "s" [trailerx] keyx = "x" Another thing. Consider such a config: [trailer "q.p"] key = "Q-p-by" The "trailer.q.p.key" config above ends up completing as just "q" because of how you use `print $2`. I see that `git commit --trailer=` itself is fairly relaxed here, so `--trailer=q` effectively ends up picking up "q.p" in the end. Tightening that is obviously out of scope here and I have no opinion if the current behavior there is intended. But maybe we should be a bit less relaxed here and complete to "q.p"? At any rate, it gets weird when you also have "trailer.q.x.key" in your config but we still just suggest the one "q". I see your patch is in next, but maybe some of this tightening might be worthwhile doing on top of it? Martin
Hi Martin, Le 2023-09-11 à 06:20, Martin Ågren a écrit : > Hi Philippe, > >> Add a __git_trailer_tokens function to list the configured trailers >> tokens, and use it in _git_commit to suggest the configured tokens, >> suffixing the completion words with ':' so that the user only has to add >> the trailer value. > > Makes sense. > > I've never dabbled in the completion scripts, so take the following with > some salt. > >> +__git_trailer_tokens () >> +{ >> + git config --name-only --get-regexp trailer.\*.key | awk -F. '{print $2}' >> +} > > The rest of this script uses `__git config` rather than `git config`. > The purpose of `__git` seems to be to respect options given on the > command line, so I think we would want to use it here. > > These "." in "trailer." and ".key" will match any character. We also > don't anchor this at beginning and end. Maybe tighten this a bit and use > '^trailer\..*\.key$' to behave better in the face of config such as > this: > > [strailer] > skeying = "s" > [trailerx] > keyx = "x" > > Another thing. Consider such a config: > > [trailer "q.p"] > key = "Q-p-by" > > The "trailer.q.p.key" config above ends up completing as just "q" > because of how you use `print $2`. I see that `git commit --trailer=` > itself is fairly relaxed here, so `--trailer=q` effectively ends up > picking up "q.p" in the end. Tightening that is obviously out of scope > here and I have no opinion if the current behavior there is intended. > But maybe we should be a bit less relaxed here and complete to "q.p"? At > any rate, it gets weird when you also have "trailer.q.x.key" in your > config but we still just suggest the one "q". > > I see your patch is in next, but maybe some of this tightening might be > worthwhile doing on top of it? > > Martin > These are all good suggestions. I'll send a new version with these fixes on top. Thanks, Philippe.
Thank you for the improvement, I believe this interactive mode with tab completion will be very useful. Philippe Blain via GitGitGadget <gitgitgadget@gmail.com> 于2023年9月8日周五 01:42写道： > > From: Philippe Blain <levraiphilippeblain@gmail.com> > > Since 2daae3d1d1 (commit: add --trailer option, 2021-03-23), 'git > commit' can add trailers to commit messages. To make that feature more > pleasant to use at the command line, update the Bash completion code to > offer configured trailer tokens. > > Add a __git_trailer_tokens function to list the configured trailers > tokens, and use it in _git_commit to suggest the configured tokens, > suffixing the completion words with ':' so that the user only has to add > the trailer value. > > Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> > --- > completion: commit: complete configured trailer tokens > > Since 2daae3d1d1 (commit: add --trailer option, 2021-03-23), 'git > commit' can add trailers to commit messages. To make that feature more > pleasant to use at the command line, update the Bash completion code to > offer configured trailer tokens. > > Add a __git_trailer_tokens function to list the configured trailers > tokens, and use it in _git_commit to suggest the configured tokens, > suffixing the completion words with ':' so that the user only has to add > the trailer value. > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1583%2Fphil-blain%2Fcompletion-commit-trailers-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1583/phil-blain/completion-commit-trailers-v1 > Pull-Request: https://github.com/gitgitgadget/git/pull/1583 > > contrib/completion/git-completion.bash | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index 133ec92bfae..b5eb75aadc5 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -1677,6 +1677,11 @@ _git_clone () > > __git_untracked_file_modes="all no normal" > > +__git_trailer_tokens () > +{ > + git config --name-only --get-regexp trailer.\*.key | awk -F. '{print $2}' > +} > + > _git_commit () > { > case "$prev" in > @@ -1701,6 +1706,10 @@ _git_commit () > __gitcomp "$__git_untracked_file_modes" "" "${cur##--untracked-files=}" > return > ;; > + --trailer=*) > + __gitcomp_nl "$(__git_trailer_tokens)" "" "${cur##--trailer=}" ":" > + return > + ;; > --*) > __gitcomp_builtin commit > return > > base-commit: 1fc548b2d6a3596f3e1c1f8b1930d8dbd1e30bf3 > -- > gitgitgadget
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 133ec92bfae..b5eb75aadc5 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1677,6 +1677,11 @@ _git_clone () __git_untracked_file_modes="all no normal" +__git_trailer_tokens () +{ + git config --name-only --get-regexp trailer.\*.key | awk -F. '{print $2}' +} + _git_commit () { case "$prev" in @@ -1701,6 +1706,10 @@ _git_commit () __gitcomp "$__git_untracked_file_modes" "" "${cur##--untracked-files=}" return ;; + --trailer=*) + __gitcomp_nl "$(__git_trailer_tokens)" "" "${cur##--trailer=}" ":" + return + ;; --*) __gitcomp_builtin commit return