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

From: Dragan Simic <[email-masked]>
Date: Tue, 04 Jun 2024 20:13:43 +0200

On 2024-06-03 22:19, Rubén Justo wrote:
> On Sun, Jun 02, 2024 at 07:36:37PM +0200, Dragan Simic wrote:
> 
>> 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.
> 
> Interesting.  However, I'd like to find a way to keep the escape codes
> when using programs like: '|head';  perhaps with the '>' command,
> suggested by Junio.
> 
> At any rate, I feel we can leave that, perhaps corner-case scenario, 
> for
> a future series.  As this series is mainly about the 'pager' machinery.

I'd suggest that "| <program>" is made to work as "--color=auto" in
the current series, i.e. with the coloring escape sequences stripped,
which is the safe approach because we don't know is "<program>" capable
of handling those escape sequences, while implementing support for
"--color=always" would be left for a follow-up series.  Using ">" for
that future command makes sense to me, because it suggests an output
redirection in raw form.

Of course, the plain "|" should still leave the coloring escape 
sequences
intact, because the configured pager (or the default pager, configured
by Git internally) is expected to handle them properly.

>> > 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
> 
> Certainly!  It is indeed a sensible idea to improve the wording, 
> avoiding
> the word "pipe" :-).  Thank you.

I'm glad that you like it. :)

>> Also, what's the single quote doing after "use a program"?
> 
> Just a typo.  Sorry.

Ah, I see.  It looked a bit strange. :)

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