All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] completion: commit: complete configured trailer tokens
@ 2023-09-07 17:42 Philippe Blain via GitGitGadget
  2023-09-07 20:20 ` Junio C Hamano
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Philippe Blain via GitGitGadget @ 2023-09-07 17:42 UTC (permalink / raw)
  To: git; +Cc: ZheNing Hu, Philippe Blain, Philippe Blain

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

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

* Re: [PATCH] completion: commit: complete configured trailer tokens
  2023-09-07 17:42 [PATCH] completion: commit: complete configured trailer tokens Philippe Blain via GitGitGadget
@ 2023-09-07 20:20 ` Junio C Hamano
  2023-09-11 10:20 ` Martin Ågren
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2023-09-07 20:20 UTC (permalink / raw)
  To: Philippe Blain via GitGitGadget; +Cc: git, ZheNing Hu, Philippe Blain

"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

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

* Re: [PATCH] completion: commit: complete configured trailer tokens
  2023-09-07 17:42 [PATCH] completion: commit: complete configured trailer tokens Philippe Blain via GitGitGadget
  2023-09-07 20:20 ` Junio C Hamano
@ 2023-09-11 10:20 ` Martin Ågren
  2023-09-12 12:02   ` Philippe Blain
  2023-09-12 17:30 ` [PATCH v2 0/2] " Philippe Blain via GitGitGadget
  2023-09-16 13:30 ` [PATCH] completion: commit: complete configured trailer tokens ZheNing Hu
  3 siblings, 1 reply; 9+ messages in thread
From: Martin Ågren @ 2023-09-11 10:20 UTC (permalink / raw)
  To: Philippe Blain via GitGitGadget; +Cc: git, ZheNing Hu, Philippe Blain

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

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

* Re: [PATCH] completion: commit: complete configured trailer tokens
  2023-09-11 10:20 ` Martin Ågren
@ 2023-09-12 12:02   ` Philippe Blain
  0 siblings, 0 replies; 9+ messages in thread
From: Philippe Blain @ 2023-09-12 12:02 UTC (permalink / raw)
  To: Martin Ågren, Philippe Blain via GitGitGadget; +Cc: git, ZheNing Hu

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.

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

* [PATCH v2 0/2] completion: commit: complete configured trailer tokens
  2023-09-07 17:42 [PATCH] completion: commit: complete configured trailer tokens Philippe Blain via GitGitGadget
  2023-09-07 20:20 ` Junio C Hamano
  2023-09-11 10:20 ` Martin Ågren
@ 2023-09-12 17:30 ` Philippe Blain via GitGitGadget
  2023-09-12 17:30   ` [PATCH v2 1/2] " Philippe Blain via GitGitGadget
  2023-09-12 17:30   ` [PATCH v2 2/2] completion: commit: complete trailers tokens more robustly Philippe Blain via GitGitGadget
  2023-09-16 13:30 ` [PATCH] completion: commit: complete configured trailer tokens ZheNing Hu
  3 siblings, 2 replies; 9+ messages in thread
From: Philippe Blain via GitGitGadget @ 2023-09-12 17:30 UTC (permalink / raw)
  To: git; +Cc: ZheNing Hu, Martin Ågren, Philippe Blain, Philippe Blain

v2: This series adds support for completing configured trailer tokens in
'git commit --trailer'.

Changes since v1:

 * added all Martin's suggestions as an additional commit on top since
   pb/complete-commit-trailer is already in next.

Philippe Blain (2):
  completion: commit: complete configured trailer tokens
  completion: commit: complete trailers tokens more robustly

 contrib/completion/git-completion.bash | 9 +++++++++
 1 file changed, 9 insertions(+)


base-commit: 1fc548b2d6a3596f3e1c1f8b1930d8dbd1e30bf3
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1583%2Fphil-blain%2Fcompletion-commit-trailers-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1583/phil-blain/completion-commit-trailers-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1583

Range-diff vs v1:

 1:  a31d7a29af9 = 1:  a31d7a29af9 completion: commit: complete configured trailer tokens
 -:  ----------- > 2:  9cb33c20294 completion: commit: complete trailers tokens more robustly

-- 
gitgitgadget

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

