threads / patch / 60937

patchcompletion: use awk for filtering the config entries

Subject: [PATCH] completion: use awk for filtering the config entries

## tl;dr

7 messages between Feb 16, 2024 and Feb 19, 2024. Diffs are folded; open one to read it.

replies: 6people: 4as markdown or json

Beat Bolli· Feb 16, 2024, 17:10 UTC · lore

Commits 1e0ee4087e (completion: add and use __git_compute_first_level_config_vars_for_section, 2024-02-10) and 6e32f718ff (completion: add and use __git_compute_second_level_config_vars_for_section, 2024-02-10) introduced new helpers for config completion.

Both helpers use a pipeline of grep and awk to filter the list of config entries. awk is perfectly capable of filtering, so let's eliminate the grep process and move the filtering into the awk script.

The "-E" grep option (extended syntax) was not necessary, as $section is a single word.

While at it, wrap the over-long lines to make them more readable.
Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---

Junio, this goes on top of 'pb/complete-config' which is on next currently.

 contrib/completion/git-completion.bash | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
Show changes to contrib/completion/git-completion.bash +4 −2
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 444b3efa63..fcf1afd75d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2673,7 +2673,8 @@ __git_compute_first_level_config_vars_for_section ()
 	__git_compute_config_vars
 	local this_section="__git_first_level_config_vars_for_section_${section}"
 	test -n "${!this_section}" ||
-	printf -v "__git_first_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars" | grep -E "^${section}\.[a-z]" | awk -F. '{print $2}')"
+	printf -v "__git_first_level_config_vars_for_section_${section}" %s \
+		"$(echo "$__git_config_vars" | awk -F. "/^${section}\.[a-z]/ { print \$2 }")"
 }
 
 __git_compute_second_level_config_vars_for_section ()
@@ -2682,7 +2683,8 @@ __git_compute_second_level_config_vars_for_section ()
 	__git_compute_config_vars_all
 	local this_section="__git_second_level_config_vars_for_section_${section}"
 	test -n "${!this_section}" ||
-	printf -v "__git_second_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars_all" | grep -E "^${section}\.<" | awk -F. '{print $3}')"
+	printf -v "__git_second_level_config_vars_for_section_${section}" %s \
+		"$(echo "$__git_config_vars_all" | awk -F. "/^${section}\.</ { print \$3 }")"
 }
 
 __git_config_sections=

-- 
2.42.0.583.ga47b40fd90
Junio C Hamano· Feb 16, 2024, 17:35 UTC · re: Beat Bolli · lore

Re: [PATCH] completion: use awk for filtering the config entries

Beat Bolli <dev+git@drbeat.li> writes:
Show 9 quoted lines
> Commits 1e0ee4087e (completion: add and use
> __git_compute_first_level_config_vars_for_section, 2024-02-10) and
> 6e32f718ff (completion: add and use
> __git_compute_second_level_config_vars_for_section, 2024-02-10)
> introduced new helpers for config completion.
>
> Both helpers use a pipeline of grep and awk to filter the list of config
> entries. awk is perfectly capable of filtering, so let's eliminate the
> grep process and move the filtering into the awk script.

Makes sense. I wonder if we can have some simple script sanity checker that catches things like this, e.g., catting a single file into pipe, grep appearing upstream of awk or sed, etc.

Show 10 quoted lines
> The "-E" grep option (extended syntax) was not necessary, as $section is
> a single word.
>
> While at it, wrap the over-long lines to make them more readable.
>
> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
> ---
>
> Junio, this goes on top of 'pb/complete-config' which is on next
> currently.

Alternatively we could redo the topic, squashing this fix in, after the release when we rewind 'next'.

