Re: [PATCH] add -p: show hunk selection state when selecting hunks

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

Abraham Samuel Adekunle <abrahamadekunle50@gmail.com> writes:

> diff --git a/add-patch.c b/add-patch.c
> index 173a53241e..e70e390506 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -45,7 +45,7 @@ static struct patch_mode patch_mode_add = {
>  		N_("Stage mode change [y,n,q,a,d%s,?]? "),
>  		N_("Stage deletion [y,n,q,a,d%s,?]? "),
>  		N_("Stage addition [y,n,q,a,d%s,?]? "),
> -		N_("Stage this hunk [y,n,q,a,d%s,?]? ")
> +		N_("Stage this hunk [y,n,q,a,d%s,?] %s? ")
>  	},

Three comments:

 * These sets of prompts exist for each front-end that uses the
   interactive patch machinery, and we are looking at the set used
   by "git add -p".  But the "I came back here with K, or I do not
   remember which between k and K I came back here with, and I
   cannot easily tell if the hunk I am looking at is already
   selected" issue is shared with other users like "git reset -p".

 * "chmod +x Makefile && echo >>Makefile && git add -p" would ask if
   you want to stage the mode change of the path and content change
   for the path separately.  You may skip, and later come back with
   K to this question.  The same "hmph, have I selected to use
   this?" issue exists, no?

 * The existing "[choices]? " was designed to be at the very end of
   the question, so that the answer given by the user will come
   immediately after the offered choices.  Adding an overly long
   "selected" or "deselected" to make it "[choices] selected?" does
   not give us a pleasant end-user experience.

Also, after you decided on one hunk when you have two hunks, typing
'j' or 'k' would tell you "No other undecided hunk".  The phrase
used here, "undecided", refers to the choice between USE or SKIP.
To convey the intent clearly, "Select"/"Deselect" feels a rather
indirect way (i.e. "selected for use" vs "selected to skip") to say
what is happening.

Ideally, if we can convey

    Stage this mode change (you previously decided to use it) [y,n,q,a,d%s,?]?
    Stage this mode change (you previously decided to skip it) [y,n,q,a,d%s,?]?
    Stage this deletion (you previously decided to use it) [y,n,q,a,d%sm,?]?
    ...

without wasting too many extra display width, that would be great,
but this patch is not quite there, I am afraid to say.

> @@ -1564,7 +1564,19 @@ static int patch_update_file(struct add_p_state *s,
>  			      (uintmax_t)(file_diff->hunk_nr
>  						? file_diff->hunk_nr
>  						: 1));
> -		printf(_(s->mode->prompt_mode[prompt_mode_type]),
> +		if (prompt_mode_type == PROMPT_HUNK) {
> +			const char *state = "";
> +			if (file_diff->hunk_nr) {
> +				if (hunk->use == USE_HUNK)
> +					state = _("[selected]");
> +				else if (hunk->use == SKIP_HUNK)
> +					state = _("[deselected]");
> +			}
> +			printf(_(s->mode->prompt_mode[prompt_mode_type]),
> +				s->buf.buf, state);
> +		}
> +		else
> +			printf(_(s->mode->prompt_mode[prompt_mode_type]),
>  		       s->buf.buf);
>  		if (*s->s.reset_color_interactive)
>  			fputs(s->s.reset_color_interactive, stdout);