add-patch: render hunks through the pager

Make the print command trigger the pager when invoked using a capital
'P', to make it easier for the user to review long hunks.

Note that if the PAGER ends unexpectedly before we've been able to send
the payload, perhaps because the user is not interested in the whole
thing, we might receive a SIGPIPE, which would abruptly and unexpectedly
terminate the interactive session for the user.

Therefore, we need to ignore a possible SIGPIPE signal.  Add a test for
this, in addition to the test for normal operation.

For the SIGPIPE test, we need to make sure that we completely fill the
operating system's buffer, otherwise we might not trigger the SIGPIPE
signal.  The normal size of this buffer in different OSs varies from a
few KBs to 1MB.  Use a payload large enough to guarantee that we exceed
this limit.

Signed-off-by: Rubén Justo <rjusto@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/add-patch.c b/add-patch.c
index 6e176cd21a062fcaf064fc592046d11bb86f41db..f2c76b7d83280aca436398346a59c37d520d8b97 100644 (file)
--- a/add-patch.c
+++ b/add-patch.c
@@ -7,9 +7,11 @@
 #include "environment.h"
 #include "gettext.h"
 #include "object-name.h"
+#include "pager.h"
 #include "read-cache-ll.h"
 #include "repository.h"
 #include "strbuf.h"
+#include "sigchain.h"
 #include "run-command.h"
 #include "strvec.h"
 #include "pathspec.h"
@@ -1391,7 +1393,7 @@ N_("j - leave this hunk undecided, see next undecided hunk\n"
    "/ - search for a hunk matching the given regex\n"
    "s - split the current hunk into smaller hunks\n"
    "e - manually edit the current hunk\n"
-   "p - print the current hunk\n"
+   "p - print the current hunk, 'P' to use the pager\n"
    "? - print help\n");
 
 static int patch_update_file(struct add_p_state *s,
@@ -1402,7 +1404,7 @@ static int patch_update_file(struct add_p_state *s,
        struct hunk *hunk;
        char ch;
        struct child_process cp = CHILD_PROCESS_INIT;
-       int colored = !!s->colored.len, quit = 0;
+       int colored = !!s->colored.len, quit = 0, use_pager = 0;
        enum prompt_mode_type prompt_mode_type;
        enum {
                ALLOW_GOTO_PREVIOUS_HUNK = 1 << 0,
@@ -1452,9 +1454,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) {
+                               if (use_pager) {
+                                       setup_pager();
+                                       sigchain_push(SIGPIPE, SIG_IGN);
+                               }
                                render_hunk(s, hunk, 0, colored, &s->buf);
                                fputs(s->buf.buf, stdout);
                                rendered_hunk_index = hunk_index;
+                               if (use_pager) {
+                                       sigchain_pop(SIGPIPE);
+                                       wait_for_pager();
+                                       use_pager = 0;
+                               }
                        }
 
                        strbuf_reset(&s->buf);
@@ -1675,8 +1686,9 @@ soft_increment:
                                hunk->use = USE_HUNK;
                                goto soft_increment;
                        }
-               } else if (s->answer.buf[0] == 'p') {
+               } else if (ch == 'p') {
                        rendered_hunk_index = -1;
+                       use_pager = (s->answer.buf[0] == 'P') ? 1 : 0;
                } else if (s->answer.buf[0] == '?') {
                        const char *p = _(help_patch_remainder), *eol = p;
 

diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 6daf3a6be0ab262eb3955b48a563cdca934e7ddf..1b8617e0c1f2c3e075250ec877b41138e638c1f1 100755 (executable)
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -591,6 +591,38 @@ 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 &&
+       <GREEN>+<RESET><GREEN>15<RESET>
+        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 git add -p >actual
+       ) &&
+       tail -n 7 <actual | test_decode_color >actual.trimmed &&
+       test_cmp expect actual.trimmed
+'
+
+test_expect_success TTY 'P handles SIGPIPE when writing to pager' '
+       test_when_finished "rm -f huge_file; git reset" &&
+       printf "\n%2500000s" Y >huge_file &&
+       git add -N huge_file &&
+       test_write_lines P q | (
+               GIT_PAGER="head -n 1" &&
+               export GIT_PAGER &&
+               test_terminal git add -p
+       )
+'
+
 test_expect_success 'split hunk "add -p (edit)"' '
        # Split, say Edit and do nothing.  Then:
        #