Thanks.
Show 28 quoted lines
>
>  contrib/completion/git-completion.bash | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 444b3efa63..fcf1afd75d 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2673,7 +2673,8 @@ __git_compute_first_level_config_vars_for_section ()
>  	__git_compute_config_vars
>  	local this_section="__git_first_level_config_vars_for_section_${section}"
>  	test -n "${!this_section}" ||
> -	printf -v "__git_first_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars" | grep -E "^${section}\.[a-z]" | awk -F. '{print $2}')"
> +	printf -v "__git_first_level_config_vars_for_section_${section}" %s \
> +		"$(echo "$__git_config_vars" | awk -F. "/^${section}\.[a-z]/ { print \$2 }")"
>  }
>  
>  __git_compute_second_level_config_vars_for_section ()
> @@ -2682,7 +2683,8 @@ __git_compute_second_level_config_vars_for_section ()
>  	__git_compute_config_vars_all
>  	local this_section="__git_second_level_config_vars_for_section_${section}"
>  	test -n "${!this_section}" ||
> -	printf -v "__git_second_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars_all" | grep -E "^${section}\.<" | awk -F. '{print $3}')"
> +	printf -v "__git_second_level_config_vars_for_section_${section}" %s \
> +		"$(echo "$__git_config_vars_all" | awk -F. "/^${section}\.</ { print \$3 }")"
>  }
>  
>  __git_config_sections=
Beat Bolli· Feb 16, 2024, 18:29 UTC · re: Junio C Hamano · lore

Re: [PATCH] completion: use awk for filtering the config entries

On 16.02.24 18:35, Junio C Hamano wrote:
Show 15 quoted lines
> Beat Bolli <dev+git@drbeat.li> writes:
> 
>> Commits 1e0ee4087e (completion: add and use
>> __git_compute_first_level_config_vars_for_section, 2024-02-10) and
>> 6e32f718ff (completion: add and use
>> __git_compute_second_level_config_vars_for_section, 2024-02-10)
>> introduced new helpers for config completion.
>>
>> Both helpers use a pipeline of grep and awk to filter the list of config
>> entries. awk is perfectly capable of filtering, so let's eliminate the
>> grep process and move the filtering into the awk script.
> 
> Makes sense.  I wonder if we can have some simple script sanity
> checker that catches things like this, e.g., catting a single file
> into pipe, grep appearing upstream of awk or sed, etc.

Yes, there are quite a few cases of these in t/. I'm not sure if it's worth the churn, though. At least it would make the tests faster on Windows...

Show 13 quoted lines
>> The "-E" grep option (extended syntax) was not necessary, as $section is
>> a single word.
>>
>> While at it, wrap the over-long lines to make them more readable.
>>
>> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
>> ---
>>
>> Junio, this goes on top of 'pb/complete-config' which is on next
>> currently.
> 
> Alternatively we could redo the topic, squashing this fix in, after
> the release when we rewind 'next'.

As you like. This commit would have to be split to apply to the two original commits.

Show 27 quoted lines
>>   contrib/completion/git-completion.bash | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index 444b3efa63..fcf1afd75d 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -2673,7 +2673,8 @@ __git_compute_first_level_config_vars_for_section ()
>>   	__git_compute_config_vars
>>   	local this_section="__git_first_level_config_vars_for_section_${section}"
>>   	test -n "${!this_section}" ||
>> -	printf -v "__git_first_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars" | grep -E "^${section}\.[a-z]" | awk -F. '{print $2}')"
>> +	printf -v "__git_first_level_config_vars_for_section_${section}" %s \
>> +		"$(echo "$__git_config_vars" | awk -F. "/^${section}\.[a-z]/ { print \$2 }")"
>>   }
>>   
>>   __git_compute_second_level_config_vars_for_section ()
>> @@ -2682,7 +2683,8 @@ __git_compute_second_level_config_vars_for_section ()
>>   	__git_compute_config_vars_all
>>   	local this_section="__git_second_level_config_vars_for_section_${section}"
>>   	test -n "${!this_section}" ||
>> -	printf -v "__git_second_level_config_vars_for_section_${section}" %s "$(echo "$__git_config_vars_all" | grep -E "^${section}\.<" | awk -F. '{print $3}')"
>> +	printf -v "__git_second_level_config_vars_for_section_${section}" %s \
>> +		"$(echo "$__git_config_vars_all" | awk -F. "/^${section}\.</ { print \$3 }")"
>>   }
>>   
>>   __git_config_sections=
Johannes Schindelin· Feb 18, 2024, 21:58 UTC · re: Beat Bolli · lore

