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

[Junio C Hamano <gitster@pobox.com>]

Amisha Chhajed <amishhhaaaa@gmail.com> writes:

> 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.

>  	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.

>  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