Re: [PATCH v3 0/6] use the pager in 'add -p'

From: Dragan Simic <dsimic@manjaro.org>
Date: Jun 2, 2024, 17:36 UTC
Message-ID: <81d52b31ce4c287765a43d87d94f526b@manjaro.org>
In-Reply-To: <b7e24b08-40a1-4b18-89f6-e25ab96facaf@gmail.com>

Hello Ruben,

On 2024-06-02 17:38, Rubén Justo wrote:

Show 23 quoted lines

> This iteration, v3, introduces a new command: '|', suggested by Junio,
> instead of the 'P' command proposed in the previous iteration.
> 
> This allows us to use the pager:
> 
>   (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,s,e,p,|,?]? |
> 
> But also to use other programs, like:
> 
>   (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,s,e,p,|,?]? | head
> 
> Or:
> 
>   (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,s,e,p,|,?]? | grep term
> 
> Hopefully, we'll find a way to avoid sending ANSI codes, on demand,
> without disabling it entirely with color.ui=never or any other global
> option.  To make this usable:
> 
>   (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,s,e,p,|,?]? | vim -
> 
> However, the current functionality meets my current needs, so I'm happy
> with it.

The way I see it, using "| <program>" should follow the de facto rules already established by the "--color=auto" command-line option in multiple utilities. Thus, when piping to a custom program, the escape codes that perform the coloring should be stripped.

Show 21 quoted lines

> This, a new 'interactive.pipeCommand' setting, or a new switch: 'add 
> -P',
> are left for discussing in, hopefully, a future series.
> 
> One final note;  I preferred to model the help text this way:
> 
>     y - stage this hunk
>     n - do not stage this hunk
>     q - quit; do not stage this hunk or any of the remaining ones
>     a - stage this hunk and all later hunks in the file
>     d - do not stage this hunk or any of the later hunks in the file
>     j - leave this hunk undecided, see next undecided hunk
>     J - leave this hunk undecided, see next hunk
>     g - select a hunk to go to
>     / - search for a hunk matching the given regex
>     s - split the current hunk into smaller hunks
>     e - manually edit the current hunk
>     p - print the current hunk
>     | - pipe the current hunk to the pager, or |<program> to use a 
> program'
>     ? - print help

I also like this form better, but I think wording could be improved. I'll think a bit more about it, maybe something like this:

       | - use pager to show the current hunk, or use |<program> to 
customize

Also, what's the single quote doing after "use a program"?

Show 20 quoted lines

> Instead of:
> 
>     y - stage this hunk
>     n - do not stage this hunk
>     q - quit; do not stage this hunk or any of the remaining ones
>     a - stage this hunk and all later hunks in the file
>     d - do not stage this hunk or any of the later hunks in the file
>     j - leave this hunk undecided, see next undecided hunk
>     J - leave this hunk undecided, see next hunk
>     g - select a hunk to go to
>     / - search for a hunk matching the given regex
>     s - split the current hunk into smaller hunks
>     e - manually edit the current hunk
>     p - print the current hunk
>     |[program] - pipe the current hunk to a program, the pager if 
> none...
>     ? - print help
> 
> Because I believe it reads better by maintaining a single character
> before the dash.  But I am not opposed to the latter.