| Message ID | eb0438e8-d7b6-478f-b2be-336e83f5d9ab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | use the pager in 'add -p' | expand |
Rubén Justo <rjusto@gmail.com> writes: > Invoke the pager when displaying hunks during "add -p" sessions, to make > it easier for the user to review hunks longer than one screen height. If the hunk fits on one screen, it is annoying to see a pager invoked and then torn down immediately, even with "less -F" (--quit-if-one-screen). As we know how much output we are throwing at the user, we'd want to make this conditional to the size of the hunk being shown and the terminal height. Or perhaps show them without such a trick, and add a new variant to 'p' that allows the user to request the output be sent to a pager (perhaps 'P')? It would certainly be an alternative with much smaller damage. The existing end-user experience would not degrade, but when the user wants to see a huge hunk, they can send it to the pager. Another, ulteriour, motive here behind this suggestion is to encourage users to work with smaller hunks. Being able to scroll around and view lines on demand (i.e. use of pager) is one thing. Being able to view all relevant lines at once (i.e. not wasting vertical screen real estate and making things fit on one screen) is very different and much nicer.
Hello Junio and Ruben, On 2024-05-20 21:30, Junio C Hamano wrote: > Rubén Justo <rjusto@gmail.com> writes: > >> Invoke the pager when displaying hunks during "add -p" sessions, to >> make >> it easier for the user to review hunks longer than one screen height. > > If the hunk fits on one screen, it is annoying to see a pager > invoked and then torn down immediately, even with "less -F" > (--quit-if-one-screen). As we know how much output we are throwing > at the user, we'd want to make this conditional to the size of the > hunk being shown and the terminal height. > > Or perhaps show them without such a trick, and add a new variant to > 'p' that allows the user to request the output be sent to a pager > (perhaps 'P')? It would certainly be an alternative with much > smaller damage. The existing end-user experience would not degrade, > but when the user wants to see a huge hunk, they can send it to the > pager. > > Another, ulteriour, motive here behind this suggestion is to > encourage users to work with smaller hunks. Being able to scroll > around and view lines on demand (i.e. use of pager) is one thing. > Being able to view all relevant lines at once (i.e. not wasting > vertical screen real estate and making things fit on one screen) is > very different and much nicer. There's another thing to consider, which makes the introduction of "P" as the new option even more desirable. Let me explain. With the upcoming changes to the way less(1) as the pager works, which was already discussed at length and even required new features to be implemented in less(1), [1] displaying anything through less(1) will not leave an accessible scrollback in the terminal emulator. Only one screen worth of text will be displayed, even after quitting less(1). That's what we have to do, to fix age-old issues with the pager-generated scrollback that easily gets corrupted and actually becomes misleading. Thus, if someone wants to have a complete longer-than-one-screen hunk displayed and use the terminal emulator scrollback to inspect the hunk in its entirety, passing such (or all) hunks through the pager would make such inspection impossible. I'd assume that at least some Git users already do that (I do, for example), and we surely don't want to make that no longer possible. That's why introducing "P" as the new option would be the desired approach. [1] https://lore.kernel.org/git/8289ef15266172cbfa10bb146afe9797@manjaro.org/
On Mon, May 20, 2024 at 09:45:51PM +0200, Dragan Simic wrote: > Thus, if someone wants to have a complete longer-than-one-screen hunk > displayed and use the terminal emulator scrollback to inspect the > hunk in its entirety, passing such (or all) hunks through the pager > would make such inspection impossible. I'd assume that at least some > Git users already do that (I do, for example), and we surely don't want > to make that no longer possible. I'm not sure if I understand the problem... disabling the pager is still an option, no? $ git -P add -p $ git -c pager.add=false add -p
On Mon, May 20, 2024 at 12:30:50PM -0700, Junio C Hamano wrote: > > Invoke the pager when displaying hunks during "add -p" sessions, to make > > it easier for the user to review hunks longer than one screen height. > > If the hunk fits on one screen, it is annoying to see a pager > invoked and then torn down immediately, Good point. > even with "less -F" > (--quit-if-one-screen). As we know how much output we are throwing > at the user, we'd want to make this conditional to the size of the > hunk being shown and the terminal height. Are you thinking of something like?: diff --git a/add-patch.c b/add-patch.c index cefa3941a3..495baad3ac 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1449,11 +1449,18 @@ static int patch_update_file(struct add_p_state *s, strbuf_reset(&s->buf); if (file_diff->hunk_nr) { if (rendered_hunk_index != hunk_index) { - setup_pager(); + int lines = 0; render_hunk(s, hunk, 0, colored, &s->buf); + for(int i = 0; i < s->buf.len; i++) { + if (s->buf.buf[i] == '\n') + lines++; + } + if (lines > term_columns()) + setup_pager(); fputs(s->buf.buf, stdout); rendered_hunk_index = hunk_index; - wait_for_pager(); + if (lines > term_columns()) + wait_for_pager(); } strbuf_reset(&s->buf); This would significantly reduce the blast radius. Thanks.
Rubén Justo <rjusto@gmail.com> writes: >> even with "less -F" >> (--quit-if-one-screen). As we know how much output we are throwing >> at the user, we'd want to make this conditional to the size of the >> hunk being shown and the terminal height. > > Are you thinking of something like?: I don't. Your hunk may have overly wide lines in which case counting the number of lines may be insuffucient to measure the necessary display height. Besides, comparison with term_columns() is meaningless unless your window is square ;-) An explicit 'P' might be palatable, though. Thanks. > > diff --git a/add-patch.c b/add-patch.c > index cefa3941a3..495baad3ac 100644 > --- a/add-patch.c > +++ b/add-patch.c > @@ -1449,11 +1449,18 @@ static int patch_update_file(struct add_p_state *s, > strbuf_reset(&s->buf); > if (file_diff->hunk_nr) { > if (rendered_hunk_index != hunk_index) { > - setup_pager(); > + int lines = 0; > render_hunk(s, hunk, 0, colored, &s->buf); > + for(int i = 0; i < s->buf.len; i++) { > + if (s->buf.buf[i] == '\n') > + lines++; > + } > + if (lines > term_columns()) > + setup_pager(); > fputs(s->buf.buf, stdout); > rendered_hunk_index = hunk_index; > - wait_for_pager(); > + if (lines > term_columns()) > + wait_for_pager(); > } > > strbuf_reset(&s->buf); > > This would significantly reduce the blast radius. > > Thanks.
On Mon, May 20, 2024 at 04:18:33PM -0700, Junio C Hamano wrote: > Besides, comparison with term_columns() is meaningless > unless your window is square ;-) XD > > An explicit 'P' might be palatable, though. OK. Thanks.
On 2024-05-21 00:35, Rubén Justo wrote: > On Mon, May 20, 2024 at 09:45:51PM +0200, Dragan Simic wrote: > >> Thus, if someone wants to have a complete longer-than-one-screen hunk >> displayed and use the terminal emulator scrollback to inspect the >> hunk in its entirety, passing such (or all) hunks through the pager >> would make such inspection impossible. I'd assume that at least some >> Git users already do that (I do, for example), and we surely don't >> want >> to make that no longer possible. > > I'm not sure if I understand the problem... disabling the pager is > still an option, no? > > $ git -P add -p > > $ git -c pager.add=false add -p Please read the thread [1] that I linked in my previous response carefully. I know, there's _a lot_ of text, but I already tried to sum it all up in my previous response. There's even a video clip [2] in that thread that shows the issue with the corrupted scrollback in a terminal emulator. Frankly, the "-c pager.add=false" approach is a bit cumbersome. Basically, the new "-P" option would be like "-c pager.add=true", while the already existing "-p" option would be like "-c pager.add=false". Though, I think that we don't want to add "pager.add" as a new configuration option, because throwing it into the mix would make the "-p" and "-P" options for "git add" quite confusing. As another idea, we might also add "p" as another option in the "y/n/q/a/d..." menu when the user decides about each hunk in "git add -p" or "git add -P". When running "git add -p", pressing "p" would display the current hunk using the pager (which would be opposite to the "git add -p"'s behavior of not using the pager), and when running "git add -P", pressing "p" would display the current hunk by bypassing the pager (again, opposite to the "add -P"'s behavior). That would allow greatest level of flexibility. [1] https://lore.kernel.org/git/8289ef15266172cbfa10bb146afe9797@manjaro.org/T/#u [2] https://youtu.be/MsxtQgrKM50
On Mon, May 20, 2024 at 09:45:51PM +0200, Dragan Simic wrote: > > Another, ulteriour, motive here behind this suggestion is to > > encourage users to work with smaller hunks. Being able to scroll > > around and view lines on demand (i.e. use of pager) is one thing. > > Being able to view all relevant lines at once (i.e. not wasting > > vertical screen real estate and making things fit on one screen) is > > very different and much nicer. > > There's another thing to consider, which makes the introduction of > "P" as the new option even more desirable. Let me explain. > > With the upcoming changes to the way less(1) as the pager works, > which was already discussed at length and even required new features > to be implemented in less(1), [1] displaying anything through less(1) > will not leave an accessible scrollback in the terminal emulator. > Only one screen worth of text will be displayed, even after quitting > less(1). That's what we have to do, to fix age-old issues with the > pager-generated scrollback that easily gets corrupted and actually > becomes misleading. This feature can be annoying even with current versions of less, depending on your $LESS variable. If you don't set "F" you'll get a pager for short inputs, and if you don't set "X" then even small hunks are cleared from the screen while we ask about them. So this definitely needs to be configurable, and I'd be tempted to say it should be off by default, just because we don't know how the user's pager will behave when invoked for multiple short snippets like this (it might not even be "less", after all). I don't think setting pager.add is enough here. You'd also need to set pager.checkout, pager.reset, and so on, since their interactive modes all invoke the same code. We'd presumably want a single config option (and possibly even one that could be set to a distinct pager command for this context, rather than the usual one). -Peff
On Tue, May 21, 2024 at 01:54:22AM +0200, Dragan Simic wrote: > Though, I think that we don't want to add "pager.add" as a new > configuration option I have no plans to add a new git-add(1) option or a new configuration option. Only a new interactive option 'P'. I do not see the need for them, but maybe I'm missing some use case. I'm going send a new iteration, v2; please, take a look at it. Thanks.
On Tue, May 21, 2024 at 03:07:52AM -0400, Jeff King wrote: > On Mon, May 20, 2024 at 09:45:51PM +0200, Dragan Simic wrote: > > > > Another, ulteriour, motive here behind this suggestion is to > > > encourage users to work with smaller hunks. Being able to scroll > > > around and view lines on demand (i.e. use of pager) is one thing. > > > Being able to view all relevant lines at once (i.e. not wasting > > > vertical screen real estate and making things fit on one screen) is > > > very different and much nicer. > > > > There's another thing to consider, which makes the introduction of > > "P" as the new option even more desirable. Let me explain. > > > > With the upcoming changes to the way less(1) as the pager works, > > which was already discussed at length and even required new features > > to be implemented in less(1), [1] displaying anything through less(1) > > will not leave an accessible scrollback in the terminal emulator. > > Only one screen worth of text will be displayed, even after quitting > > less(1). That's what we have to do, to fix age-old issues with the > > pager-generated scrollback that easily gets corrupted and actually > > becomes misleading. > > This feature can be annoying even with current versions of less, Hopefully, reducing the blast radius to a new 'P' option, will make it palatable. > depending on your $LESS variable. If you don't set "F" you'll get a > pager for short inputs, and if you don't set "X" then even small hunks > are cleared from the screen while we ask about them. > > So this definitely needs to be configurable, and I'd be tempted to say > it should be off by default, just because we don't know how the user's > pager will behave when invoked for multiple short snippets like this (it > might not even be "less", after all). > > I don't think setting pager.add is enough here. You'd also need to set > pager.checkout, pager.reset, and so on, since their interactive modes > all invoke the same code. We'd presumably want a single config option > (and possibly even one that could be set to a distinct pager command for > this context, rather than the usual one). > > -Peff
On Tue, May 21, 2024 at 09:59:55PM +0200, Rubén Justo wrote: > > This feature can be annoying even with current versions of less, > > Hopefully, reducing the blast radius to a new 'P' option, will make it > palatable. Yeah, that would be perfect. I might even use it, then, for the rare cases when I want to look at a really big hunk. I do still think it would be useful to be able to configure its pager separately (in my case, I'd use "less -FX" rather than my default setup, which doesn't use either of those options). But I'm also OK to leave that for now and let it be somebody else's itch to scratch later. -Peff
Jeff King <peff@peff.net> writes: > I do still think it > would be useful to be able to configure its pager separately (in my > case, I'd use "less -FX" rather than my default setup, which doesn't use > either of those options). Even better. Allow to optionally have the command after the option, e.g., (1/1) Use this hunk [y,n,q,j,k,e,p,P] P<RET> (1/1) Use this hunk [y,n,q,j,k,e,p,P] Pless -FX<RET> (1/1) Use this hunk [y,n,q,j,k,e,p,P] Pcat<RET> The first one feeds the default program with the hunk via pipe, the second one instead invokes command you specifed, "less -FX", and feeds the hunk to it via a pipe. The last one emulates a plain 'p' behaviour. And for usability, perhaps giving a specific command would change the default program a bare 'P' invokes for the rest of the session until another specific command overrides. Another usability hack may be "[interactive] pipecommand = less -FX" configuration variable gives the initial default for each session. At that point, we can explain it as p - print the current hunk P[<program>] - pipe the current hunk to a program or even use '|' instead of 'P'.
On 2024-05-23 16:00, Junio C Hamano wrote: > Jeff King <peff@peff.net> writes: > >> I do still think it >> would be useful to be able to configure its pager separately (in my >> case, I'd use "less -FX" rather than my default setup, which doesn't >> use >> either of those options). > > Even better. Allow to optionally have the command after the option, > e.g., > > (1/1) Use this hunk [y,n,q,j,k,e,p,P] P<RET> > (1/1) Use this hunk [y,n,q,j,k,e,p,P] Pless -FX<RET> > (1/1) Use this hunk [y,n,q,j,k,e,p,P] Pcat<RET> Please note that "-X" will no longer be used as one of the options passed to less(1) as the pager, in the upcoming resolution of the age-old pager issues. [1] In more detail, "-X" is actually an ugly hack that was nothing more than a stopgap measure, but it has never been resolved properly. That is, until recently, when I started to collaborate with the author of less(1) towards finding and implementing the real solution. [1] https://lore.kernel.org/git/8289ef15266172cbfa10bb146afe9797@manjaro.org/T/#u > The first one feeds the default program with the hunk via pipe, the > second one instead invokes command you specifed, "less -FX", and > feeds the hunk to it via a pipe. The last one emulates a plain 'p' > behaviour. Frankly, that would be a lot of typing, and may even open a path for some unforeseen security issues. > And for usability, perhaps giving a specific command would change > the default program a bare 'P' invokes for the rest of the session > until another specific command overrides. Another usability hack > may be "[interactive] pipecommand = less -FX" configuration variable > gives the initial default for each session. I think that would be way too complicated. > At that point, we can explain it as > > p - print the current hunk > P[<program>] - pipe the current hunk to a program > > or even use '|' instead of 'P'.
On Thu, May 23, 2024 at 05:06:01AM -0400, Jeff King wrote: > On Tue, May 21, 2024 at 09:59:55PM +0200, Rubén Justo wrote: > > > > This feature can be annoying even with current versions of less, > > > > Hopefully, reducing the blast radius to a new 'P' option, will make it > > palatable. > > Yeah, that would be perfect. I might even use it, then, for the rare > cases when I want to look at a really big hunk. In addition to that, I have two use-cases that make sense to me: - avoiding a huuge but split-able hunk to go all through my terminal before I can say: split, 's'. For this, perhaps the '-P' suggested by Dragan is the way to go. - a lot of mid-sized hunks that I only need to see, to decide, the result of "head -3". Here, the pager would be acting as a 'filter'. Perhaps I am stretching the meaning of 'pager' too far... > I do still think it would be useful to be able to configure its pager > separately (in my case, I'd use "less -FX" rather than my default > setup, which doesn't use either of those options). A new "interactive.pager" setting? Perhaps with higher preference than "add.pager"? Just questioning, do not take this as an intention of scratching that itch :-)
On 2024-05-24 00:25, Rubén Justo wrote: > On Thu, May 23, 2024 at 05:06:01AM -0400, Jeff King wrote: [...] > In addition to that, I have two use-cases that make sense to me: > > - avoiding a huuge but split-able hunk to go all through my terminal > before I can say: split, 's'. For this, perhaps the '-P' suggested > by Dragan is the way to go. A possible UX issue with the "-P" option is that the menu wouldn't be displayed right after executing "git add -P" if the first displayed hunk is longer than one screen, leaving the users wondering what actually happened. Though, that perhaps could be addressed in the documentation. > - a lot of mid-sized hunks that I only need to see, to decide, the > result of "head -3". Here, the pager would be acting as a > 'filter'. > > Perhaps I am stretching the meaning of 'pager' too far... > >> I do still think it would be useful to be able to configure its pager >> separately (in my case, I'd use "less -FX" rather than my default >> setup, which doesn't use either of those options). > > A new "interactive.pager" setting? Perhaps with higher preference than > "add.pager"? Just questioning, do not take this as an intention of > scratching that itch :-) Huh, I see some possible issues with separate pager configurations, resulting from the upcoming rework of the default less(1)-as-pager options, so perhaps it would, in addition, be better to wait until those changes settle first. I intend to get into that rather soon, not only for Git, but also for a few other projects that use less(1) as their pager by default, such as util-linux.
Dragan Simic <dsimic@manjaro.org> writes: >> And for usability, perhaps giving a specific command would change >> the default program a bare 'P' invokes for the rest of the session >> until another specific command overrides. Another usability hack >> may be "[interactive] pipecommand = less -FX" configuration variable >> gives the initial default for each session. > > I think that would be way too complicated. It is modelled after how "less" and "vi" remembers the last pattern fed to their "/" command. You once give, say, "/test_<ENTER>" to find one instance of "test_", then "/<ENTER>" takes to the next instance. As I expect our target audiences are used to such a behaviour, I do not think I agree with your "way too complicated".
On 2024-05-24 01:04, Junio C Hamano wrote: > Dragan Simic <dsimic@manjaro.org> writes: > >>> And for usability, perhaps giving a specific command would change >>> the default program a bare 'P' invokes for the rest of the session >>> until another specific command overrides. Another usability hack >>> may be "[interactive] pipecommand = less -FX" configuration variable >>> gives the initial default for each session. >> >> I think that would be way too complicated. > > It is modelled after how "less" and "vi" remembers the last pattern > fed to their "/" command. You once give, say, "/test_<ENTER>" to > find one instance of "test_", then "/<ENTER>" takes to the next > instance. Huh, less(1) actually remembers nothing when the secure mode is turned on. That's another thing I've collaborated with the author of less(1), to make it possible to remember the last search term when running less(1) in secure mode. > As I expect our target audiences are used to such a behaviour, I do > not think I agree with your "way too complicated". Hmm. Where would that state be stored?
On 2024-05-24 01:28, Dragan Simic wrote: > On 2024-05-24 01:04, Junio C Hamano wrote: >> Dragan Simic <dsimic@manjaro.org> writes: >> >>>> And for usability, perhaps giving a specific command would change >>>> the default program a bare 'P' invokes for the rest of the session >>>> until another specific command overrides. Another usability hack >>>> may be "[interactive] pipecommand = less -FX" configuration variable >>>> gives the initial default for each session. >>> >>> I think that would be way too complicated. >> >> It is modelled after how "less" and "vi" remembers the last pattern >> fed to their "/" command. You once give, say, "/test_<ENTER>" to >> find one instance of "test_", then "/<ENTER>" takes to the next >> instance. > > Huh, less(1) actually remembers nothing when the secure mode is > turned on. That's another thing I've collaborated with the author > of less(1), to make it possible to remember the last search term > when running less(1) in secure mode. > >> As I expect our target audiences are used to such a behaviour, I do >> not think I agree with your "way too complicated". > > Hmm. Where would that state be stored? Ah, sorry, I misread your description a bit. There's obviously no need to store any state, because this additional feature doesn't break the boundaries of a single session. However, I'd suggest that we leave this additional feature aside for now, until the upcoming pager-related changes and fixes settle down.
Dragan Simic <dsimic@manjaro.org> writes: > On 2024-05-24 01:04, Junio C Hamano wrote: >> Dragan Simic <dsimic@manjaro.org> writes: >> >>>> And for usability, perhaps giving a specific command would change >>>> the default program a bare 'P' invokes for the rest of the session >>>> until another specific command overrides. Another usability hack >>>> may be "[interactive] pipecommand = less -FX" configuration variable >>>> gives the initial default for each session. >>> I think that would be way too complicated. >> It is modelled after how "less" and "vi" remembers the last pattern >> fed to their "/" command. You once give, say, "/test_<ENTER>" to >> find one instance of "test_", then "/<ENTER>" takes to the next >> instance. > > Huh, less(1) actually remembers nothing when the secure mode is > turned on. Are you sure you read me right? I wasn't talking about storing anything on disk for the "usability hack", and made it explicitly clear with "for the rest of the session".
On 2024-05-24 01:54, Junio C Hamano wrote: > Dragan Simic <dsimic@manjaro.org> writes: > >> On 2024-05-24 01:04, Junio C Hamano wrote: >>> Dragan Simic <dsimic@manjaro.org> writes: >>> >>>>> And for usability, perhaps giving a specific command would change >>>>> the default program a bare 'P' invokes for the rest of the session >>>>> until another specific command overrides. Another usability hack >>>>> may be "[interactive] pipecommand = less -FX" configuration >>>>> variable >>>>> gives the initial default for each session. >>>> I think that would be way too complicated. >>> >>> It is modelled after how "less" and "vi" remembers the last pattern >>> fed to their "/" command. You once give, say, "/test_<ENTER>" to >>> find one instance of "test_", then "/<ENTER>" takes to the next >>> instance. >> >> Huh, less(1) actually remembers nothing when the secure mode is >> turned on. > > Are you sure you read me right? I wasn't talking about storing > anything on disk for the "usability hack", and made it explicitly > clear with "for the rest of the session". I misread it at first, but I corrected myself. [1] [1] https://lore.kernel.org/git/3391a15907a055c281106c4995fb8272@manjaro.org/
On Thu, May 23, 2024 at 07:00:47AM -0700, Junio C Hamano wrote: > Jeff King <peff@peff.net> writes: > > > I do still think it > > would be useful to be able to configure its pager separately (in my > > case, I'd use "less -FX" rather than my default setup, which doesn't use > > either of those options). > > Even better. Allow to optionally have the command after the option, > e.g., > > (1/1) Use this hunk [y,n,q,j,k,e,p,P] P<RET> > (1/1) Use this hunk [y,n,q,j,k,e,p,P] Pless -FX<RET> > (1/1) Use this hunk [y,n,q,j,k,e,p,P] Pcat<RET> > > The first one feeds the default program with the hunk via pipe, the > second one instead invokes command you specifed, "less -FX", and > feeds the hunk to it via a pipe. The last one emulates a plain 'p' > behaviour. > > And for usability, perhaps giving a specific command would change > the default program a bare 'P' invokes for the rest of the session > until another specific command overrides. Another usability hack > may be "[interactive] pipecommand = less -FX" configuration variable > gives the initial default for each session. > > At that point, we can explain it as > > p - print the current hunk > P[<program>] - pipe the current hunk to a program > > or even use '|' instead of 'P'. Ooh, I like all of that (including "|", which is what triggers the similar feature in mutt). If interactive.pipeCommand defaults to the usual pager, then a bare "|" would do what most people want. -Peff
diff --git a/add-patch.c b/add-patch.c index 2252895c28..cefa3941a3 100644 --- a/add-patch.c +++ b/add-patch.c @@ -5,6 +5,7 @@ #include "environment.h" #include "gettext.h" #include "object-name.h" +#include "pager.h" #include "read-cache-ll.h" #include "repository.h" #include "strbuf.h" @@ -1448,9 +1449,11 @@ static int patch_update_file(struct add_p_state *s, strbuf_reset(&s->buf); if (file_diff->hunk_nr) { if (rendered_hunk_index != hunk_index) { + setup_pager(); render_hunk(s, hunk, 0, colored, &s->buf); fputs(s->buf.buf, stdout); rendered_hunk_index = hunk_index; + wait_for_pager(); } strbuf_reset(&s->buf); diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 52d7830de2..6c4af8904e 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -558,6 +558,27 @@ test_expect_success 'print again the hunk' ' test_cmp expect actual.trimmed ' +test_expect_success TTY 'print again the hunk (PAGER)' ' + test_when_finished "git reset" && + cat >expect <<-EOF && + PAGER <GREEN>+<RESET><GREEN>15<RESET> + PAGER 20<RESET> + <BOLD;BLUE>(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? <RESET>PAGER <CYAN>@@ -1,2 +1,3 @@<RESET> + PAGER 10<RESET> + PAGER <GREEN>+<RESET><GREEN>15<RESET> + PAGER 20<RESET> + <BOLD;BLUE>(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? <RESET> + EOF + test_write_lines s y g 1 p | + ( + GIT_PAGER="sed s/^/PAGER\ /" && + export GIT_PAGER && + test_terminal --no-stdin-pty git add -p >actual + ) && + tail -n 7 <actual | test_decode_color >actual.trimmed && + test_cmp expect actual.trimmed +' + test_expect_success 'navigate to hunk via regex' ' test_when_finished "git reset" && tr _ " " >expect <<-EOF &&
Invoke the pager when displaying hunks during "add -p" sessions, to make it easier for the user to review hunks longer than one screen height. Signed-off-by: Rubén Justo <rjusto@gmail.com> --- add-patch.c | 3 +++ t/t3701-add-interactive.sh | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+)