Re: [PATCH v4 1/1] help: cleanup the contruction of keys_uniq

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 2, 2026, 16:04 UTC
Message-ID: <xmqqwlzu43rh.fsf@gitster.g>
In-Reply-To: <20260228104654.80831-2-amishhhaaaa@gmail.com>

Amisha Chhajed <amishhhaaaa@gmail.com> writes:

Show 15 quoted lines

> diff --git a/t/t0012-help.sh b/t/t0012-help.sh
> index d3a0967e9d..03104b3bf4 100755
> --- a/t/t0012-help.sh
> +++ b/t/t0012-help.sh
> @@ -141,20 +141,20 @@ test_expect_success 'git help -c' '
>  
>  	'\''git help config'\'' for more information
>  	EOF
> -	grep -v -E \
> -		-e "^[^.]+\.[^.]+$" \
> -		-e "^[^.]+\.[^.]+\.[^.]+$" \
> +	sed \
> +		-e "/^[^.]*\.[^.]*$/d" \
> +		-e "/^[^.]*\.[^.]*\.[^.]*$/d" \
>  		help.output >actual &&

We used to require at least one non-dot byte between each dot in the original, but now we do not. Is this change in semantics intended?

You could fix it with "sed -E" and keeping the ERE in the original, I guess?

It was in a distant past when I tried benchmarking them for the last time, but I recall "sed" was a lot slower than "grep" on a "match and print" job that "grep" could be an alternative. So I am not sure what the point of the change in this hunk is.

Show 14 quoted lines

>  	test_cmp expect actual
>  '
>  
>  test_expect_success 'git help --config-for-completion' '
>  	git help -c >human &&
> -	grep -E \
> -	     -e "^[^.]+\.[^.]+$" \
> -	     -e "^[^.]+\.[^.]+\.[^.]+$" human |
> -	     sed -e "s/\*.*//" -e "s/<.*//" |
> -	     sort -u >human.munged &&
> +	sed -n \
> +	     -e "/^[^.]*\.[^.]*$/p" \
> +	     -e "/^[^.]*\.[^.]*\.[^.]*$/p" human |
> +	sed -e "s/\*.*//" -e "s/<.*//" |

Ditto.

Show 11 quoted lines

>  test_expect_success 'git help --config-sections-for-completion' '
>  	git help -c >human &&
> -	grep -E \
> -	     -e "^[^.]+\.[^.]+$" \
> -	     -e "^[^.]+\.[^.]+\.[^.]+$" human |
> -	     sed -e "s/\..*//" |
> -	     sort -u >human.munged &&
> +	sed -n \
> +	     -e "/^[^.]*\.[^.]*$/p" \
> +	     -e "/^[^.]*\.[^.]*\.[^.]*$/p" human |
> +	sed -e "s/\..*//" |

Ditto.

Just like piping "grep" output to "sed" is an anti-pattern, piping "sed" output to an invocation of "sed" is often an anti-pattern.

Perhaps something like this would replace the original "grep | sed" pipeline?

	sed -E -e "
		/^[^.]+\.[^.]+$/b out
		/^[^.]+\.[^.]+\.[^.]+$/b out
		d
		: out
		s/\..*//
	" human |
	sort -u