{"thread":{"id":"60937","subject":"[PATCH] completion: use awk for filtering the config entries","startedAt":"2024-02-16T17:13:47Z","lastAt":"2024-02-19T17:19:34Z","messageCount":7,"participants":["Beat Bolli","Junio C Hamano","Philippe Blain","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"488798","messageId":"20240216171046.927552-1-dev+git@drbeat.li","threadId":"60937","inReplyTo":null,"subject":"[PATCH] completion: use awk for filtering the config entries","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2024-02-16T17:10:46Z","receivedAt":"2024-02-16T17:13:47Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"Commits 1e0ee4087e (completion: add and use\n__git_compute_first_level_config_vars_for_section, 2024-02-10) and\n6e32f718ff (completion: add and use\n__git_compute_second_level_config_vars_for_section, 2024-02-10)\nintroduced new helpers for config completion.\n\nBoth helpers use a pipeline of grep and awk to filter the list of config\nentries. awk is perfectly capable of filtering, so let's eliminate the\ngrep process and move the filtering into the awk script.\n\nThe \"-E\" grep option (extended syntax) was not necessary, as $section is\na single word.\n\nWhile at it, wrap the over-long lines to make them more readable.\n\nSigned-off-by: Beat Bolli \n---\n\nJunio, this goes on top of 'pb/complete-config' which is on next\ncurrently.\n\n contrib/completion/git-completion.bash | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 444b3efa63..fcf1afd75d 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2673,7 +2673,8 @@ __git_compute_first_level_config_vars_for_section ()\n \t__git_compute_config_vars\n \tlocal this_section=\"__git_first_level_config_vars_for_section_${section}\"\n \ttest -n \"${!this_section}\" ||\n-\tprintf -v \"__git_first_level_config_vars_for_section_${section}\" %s \"$(echo \"$__git_config_vars\" | grep -E \"^${section}\\.[a-z]\" | awk -F. '{print $2}')\"\n+\tprintf -v \"__git_first_level_config_vars_for_section_${section}\" %s \\\n+\t\t\"$(echo \"$__git_config_vars\" | awk -F. \"/^${section}\\.[a-z]/ { print \\$2 }\")\"\n }\n \n __git_compute_second_level_config_vars_for_section ()\n@@ -2682,7 +2683,8 @@ __git_compute_second_level_config_vars_for_section ()\n \t__git_compute_config_vars_all\n \tlocal this_section=\"__git_second_level_config_vars_for_section_${section}\"\n \ttest -n \"${!this_section}\" ||\n-\tprintf -v \"__git_second_level_config_vars_for_section_${section}\" %s \"$(echo \"$__git_config_vars_all\" | grep -E \"^${section}\\.<\" | awk -F. '{print $3}')\"\n+\tprintf -v \"__git_second_level_config_vars_for_section_${section}\" %s \\\n+\t\t\"$(echo \"$__git_config_vars_all\" | awk -F. \"/^${section}\\. writes:\n\n> Commits 1e0ee4087e (completion: add and use\n> __git_compute_first_level_config_vars_for_section, 2024-02-10) and\n> 6e32f718ff (completion: add and use\n> __git_compute_second_level_config_vars_for_section, 2024-02-10)\n> introduced new helpers for config completion.\n>\n> Both helpers use a pipeline of grep and awk to filter the list of config\n> entries. awk is perfectly capable of filtering, so let's eliminate the\n> grep process and move the filtering into the awk script.\n\nMakes sense. I wonder if we can have some simple script sanity\nchecker that catches things like this, e.g., catting a single file\ninto pipe, grep appearing upstream of awk or sed, etc.\n\n> The \"-E\" grep option (extended syntax) was not necessary, as $section is\n> a single word.\n>\n> While at it, wrap the over-long lines to make them more readable.\n>\n> Signed-off-by: Beat Bolli \n> ---\n>\n> Junio, this goes on top of 'pb/complete-config' which is on next\n> currently.\n\nAlternatively we could redo the topic, squashing this fix in, after\nthe release when we rewind 'next'.\n\nThanks.\n\n>\n> contrib/completion/git-completion.bash | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 444b3efa63..fcf1afd75d 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -2673,7 +2673,8 @@ __git_compute_first_level_config_vars_for_section ()\n> \t__git_compute_config_vars\n> \tlocal this_section=\"__git_first_level_config_vars_for_section_${section}\"\n> \ttest -n \"${!this_section}\" ||\n> -\tprintf -v \"__git_first_level_config_vars_for_section_${section}\" %s \"$(echo \"$__git_config_vars\" | grep -E \"^${section}\\.[a-z]\" | awk -F. '{print $2}')\"\n> +\tprintf -v \"__git_first_level_config_vars_for_section_${section}\" %s \\\n> +\t\t\"$(echo \"$__git_config_vars\" | awk -F. \"/^${section}\\.[a-z]/ { print \\$2 }\")\"\n> }\n> \n> __git_compute_second_level_config_vars_for_section ()\n> @@ -2682,7 +2683,8 @@ __git_compute_second_level_config_vars_for_section ()\n> \t__git_compute_config_vars_all\n> \tlocal this_section=\"__git_second_level_config_vars_for_section_${section}\"\n> \ttest -n \"${!this_section}\" ||\n> -\tprintf -v \"__git_second_level_config_vars_for_section_${section}\" %s \"$(echo \"$__git_config_vars_all\" | grep -E \"^${section}\\.<\" | awk -F. '{print $3}')\"\n> +\tprintf -v \"__git_second_level_config_vars_for_section_${section}\" %s \\\n> +\t\t\"$(echo \"$__git_config_vars_all\" | awk -F. \"/^${section}\\. }\n> \n> __git_config_sections=\n"},{"id":"488802","messageId":"4a1d3618-cebe-4c20-89ce-c5dab51af21a@drbeat.li","threadId":"60937","inReplyTo":"xmqqr0hcjorg.fsf@gitster.g","subject":"Re: [PATCH] completion: use awk for filtering the config entries","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2024-02-16T18:29:32Z","receivedAt":"2024-02-16T18:29:36Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"On 16.02.24 18:35, Junio C Hamano wrote:\n> Beat Bolli writes:\n> \n>> Commits 1e0ee4087e (completion: add and use\n>> __git_compute_first_level_config_vars_for_section, 2024-02-10) and\n>> 6e32f718ff (completion: add and use\n>> __git_compute_second_level_config_vars_for_section, 2024-02-10)\n>> introduced new helpers for config completion.\n>>\n>> Both helpers use a pipeline of grep and awk to filter the list of config\n>> entries. awk is perfectly capable of filtering, so let's eliminate the\n>> grep process and move the filtering into the awk script.\n> \n> Makes sense. I wonder if we can have some simple script sanity\n> checker that catches things like this, e.g., catting a single file\n> into pipe, grep appearing upstream of awk or sed, etc.\n\nYes, there are quite a few cases of these in t/. I'm not sure if it's \nworth the churn, though. At least it would make the tests faster on \nWindows...\n\n>> The \"-E\" grep option (extended syntax) was not necessary, as $section is\n>> a single word.\n>>\n>> While at it, wrap the over-long lines to make them more readable.\n>>\n>> Signed-off-by: Beat Bolli \n>> ---\n>>\n>> Junio, this goes on top of 'pb/complete-config' which is on next\n>> currently.\n> \n> Alternatively we could redo the topic, squashing this fix in, after\n> the release when we rewind 'next'.\n\nAs you like. This commit would have to be split to apply to the two \noriginal commits.\n\n>> contrib/completion/git-completion.bash | 6 ++++--\n>> 1 file changed, 4 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n>> index 444b3efa63..fcf1afd75d 100644\n>> --- a/contrib/completion/git-completion.bash\n>> +++ b/contrib/completion/git-completion.bash\n>> @@ -2673,7 +2673,8 @@ __git_compute_first_level_config_vars_for_section ()\n>> \t__git_compute_config_vars\n>> \tlocal this_section=\"__git_first_level_config_vars_for_section_${section}\"\n>> \ttest -n \"${!this_section}\" ||\n>> -\tprintf -v \"__git_first_level_config_vars_for_section_${section}\" %s \"$(echo \"$__git_config_vars\" | grep -E \"^${section}\\.[a-z]\" | awk -F. '{print $2}')\"\n>> +\tprintf -v \"__git_first_level_config_vars_for_section_${section}\" %s \\\n>> +\t\t\"$(echo \"$__git_config_vars\" | awk -F. \"/^${section}\\.[a-z]/ { print \\$2 }\")\"\n>> }\n>> \n>> __git_compute_second_level_config_vars_for_section ()\n>> @@ -2682,7 +2683,8 @@ __git_compute_second_level_config_vars_for_section ()\n>> \t__git_compute_config_vars_all\n>> \tlocal this_section=\"__git_second_level_config_vars_for_section_${section}\"\n>> \ttest -n \"${!this_section}\" ||\n>> -\tprintf -v \"__git_second_level_config_vars_for_section_${section}\" %s \"$(echo \"$__git_config_vars_all\" | grep -E \"^${section}\\.<\" | awk -F. '{print $3}')\"\n>> +\tprintf -v \"__git_second_level_config_vars_for_section_${section}\" %s \\\n>> +\t\t\"$(echo \"$__git_config_vars_all\" | awk -F. \"/^${section}\\.> }\n>> \n>> __git_config_sections=\n\n"},{"id":"488804","messageId":"fcd3f999-a8d1-9f9d-e8fd-071b38124edc@gmail.com","threadId":"60937","inReplyTo":"xmqqr0hcjorg.fsf@gitster.g","subject":"Re: [PATCH] completion: use awk for filtering the config entries","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2024-02-16T18:47:42Z","receivedAt":"2024-02-16T18:47:44Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Beat and Junio,\n\nLe 2024-02-16 à 12:35, Junio C Hamano a écrit :\n> Beat Bolli writes:\n> \n>> Commits 1e0ee4087e (completion: add and use\n>> __git_compute_first_level_config_vars_for_section, 2024-02-10) and\n>> 6e32f718ff (completion: add and use\n>> __git_compute_second_level_config_vars_for_section, 2024-02-10)\n>> introduced new helpers for config completion.\n>>\n>> Both helpers use a pipeline of grep and awk to filter the list of config\n>> entries. awk is perfectly capable of filtering, so let's eliminate the\n>> grep process and move the filtering into the awk script.\n> \n> Makes sense. \n\nYes, thanks for improving that!\n\n> I wonder if we can have some simple script sanity\n> checker that catches things like this, e.g., catting a single file\n> into pipe, grep appearing upstream of awk or sed, etc.\n> \n>> The \"-E\" grep option (extended syntax) was not necessary, as $section is\n>> a single word.\n>>\n>> While at it, wrap the over-long lines to make them more readable.\n>>\n>> Signed-off-by: Beat Bolli \n>> ---\n>>\n>> Junio, this goes on top of 'pb/complete-config' which is on next\n>> currently.\n> \n> Alternatively we could redo the topic, squashing this fix in, after\n> the release when we rewind 'next'.\n> \n> Thanks.\n\nActually you already merged this topic to master in 89400c3615, so it would \ngo on top, no ? \n\nThanks,\nPhilippe.\n"},{"id":"488809","messageId":"xmqqwmr4gocd.fsf@gitster.g","threadId":"60937","inReplyTo":"fcd3f999-a8d1-9f9d-e8fd-071b38124edc@gmail.com","subject":"Re: [PATCH] completion: use awk for filtering the config entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-16T20:12:50Z","receivedAt":"2024-02-16T20:12:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philippe Blain writes:\n\n>>> Junio, this goes on top of 'pb/complete-config' which is on next\n>>> currently.\n>> \n>> Alternatively we could redo the topic, squashing this fix in, after\n>> the release when we rewind 'next'.\n>> \n>> Thanks.\n>\n> Actually you already merged this topic to master in 89400c3615, so it would \n> go on top, no ? \n\nAh, then that's fine. I didn't check what I read above myself\nbefore responding X-<.\n\n"},{"id":"488891","messageId":"aa3e36a1-52d3-0c15-b70b-83c6664757f5@gmx.de","threadId":"60937","inReplyTo":"4a1d3618-cebe-4c20-89ce-c5dab51af21a@drbeat.li","subject":"Re: [PATCH] completion: use awk for filtering the config entries","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2024-02-18T21:58:20Z","receivedAt":"2024-02-18T21:58:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Beat,\n\nOn Fri, 16 Feb 2024, Beat Bolli wrote:\n\n> On 16.02.24 18:35, Junio C Hamano wrote:\n> > Beat Bolli writes:\n> >\n> > > Commits 1e0ee4087e (completion: add and use\n> > > __git_compute_first_level_config_vars_for_section, 2024-02-10) and\n> > > 6e32f718ff (completion: add and use\n> > > __git_compute_second_level_config_vars_for_section, 2024-02-10)\n> > > introduced new helpers for config completion.\n> > >\n> > > Both helpers use a pipeline of grep and awk to filter the list of config\n> > > entries. awk is perfectly capable of filtering, so let's eliminate the\n> > > grep process and move the filtering into the awk script.\n> >\n> > Makes sense. I wonder if we can have some simple script sanity\n> > checker that catches things like this, e.g., catting a single file\n> > into pipe, grep appearing upstream of awk or sed, etc.\n>\n> Yes, there are quite a few cases of these in t/. I'm not sure if it's worth\n> the churn, though. At least it would make the tests faster on Windows...\n\nThank you for caring about the speed on Windows!\n\nCiao,\nJohannes\n"},{"id":"488915","messageId":"xmqq4je4e5i9.fsf@gitster.g","threadId":"60937","inReplyTo":"aa3e36a1-52d3-0c15-b70b-83c6664757f5@gmx.de","subject":"Re: [PATCH] completion: use awk for filtering the config entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-19T17:19:26Z","receivedAt":"2024-02-19T17:19:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> > Makes sense. I wonder if we can have some simple script sanity\n>> > checker that catches things like this, e.g., catting a single file\n>> > into pipe, grep appearing upstream of awk or sed, etc.\n>>\n>> Yes, there are quite a few cases of these in t/. I'm not sure if it's worth\n>> the churn, though. At least it would make the tests faster on Windows...\n>\n> Thank you for caring about the speed on Windows!\n\nYup, that is why I asked ;-)\n"}]}