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

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Rubén

On 09/06/2024 08:44, Rubén Justo wrote:
> On Sat, Jun 08, 2024 at 07:54:34AM +0200, Dragan Simic wrote:
> 
>> When "|xyz" is used instead, the version of the hunk with no coloring
>> escape sequences should be piped to xyz.
> 
> That is a sane and conservative approach, and I'm not opposed.  However,
> giving the colorful version though a custom pager is a good thing to
> have, I think, i.e: allowing a simple "head" without losing the
> coloring.
> 
> Let's recap a bit.
> 
> Initially, this series aimed to enable sending chunks to the pager
> during "add -p" sessions.
> 
> To reduce the blast radius of spawning a pager for each chunk, we
> introduced a new command "P".
> 
> Junio suggested opening up the command to allow specifying a custom
> pager, in the form of "P<program>".
> 
> The "P" command started to resemble a lot to the common pipe operator.
> Thus, we shifted to "|<program>".
> 
> Some concerns were raised about controlling when to send coloring escape
> sequences.

I'm still not really convinced that the escape sequences are a problem. 
As Peff has pointed out [1] this new command exists primarily to display 
the current hunk. I've asked for concrete examples of programs that it 
would be useful to run from "git add -p" where the escape codes are a 
problem [2,3]. Sadly the replies talked in generic terms about an 
imaginary program without any reference to displaying or processing a 
hunk from "git add -p". Without a clear use case for stripping the 
escape codes I think we should add a single command that pipes the 
colored output to a user specified program. We can make it clear in the 
documentation that the input to the user's command will contain escape 
sequences unless they pass "-c color.diff=false" when starting "git add 
-p". If it becomes clear that there is a use for the plain output we can 
add that at a later stage.

Best Wishes

Phillip

[1] 
https://lore.kernel.org/git/20240605090935.GF2345232@coredump.intra.peff.net
[2] 
https://lore.kernel.org/git/a2a59f5e-fd55-41d3-8472-b99256e1f428@gmail.com>
[3] 
https://lore.kernel.org/git/1ae0715d-df76-4019-995e-f00f3506f2ac@gmail.com

> Several ideas were discussed to address this, including
> introducing a new command ">", a modifier for "|": "||", and others.
> Alternatively, we could leave it up to the user to filter as needed.
> Or, simply, do not send escape codes at all.
> 
> So, looking back at the ideas discussed in the thread, perhaps a
> reasonable next step might be to reintroduce the 'P<program>' command
> and let '|<program>' be the way to send raw, uncolored, chunks.
> 
> This approach makes sense to me.  I'll wait a bit before sending a
> reroll to gather feedback, though.
> 
> Thanks.