completion: commit: complete configured trailer tokens

PATCH Superseded Philippe Blain via GitGitGadget 5 participants 9 messages Sep 7, 2023 – Sep 16, 2023

lore

Share & tools

lore

Philippe Blain via GitGitGadget

author

2023-09-07 17:43 UTC #0 lore raw

PATCH 1/1 completion: commit: complete configured trailer tokens

From: Philippe Blain <[email protected]>

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 <[email protected]>

---
completion: commit: complete configured trailer tokens

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(+)

contrib/completion/git-completion.bash +9 -0

index 133ec92bfae..b5eb75aadc5 100644
@@ -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

Signature · 1 line

gitgitgadget

Junio C Hamano

2023-09-07 20:20 UTC #1 lore raw

Quoted 11 lines · Philippe Blain via GitGitGadget

From: Philippe Blain <[email protected]>
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.

Quoted 50 lines

Signed-off-by Philippe Blain <[email protected]>
---
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(+)
contrib/completion/git-completion.bash +9 -0
index 133ec92bfae..b5eb75aadc5 100644
@@ -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

2023-09-11 21:39 UTC #2 lore raw

In reply to Philippe Blain via GitGitGadget

Hi Philippe,

Quoted 4 lines

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.

contrib/completion/git-completion.bash _git_clone () · line 1683

Quoted 4 lines

contrib/completion/git-completion.bash +4 -0

+__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

2023-09-12 12:02 UTC #3 lore raw

Hi Martin,

Le 2023-09-11 à 06:20, Martin Ågren a écrit :

Quoted 45 lines

Hi Philippe,
Quoted 4 lines
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.
contrib/completion/git-completion.bash _git_clone () · line 1683
Quoted 4 lines
contrib/completion/git-completion.bash +4 -0
+__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".

Quoted 5 lines

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.

Philippe Blain via GitGitGadget

author

2023-09-12 17:30 UTC #4 lore raw

v2 cover completion: commit: complete configured trailer tokens

In reply to Philippe Blain via GitGitGadget

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

Signature · 1 line

gitgitgadget

Philippe Blain via GitGitGadget

author

2023-09-12 17:30 UTC #5 lore raw

PATCH v2 1/2 completion: commit: complete configured trailer tokens

From: Philippe Blain <[email protected]>

Signed-off-by Philippe Blain <[email protected]>

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

contrib/completion/git-completion.bash +9 -0

index 133ec92bfae..b5eb75aadc5 100644
@@ -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

Signature · 1 line

gitgitgadget

Philippe Blain via GitGitGadget

author

2023-09-12 17:30 UTC #6 lore raw

PATCH v2 2/2 completion: commit: complete trailers tokens more robustly

In reply to Philippe Blain via GitGitGadget

From: Philippe Blain <[email protected]>

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 <[email protected]>

---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

contrib/completion/git-completion.bash +1 -1

index b5eb75aadc5..c23465886e0 100644
@@ -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 ()

Signature · 1 line

gitgitgadget

Junio C Hamano

2023-09-13 00:34 UTC #7 lore raw

Re: [PATCH v2 2/2] completion: commit: complete trailers tokens more robustly

Quoted 7 lines · Philippe Blain via GitGitGadget

From: Philippe Blain <[email protected]>
In the previous commit, we added support for completing configured trailer tokens in 'git commit --trailer'.
Make the implementation more robust by:

Quoted 8 lines

- 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 <[email protected]>
---

Thanks, both, for a quick fix.

Queued.

ZheNing Hu

2023-09-16 13:31 UTC #8 lore raw

In reply to Philippe Blain via GitGitGadget

Thank you for the improvement, I believe this interactive mode with tab completion will be very useful.

Quoted 65 lines · Philippe Blain via GitGitGadget <[email protected]> 于2023年9月8日周五 01:42写道：

From: Philippe Blain <[email protected]>
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 <[email protected]>
---
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(+)
contrib/completion/git-completion.bash +9 -0
index 133ec92bfae..b5eb75aadc5 100644
@@ -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