Re: [PATCH v3 0/6] use the pager in 'add -p'
- From
- Dragan Simic <dsimic@manjaro.org>
- Date
- Jun 4, 2024, 18:13 UTC
- Message-ID
- <844704794168f9fcb85c75014c84cde0@manjaro.org>
- In-Reply-To
- <9d05b41f-c120-4db0-9ee5-e24d20389129@gmail.com>
On 2024-06-03 22:19, Rubén Justo wrote:
Show 16 quoted lines
> 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.
Show 31 quoted lines
>> > 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. :)
Show 20 quoted lines
>> > 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.