From: Phillip Wood Date: Mon, 10 Jun 2024 14:09:48 GMT Subject: Re: [PATCH v3 0/6] use the pager in 'add -p' Message-ID: <9f1884ae-0f9f-4d9f-a262-b6929b81d7d8@gmail.com> In-Reply-To: <219a195c-74d0-4c21-bf54-0752bb5b01df@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". > > The "P" command started to resemble a lot to the common pipe operator. > Thus, we shifted to "|". > > 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' command > and let '|' 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.