Rubén Justo <rjusto@xxxxxxxxx> writes:
> @@ -1389,6 +1390,7 @@ N_("j - leave this hunk undecided, see next undecided hunk\n"
> "s - split the current hunk into smaller hunks\n"
> "e - manually edit the current hunk\n"
> "p - print the current hunk\n"
> + "| - use pager to show the current hunk, or use |<program> to customize\n"
> "? - print help\n");
"to customize" strongly hints that the customization will stick, at
least during this session. Is that what actually happens?
> @@ -1401,6 +1403,7 @@ static int patch_update_file(struct add_p_state *s,
> struct child_process cp = CHILD_PROCESS_INIT;
> int colored = !!s->colored.len, quit = 0;
> enum prompt_mode_type prompt_mode_type;
> + const char* pager = NULL;
The asterisk sticks to "pager", not its type.
> enum {
> ALLOW_GOTO_PREVIOUS_HUNK = 1 << 0,
> ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK = 1 << 1,
> @@ -1449,9 +1452,15 @@ 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 (pager)
> + setup_custom_pager(pager);
> render_hunk(s, hunk, 0, colored, &s->buf);
> fputs(s->buf.buf, stdout);
> rendered_hunk_index = hunk_index;
> + if (pager) {
> + wait_for_pager();
> + pager = NULL;
> + }
> }
>
> strbuf_reset(&s->buf);
> @@ -1485,6 +1494,7 @@ static int patch_update_file(struct add_p_state *s,
> strbuf_addstr(&s->buf, ",e");
> }
> strbuf_addstr(&s->buf, ",p");
> + strbuf_addstr(&s->buf, ",|");
> }
> if (file_diff->deleted)
> prompt_mode_type = PROMPT_DELETION;
> @@ -1512,8 +1522,8 @@ static int patch_update_file(struct add_p_state *s,
> continue;
> ch = tolower(s->answer.buf[0]);
>
> - /* 'g' takes a hunk number and '/' takes a regexp */
> - if (s->answer.len != 1 && (ch != 'g' && ch != '/')) {
> + /* 'g' takes a hunk number, '/' takes a regexp and '|' takes a program */
> + if (s->answer.len != 1 && (ch != 'g' && ch != '/' && ch != '|')) {
Not limited to this instance, but a good discipline is to stop and
think twice before adding the third thing to already existing two.
Perhaps
/*
* 'g' takes a hunk number to go to.
* '/' takes a regexp to match.
* '|' takes a program to pipe to.
*/
if (s->answer.len != 1 && !strchr("g/|", ch))
> @@ -1674,6 +1684,17 @@ static int patch_update_file(struct add_p_state *s,
> }
> } else if (s->answer.buf[0] == 'p') {
> rendered_hunk_index = -1;
> + } else if (ch == '|') {
> + strbuf_remove(&s->answer, 0, 1);
> + if (s->s.use_single_key && s->answer.len == 0) {
If you check .use_single_key, you do not need to check answer.len,
do you? Can it ever be anything other than 0 here in the single-key
mode?
> + printf("%s", _("program? "));
> + fflush(stdout);
> + strbuf_getline(&s->answer, stdin);
> + strbuf_trim_trailing_newline(&s->answer);
> + }
> + strbuf_trim(&s->answer);
> + pager = s->answer.buf;
Is it safe to peek into s->answer.buf and expect it to be live until
we have to use the pager like this?
By the way, it should be trivial to make the "custom" pager more sticky.
} else if (ch == '|') {
if (s->s.use_single_key) {
... read into s->answer ...
} else {
strbuf_remove(&s->answer, 0, 1);
}
strbuf_trim_trailing_newline(&s->answer);
/*
* If it is completely empty, use the last
* one, if it is semi-empty, reset to the default.
*/
if (!s->answer.len) {
;
} else {
FREE_AND_NULL(pager);
strbuf_trim(&s->answer);
if (!s->answer.len)
pager = xstrdup(s->answer.buf);
}
Even better, we can lift the scope of "pager" one level up, define
it as an on-stack variable in run_add_p(), add a new parameter of
type "const char **" to patch_update_file(), and use it throughout
patch_update_file(), so that a "custom" pager set here will be
carried across file boundaries.
> + rendered_hunk_index = -1;
> } 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 6f6d174687..7b3ebb671d 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -64,8 +64,8 @@ test_expect_success 'unknown command' '
> git add -N command &&
> git diff command >expect &&
> cat >>expect <<-EOF &&
> - (1/1) Stage addition [y,n,q,a,d,e,p,?]? Unknown command ${SQ}W${SQ} (use ${SQ}?${SQ} for help)
> - (1/1) Stage addition [y,n,q,a,d,e,p,?]?$SP
> + (1/1) Stage addition [y,n,q,a,d,e,p,|,?]? Unknown command ${SQ}W${SQ} (use ${SQ}?${SQ} for help)
> + (1/1) Stage addition [y,n,q,a,d,e,p,|,?]?$SP
> EOF
> git add -p -- command <command >actual 2>&1 &&
> test_cmp expect actual
> @@ -348,9 +348,9 @@ test_expect_success 'different prompts for mode change/deleted' '
> git -c core.filemode=true add -p >actual &&
> sed -n "s/^\(([0-9/]*) Stage .*?\).*/\1/p" actual >actual.filtered &&
> cat >expect <<-\EOF &&
> - (1/1) Stage deletion [y,n,q,a,d,p,?]?
> - (1/2) Stage mode change [y,n,q,a,d,j,J,g,/,p,?]?
> - (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]?
> + (1/1) Stage deletion [y,n,q,a,d,p,|,?]?
> + (1/2) Stage mode change [y,n,q,a,d,j,J,g,/,p,|,?]?
> + (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,|,?]?
> EOF
> test_cmp expect actual.filtered
> '
> @@ -537,13 +537,13 @@ test_expect_success 'split hunk setup' '
> test_expect_success 'goto hunk 1 with "g 1"' '
> test_when_finished "git reset" &&
> tr _ " " >expect <<-EOF &&
> - (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]? + 1: -1,2 +1,3 +15
> + (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,|,?]? + 1: -1,2 +1,3 +15
> _ 2: -2,4 +3,8 +21
> go to which hunk? @@ -1,2 +1,3 @@
> _10
> +15
> _20
> - (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?_
> + (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,|,?]?_
> EOF
> test_write_lines s y g 1 | git add -p >actual &&
> tail -n 7 <actual >actual.trimmed &&
> @@ -556,7 +556,7 @@ test_expect_success 'goto hunk 1 with "g1"' '
> _10
> +15
> _20
> - (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?_
> + (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,|,?]?_
> EOF
> test_write_lines s y g1 | git add -p >actual &&
> tail -n 4 <actual >actual.trimmed &&
> @@ -566,11 +566,11 @@ test_expect_success 'goto hunk 1 with "g1"' '
> test_expect_success 'navigate to hunk via regex /pattern' '
> test_when_finished "git reset" &&
> tr _ " " >expect <<-EOF &&
> - (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]? @@ -1,2 +1,3 @@
> + (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,|,?]? @@ -1,2 +1,3 @@
> _10
> +15
> _20
> - (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?_
> + (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,|,?]?_
> EOF
> test_write_lines s y /1,2 | git add -p >actual &&
> tail -n 5 <actual >actual.trimmed &&
> @@ -583,7 +583,7 @@ test_expect_success 'navigate to hunk via regex / pattern' '
> _10
> +15
> _20
> - (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?_
> + (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,|,?]?_
> EOF
> test_write_lines s y / 1,2 | git add -p >actual &&
> tail -n 4 <actual >actual.trimmed &&
> @@ -595,17 +595,38 @@ test_expect_success 'print again the hunk' '
> tr _ " " >expect <<-EOF &&
> +15
> 20
> - (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? @@ -1,2 +1,3 @@
> + (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,|,?]? @@ -1,2 +1,3 @@
> 10
> +15
> 20
> - (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?_
> + (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,|,?]?_
> EOF
> test_write_lines s y g 1 p | git add -p >actual &&
> tail -n 7 <actual >actual.trimmed &&
> 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 \| |
> + (
> + 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 'split hunk "add -p (edit)"' '
> # Split, say Edit and do nothing. Then:
> #
> @@ -780,21 +801,21 @@ test_expect_success 'colors can be overridden' '
> <BLUE>+<RESET><BLUE>new<RESET>
> <CYAN> more-context<RESET>
> <BLUE>+<RESET><BLUE>another-one<RESET>
> - <YELLOW>(1/1) Stage this hunk [y,n,q,a,d,s,e,p,?]? <RESET><BOLD>Split into 2 hunks.<RESET>
> + <YELLOW>(1/1) Stage this hunk [y,n,q,a,d,s,e,p,|,?]? <RESET><BOLD>Split into 2 hunks.<RESET>
> <MAGENTA>@@ -1,3 +1,3 @@<RESET>
> <CYAN> context<RESET>
> <BOLD>-old<RESET>
> <BLUE>+<RESET><BLUE>new<RESET>
> <CYAN> more-context<RESET>
> - <YELLOW>(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? <RESET><MAGENTA>@@ -3 +3,2 @@<RESET>
> + <YELLOW>(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,|,?]? <RESET><MAGENTA>@@ -3 +3,2 @@<RESET>
> <CYAN> more-context<RESET>
> <BLUE>+<RESET><BLUE>another-one<RESET>
> - <YELLOW>(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]? <RESET><MAGENTA>@@ -1,3 +1,3 @@<RESET>
> + <YELLOW>(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,|,?]? <RESET><MAGENTA>@@ -1,3 +1,3 @@<RESET>
> <CYAN> context<RESET>
> <BOLD>-old<RESET>
> <BLUE>+new<RESET>
> <CYAN> more-context<RESET>
> - <YELLOW>(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? <RESET>
> + <YELLOW>(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,|,?]? <RESET>
> EOF
> test_cmp expect actual
> '
