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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxxxxxxxx [2] https://lore.kernel.org/git/a2a59f5e-fd55-41d3-8472-b99256e1f428@xxxxxxxxx> [3] https://lore.kernel.org/git/1ae0715d-df76-4019-995e-f00f3506f2ac@xxxxxxxxx

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.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux