diff mbox series

[v2,2/5] completion: complete 'submodule.*' config variables

Message ID 426374ff9b3820512f73ef094f9533e6a1ea5cad.1706534882.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series completion: remove hardcoded config variable names | expand

Commit Message

Philippe Blain Jan. 29, 2024, 1:27 p.m. UTC
From: Philippe Blain <levraiphilippeblain@gmail.com>

In the Bash completion script, function
__git_complete_config_variable_name completes config variables and has
special logic to deal with config variables involving user-defined
names, like branch.<name>.* and remote.<name>.*.

This special logic is missing for submodule-related config variables.
Add the appropriate branches to the case statement, making use of the
in-tree '.gitmodules' to list relevant submodules.

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

Comments

Patrick Steinhardt Feb. 8, 2024, 7:42 a.m. UTC | #1
On Mon, Jan 29, 2024 at 01:27:58PM +0000, Philippe Blain via GitGitGadget wrote:
> From: Philippe Blain <levraiphilippeblain@gmail.com>
> 
> In the Bash completion script, function
> __git_complete_config_variable_name completes config variables and has
> special logic to deal with config variables involving user-defined
> names, like branch.<name>.* and remote.<name>.*.
> 
> This special logic is missing for submodule-related config variables.
> Add the appropriate branches to the case statement, making use of the
> in-tree '.gitmodules' to list relevant submodules.
> 
> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 159a4fd8add..8af9bc3f4e1 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2803,6 +2803,19 @@ __git_complete_config_variable_name ()
>  		__gitcomp_nl_append "pushDefault" "$pfx" "$cur_" "${sfx:- }"
>  		return
>  		;;
> +	submodule.*.*)
> +		local pfx="${cur_%.*}."
> +		cur_="${cur_##*.}"
> +		__gitcomp "url update branch fetchRecurseSubmodules ignore active" "$pfx" "$cur_" "$sfx"
> +		return
> +		;;
> +	submodule.*)
> +		local pfx="${cur_%.*}."
> +		cur_="${cur_#*.}"
> +		__gitcomp_nl "$(__git config -f "$(__git rev-parse --show-toplevel)/.gitmodules" --get-regexp 'submodule.*.path' | awk -F. '{print $2}')" "$pfx" "$cur_" "."
> +		__gitcomp_nl_append $'alternateErrorStrategy\nfetchJobs\nactive\nalternateLocation\nrecurse\npropagateBranches' "$pfx" "$cur_" "${sfx:- }"
> +		return
> +		;;

Hm, it feels quite awkward that we have to manually massage the
gitmodules config like this. But the closest tool I could find is
`git submodule status`, which would also end up describing commits in
each of the submodules and thus do needless work. And second, it prints
submodule paths and not submodule names, so it surfaces the wrong info
in the first place.

Ideally, we would create such a tool that makes the information more
accessible to us. But that certainly seems out of scope of this patch
series.

In any case though it would be nice to add some tests for these new
completions.

Patrick

>  	url.*.*)
>  		local pfx="${cur_%.*}."
>  		cur_="${cur_##*.}"
> -- 
> gitgitgadget
> 
>
Philippe Blain Feb. 10, 2024, 3:39 p.m. UTC | #2
Hi Patrick,

Le 2024-02-08 à 02:42, Patrick Steinhardt a écrit :
> On Mon, Jan 29, 2024 at 01:27:58PM +0000, Philippe Blain via GitGitGadget wrote:
>> From: Philippe Blain <levraiphilippeblain@gmail.com>
>>
>> In the Bash completion script, function
>> __git_complete_config_variable_name completes config variables and has
>> special logic to deal with config variables involving user-defined
>> names, like branch.<name>.* and remote.<name>.*.
>>
>> This special logic is missing for submodule-related config variables.
>> Add the appropriate branches to the case statement, making use of the
>> in-tree '.gitmodules' to list relevant submodules.
>>
>> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
>> ---
>>  contrib/completion/git-completion.bash | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index 159a4fd8add..8af9bc3f4e1 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -2803,6 +2803,19 @@ __git_complete_config_variable_name ()
>>  		__gitcomp_nl_append "pushDefault" "$pfx" "$cur_" "${sfx:- }"
>>  		return
>>  		;;
>> +	submodule.*.*)
>> +		local pfx="${cur_%.*}."
>> +		cur_="${cur_##*.}"
>> +		__gitcomp "url update branch fetchRecurseSubmodules ignore active" "$pfx" "$cur_" "$sfx"
>> +		return
>> +		;;
>> +	submodule.*)
>> +		local pfx="${cur_%.*}."
>> +		cur_="${cur_#*.}"
>> +		__gitcomp_nl "$(__git config -f "$(__git rev-parse --show-toplevel)/.gitmodules" --get-regexp 'submodule.*.path' | awk -F. '{print $2}')" "$pfx" "$cur_" "."
>> +		__gitcomp_nl_append $'alternateErrorStrategy\nfetchJobs\nactive\nalternateLocation\nrecurse\npropagateBranches' "$pfx" "$cur_" "${sfx:- }"
>> +		return
>> +		;;
> 
> Hm, it feels quite awkward that we have to manually massage the
> gitmodules config like this. But the closest tool I could find is
> `git submodule status`, which would also end up describing commits in
> each of the submodules and thus do needless work. And second, it prints
> submodule paths and not submodule names, so it surfaces the wrong info
> in the first place.
> 
> Ideally, we would create such a tool that makes the information more
> accessible to us. But that certainly seems out of scope of this patch
> series.
> 
> In any case though it would be nice to add some tests for these new
> completions.

OK, I end up testing them in 3/5 via the __git_compute_first_level_config_vars_for_section
function I'm adding. But it's true I could add the test directly
in 2/5, if it makes more sense.

Thanks for your review !

Philippe.
diff mbox series

Patch

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 159a4fd8add..8af9bc3f4e1 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2803,6 +2803,19 @@  __git_complete_config_variable_name ()
 		__gitcomp_nl_append "pushDefault" "$pfx" "$cur_" "${sfx:- }"
 		return
 		;;
+	submodule.*.*)
+		local pfx="${cur_%.*}."
+		cur_="${cur_##*.}"
+		__gitcomp "url update branch fetchRecurseSubmodules ignore active" "$pfx" "$cur_" "$sfx"
+		return
+		;;
+	submodule.*)
+		local pfx="${cur_%.*}."
+		cur_="${cur_#*.}"
+		__gitcomp_nl "$(__git config -f "$(__git rev-parse --show-toplevel)/.gitmodules" --get-regexp 'submodule.*.path' | awk -F. '{print $2}')" "$pfx" "$cur_" "."
+		__gitcomp_nl_append $'alternateErrorStrategy\nfetchJobs\nactive\nalternateLocation\nrecurse\npropagateBranches' "$pfx" "$cur_" "${sfx:- }"
+		return
+		;;
 	url.*.*)
 		local pfx="${cur_%.*}."
 		cur_="${cur_##*.}"