* [PATCH v2 1/2] completion: commit: complete configured trailer tokens
  2023-09-12 17:30 ` [PATCH v2 0/2] " Philippe Blain via GitGitGadget
@ 2023-09-12 17:30   ` Philippe Blain via GitGitGadget
  2023-09-12 17:30   ` [PATCH v2 2/2] completion: commit: complete trailers tokens more robustly Philippe Blain via GitGitGadget
  1 sibling, 0 replies; 9+ messages in thread
From: Philippe Blain via GitGitGadget @ 2023-09-12 17:30 UTC (permalink / raw)
  To: git
  Cc: ZheNing Hu, Martin Ågren, Philippe Blain, Philippe Blain,
	Philippe Blain

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


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

* [PATCH v2 2/2] completion: commit: complete trailers tokens more robustly
  2023-09-12 17:30 ` [PATCH v2 0/2] " Philippe Blain via GitGitGadget
  2023-09-12 17:30   ` [PATCH v2 1/2] " Philippe Blain via GitGitGadget
@ 2023-09-12 17:30   ` Philippe Blain via GitGitGadget
  2023-09-13  0:34     ` Junio C Hamano
  1 sibling, 1 reply; 9+ messages in thread
From: Philippe Blain via GitGitGadget @ 2023-09-12 17:30 UTC (permalink / raw)
  To: git
  Cc: ZheNing Hu, Martin Ågren, Philippe Blain, Philippe Blain,
	Philippe Blain

From: Philippe Blain <levraiphilippeblain@gmail.com>

In the previous commit, we added support for completing configured
trailer tokens in 'git commit --trailer'.

Make the implementation more robust by:

- using '__git' instead of plain 'git', as the rest of the completion
  script does
- using a stricter pattern for --get-regexp to avoid false hits
- using 'cut' and 'rev' instead of 'awk' to account for tokens including
  dots.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b5eb75aadc5..c23465886e0 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1679,7 +1679,7 @@ __git_untracked_file_modes="all no normal"
 
 __git_trailer_tokens ()
 {
-	git config --name-only --get-regexp trailer.\*.key | awk -F. '{print $2}'
+	__git config --name-only --get-regexp '^trailer\..*\.key$' | cut -d. -f 2- | rev | cut -d. -f2- | rev
 }
 
 _git_commit ()
-- 
gitgitgadget

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

* Re: [PATCH v2 2/2] completion: commit: complete trailers tokens more robustly
  2023-09-12 17:30   ` [PATCH v2 2/2] completion: commit: complete trailers tokens more robustly Philippe Blain via GitGitGadget
@ 2023-09-13  0:34     ` Junio C Hamano
  0 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2023-09-13  0:34 UTC (permalink / raw)
  To: Philippe Blain via GitGitGadget
  Cc: git, ZheNing Hu, Martin Ågren, Philippe Blain

"Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Philippe Blain <levraiphilippeblain@gmail.com>
>
> In the previous commit, we added support for completing configured
> trailer tokens in 'git commit --trailer'.
>
> Make the implementation more robust by:
>
> - using '__git' instead of plain 'git', as the rest of the completion
>   script does
> - using a stricter pattern for --get-regexp to avoid false hits
> - using 'cut' and 'rev' instead of 'awk' to account for tokens including
>   dots.
>
> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
> ---

Thanks, both, for a quick fix.

Queued.

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

* Re: [PATCH] completion: commit: complete configured trailer tokens
  2023-09-07 17:42 [PATCH] completion: commit: complete configured trailer tokens Philippe Blain via GitGitGadget
                   ` (2 preceding siblings ...)
  2023-09-12 17:30 ` [PATCH v2 0/2] " Philippe Blain via GitGitGadget
@ 2023-09-16 13:30 ` ZheNing Hu
  3 siblings, 0 replies; 9+ messages in thread
From: ZheNing Hu @ 2023-09-16 13:30 UTC (permalink / raw)
  To: Philippe Blain via GitGitGadget; +Cc: git, Philippe Blain

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

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

end of thread, other threads:[~2023-09-16 13:31 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-07 17:42 [PATCH] completion: commit: complete configured trailer tokens Philippe Blain via GitGitGadget
2023-09-07 20:20 ` Junio C Hamano
2023-09-11 10:20 ` Martin Ågren
2023-09-12 12:02   ` Philippe Blain
2023-09-12 17:30 ` [PATCH v2 0/2] " Philippe Blain via GitGitGadget
2023-09-12 17:30   ` [PATCH v2 1/2] " Philippe Blain via GitGitGadget
2023-09-12 17:30   ` [PATCH v2 2/2] completion: commit: complete trailers tokens more robustly Philippe Blain via GitGitGadget
2023-09-13  0:34     ` Junio C Hamano
2023-09-16 13:30 ` [PATCH] completion: commit: complete configured trailer tokens ZheNing Hu

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.