Re: [PATCH] completion: use awk for filtering the config entries

Hi Beat,
On Fri, 16 Feb 2024, Beat Bolli wrote:
Show 19 quoted lines
> On 16.02.24 18:35, Junio C Hamano wrote:
> > Beat Bolli <dev+git@drbeat.li> writes:
> >
> > > Commits 1e0ee4087e (completion: add and use
> > > __git_compute_first_level_config_vars_for_section, 2024-02-10) and
> > > 6e32f718ff (completion: add and use
> > > __git_compute_second_level_config_vars_for_section, 2024-02-10)
> > > introduced new helpers for config completion.
> > >
> > > Both helpers use a pipeline of grep and awk to filter the list of config
> > > entries. awk is perfectly capable of filtering, so let's eliminate the
> > > grep process and move the filtering into the awk script.
> >
> > Makes sense.  I wonder if we can have some simple script sanity
> > checker that catches things like this, e.g., catting a single file
> > into pipe, grep appearing upstream of awk or sed, etc.
>
> Yes, there are quite a few cases of these in t/. I'm not sure if it's worth
> the churn, though. At least it would make the tests faster on Windows...
Thank you for caring about the speed on Windows!

Ciao, Johannes

Junio C Hamano· Feb 19, 2024, 17:19 UTC · re: Johannes Schindelin · lore

Re: [PATCH] completion: use awk for filtering the config entries

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 8 quoted lines
>> > Makes sense.  I wonder if we can have some simple script sanity
>> > checker that catches things like this, e.g., catting a single file
>> > into pipe, grep appearing upstream of awk or sed, etc.
>>
>> Yes, there are quite a few cases of these in t/. I'm not sure if it's worth
>> the churn, though. At least it would make the tests faster on Windows...
>
> Thank you for caring about the speed on Windows!
Yup, that is why I asked ;-)
Philippe Blain· Feb 16, 2024, 18:47 UTC · re: Junio C Hamano · lore

Re: [PATCH] completion: use awk for filtering the config entries

Hi Beat and Junio,
Le 2024-02-16 à 12:35, Junio C Hamano a écrit :
Show 13 quoted lines
> Beat Bolli <dev+git@drbeat.li> writes:
> 
>> Commits 1e0ee4087e (completion: add and use
>> __git_compute_first_level_config_vars_for_section, 2024-02-10) and
>> 6e32f718ff (completion: add and use
>> __git_compute_second_level_config_vars_for_section, 2024-02-10)
>> introduced new helpers for config completion.
>>
>> Both helpers use a pipeline of grep and awk to filter the list of config
>> entries. awk is perfectly capable of filtering, so let's eliminate the
>> grep process and move the filtering into the awk script.
> 
> Makes sense.  
Yes, thanks for improving that!
Show 19 quoted lines
> I wonder if we can have some simple script sanity
> checker that catches things like this, e.g., catting a single file
> into pipe, grep appearing upstream of awk or sed, etc.
> 
>> The "-E" grep option (extended syntax) was not necessary, as $section is
>> a single word.
>>
>> While at it, wrap the over-long lines to make them more readable.
>>
>> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
>> ---
>>
>> Junio, this goes on top of 'pb/complete-config' which is on next
>> currently.
> 
> Alternatively we could redo the topic, squashing this fix in, after
> the release when we rewind 'next'.
> 
> Thanks.

Actually you already merged this topic to master in 89400c3615, so it would go on top, no ?

Thanks, Philippe.

Junio C Hamano· Feb 16, 2024, 20:12 UTC · re: Philippe Blain · lore

Re: [PATCH] completion: use awk for filtering the config entries

Philippe Blain <levraiphilippeblain@gmail.com> writes:
Show 10 quoted lines
>>> Junio, this goes on top of 'pb/complete-config' which is on next
>>> currently.
>> 
>> Alternatively we could redo the topic, squashing this fix in, after
>> the release when we rewind 'next'.
>> 
>> Thanks.
>
> Actually you already merged this topic to master in 89400c3615, so it would 
> go on top, no ?

Ah, then that's fine. I didn't check what I read above myself before responding X-<.

← back to recent threads