diff mbox series

completion: commit: complete configured trailer tokens

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

Commit Message

Philippe Blain Sept. 7, 2023, 5:42 p.m. UTC
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(+)


base-commit: 1fc548b2d6a3596f3e1c1f8b1930d8dbd1e30bf3

Comments

Junio C Hamano Sept. 7, 2023, 8:20 p.m. UTC | #1
"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
Martin Ågren Sept. 11, 2023, 10:20 a.m. UTC | #2
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
Philippe Blain Sept. 12, 2023, 12:02 p.m. UTC | #3
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.
ZheNing Hu Sept. 16, 2023, 1:30 p.m. UTC | #4
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 mbox series

Patch

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