From: Dragan Simic <dsimic@manjaro.org>
Date: Mon, 10 Jun 2024 21:02:08 GMT
Subject: Re: [PATCH v3 0/6] use the pager in 'add -p'
Message-ID: <3268c498c9ee60803384db6db2d0e94b@manjaro.org>
In-Reply-To: <c4f54fdb-bf89-405b-a48a-acb58088557c@gmail.com>

Hello all,

On 2024-06-10 21:09, Rubén Justo wrote:
> On Sun, Jun 09, 2024 at 09:57:20AM +0200, Dragan Simic wrote:
>> On 2024-06-09 09:44, Rubén Justo wrote:
>> > Some concerns were raised about controlling when to send coloring escape
>> > sequences.  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.
>> 
>> Actually, it would be better to re-introduce the "P" option, without
>> any parameters, which would display the current hunk through the
>> already configured pager
> 
> I'm sorry, but why limit the "P" command now?

I'll explain below, and provide an alternate approach.

> I understand the caution expressed in another message of this thread
> about playing it safe, but I think the user won't be surprised if we
> respect here the "color.diff" setting, just like we do with "p", and
> ...

Let me provide a quotation from git-config(1):

   color.diff
            Whether to use ANSI escape sequences to add color to
            patches. If this is set to always, git-diff(1), git-log(1),
            and git-show(1) will use color for all patches. If it
            is set to true or auto, those commands will only use color
            when output is to the terminal. If unset, then the value
            of color.ui is used (auto by default).

The key takeaway there is "when output is to the terminal", but when
you think about it, the description is a bit vague, because the output
to the terminal is also piped through the default pager when running
git-log(1), for example.  Perhaps the more accurate wording could be
"when the known destination of the output is the terminal", but that
might also be a bit confusing.

Thus, if we want to honor the "color.diff" setting, which I think we
should, then we can do everything with only one new command for "git
add -p", which would be "P<program>".  It would be seen as just another
"channel" for the "output to the terminal" category, and it would honor
"color.diff" just like git-log(1) does, for example, both when received
as "P" and as "Pxyz" from the user.

As a result, there would be no way to pipe a hunk to "xyz" with the
coloring escape sequences removed, unless "color.diff" is set to false.
That would be the most consistent approach, because "P<program>" would
be just another "channel" to reaching the terminal with the output.

Also, we'd need to update the documentation for "color.diff" to include
a description of the new "git add -p" command.

>> and let "|<program>" be the new option
>> that pipes hunks _without_ coloring escape sequences to "<program>".
> 
> ... we'll offer the command "|" to allow the user to process the raw
> chunk.

Sorry, but now I have to ask why do we want that?  If the goal is to
open the hunk in an editor, there's already a command in "git add -p"
for that purpose.