Subject: Re: [PATCH v2 1/1] git-add--interactive.perl: Add progress counter in the prompt

From: Junio C Hamano <[email-masked]>
Date: Mon, 30 Sep 2019 10:34:13 +0900

"Kunal Tyagi via GitGitGadget" <[email-masked]> writes:

> From: Kunal Tyagi <[email-masked]>
> Subject: Re: [PATCH v2 1/1] git-add--interactive.perl: Add progress counter in the prompt

Either of these two, perhaps (I'd use the former if I were writing
this patch):

	add -i: show progress counter in the prompt
	add -i: add progress counter to the prompt

Give an order to the codebase to "become like so".  E.g.

	Report how many hunks have been processed and the total
	number of hunks in the current file in the prompt, and
	adjust the expected output in some tests.

	Signed-off-by: ...

> Adds a feedback regarding how many hunks from the current file have been
> processed and total number of hunks
> Also included: changes to the test script due to change in expected
> output string
>
> Change in UI: displays '(${current-hunk-id} + 1/${total-hunks-in-file})' before
> displaying the current prompt during user interaction
>
> Signed-off-by: Kunal Tyagi <[email-masked]>
> ---
>  git-add--interactive.perl  | 2 +-
>  t/t3701-add-interactive.sh | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index c20ae9e210..51c30e08cb 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -1541,7 +1541,7 @@ sub patch_update_file {
>  		for (@{$hunk[$ix]{DISPLAY}}) {
>  			print;
>  		}
> -		print colored $prompt_color,
> +		print colored $prompt_color,"(". ($ix+1) ."/$num) ",

Don't omit space after comma. or around the "." operators---if you
want to use "." here, that is.  I am not convinced it makes the code
better to use ".", though.

	print colored $prompt_color, "(", ($ix+1), "/$num) ",

is probably how I would write this, as I expect those writing Perl
would know that elements of a comma separated list fed to 'print'
are shown without any extra space in between.

>  			sprintf(__($patch_update_prompt_modes{$patch_mode}{$hunk[$ix]{TYPE}}), $other);
>  
>  		my $line = prompt_single_character;
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index 69991a3168..3a2d9fb607 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -314,7 +314,7 @@ test_expect_success C_LOCALE_OUTPUT 'add first line works' '
>  	git commit -am "clear local changes" &&
>  	git apply patch &&
>  	printf "%s\n" s y y | git add -p file 2>error |
> -		sed -n -e "s/^Stage this hunk[^@]*\(@@ .*\)/\1/" \
> +		sed -n -e "s/^(.*) Stage this hunk[^@]*\(@@ .*\)/\1/" \

Shouldn't this pattern be a bit tighter than "(.*)" (i.e. anything
can come between paren)?

>  		       -e "/^[-+@ \\\\]"/p  >output &&
>  	test_must_be_empty error &&
>  	git diff --cached >diff &&