| Message ID | fa64a975-40e4-40f2-bdcf-fd2da4fc506e@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | improve interactive-patch | expand |
Rubén Justo <rjusto@gmail.com> writes: > Shortly we're going make interactive-patch stop printing automatically > the hunk under certain circumstances. > > Let's introduce a new option to allow the user to explicitly request > the printing. That is good, but ... > Signed-off-by: Rubén Justo <rjusto@gmail.com> > --- > add-patch.c | 4 ++++ > t/t3701-add-interactive.sh | 22 +++++++++++----------- > 2 files changed, 15 insertions(+), 11 deletions(-) > > diff --git a/add-patch.c b/add-patch.c > index 68f525b35c..52be1ddb15 100644 > --- a/add-patch.c > +++ b/add-patch.c > @@ -1388,6 +1388,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 again the current hunk\n" > "? - print help\n"); > > static int patch_update_file(struct add_p_state *s, > @@ -1480,6 +1481,7 @@ static int patch_update_file(struct add_p_state *s, > permitted |= ALLOW_EDIT; > strbuf_addstr(&s->buf, ",e"); > } > + strbuf_addstr(&s->buf, ",p"); > } > if (file_diff->deleted) > prompt_mode_type = PROMPT_DELETION; > @@ -1658,6 +1660,8 @@ static int patch_update_file(struct add_p_state *s, > hunk->use = USE_HUNK; > goto soft_increment; > } > + } else if (s->answer.buf[0] == 'p') { > + /* nothing to do */ This is not good. If we are taking a new input, why doesn't the code already respond to it? "Showing it again" should be a separate feature even if some other codepaths still do show when [2/2] would prevent them to show, no? Also, in addition to the changes to the patch to unbreak it, we'd need to update the git-add(1) manual page, especially its section on the interactive mode. I think a single-line addition should suffice, but it has to be there, added by the same patch as the one that starts accepting 'p' and acting on that input. Thanks. > diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh > index 0b5339ac6c..bc55255b0a 100755 > --- a/t/t3701-add-interactive.sh > +++ b/t/t3701-add-interactive.sh > @@ -325,9 +325,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,?]? > - (1/2) Stage mode change [y,n,q,a,d,j,J,g,/,?]? > - (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,?]? > + (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 > ' > @@ -514,13 +514,13 @@ test_expect_success 'split hunk setup' ' > test_expect_success 'goto hunk' ' > test_when_finished "git reset" && > tr _ " " >expect <<-EOF && > - (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,?]? + 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,?]?_ > + (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 && > @@ -530,11 +530,11 @@ test_expect_success 'goto hunk' ' > test_expect_success 'navigate to hunk via regex' ' > test_when_finished "git reset" && > tr _ " " >expect <<-EOF && > - (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,?]? @@ -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,?]?_ > + (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 && > @@ -715,21 +715,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,?]? <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,?]? <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,?]? <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,?]? <RESET> > + <YELLOW>(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? <RESET> > EOF > test_cmp expect actual > '
On Mon, Mar 25, 2024 at 02:38:30PM -0700, Junio C Hamano wrote: > Rubén Justo <rjusto@gmail.com> writes: > > > Shortly we're going make interactive-patch stop printing automatically > > the hunk under certain circumstances. > > > > Let's introduce a new option to allow the user to explicitly request > > the printing. > > That is good, but ... > > > Signed-off-by: Rubén Justo <rjusto@gmail.com> > > --- > > add-patch.c | 4 ++++ > > t/t3701-add-interactive.sh | 22 +++++++++++----------- > > 2 files changed, 15 insertions(+), 11 deletions(-) > > > > diff --git a/add-patch.c b/add-patch.c > > index 68f525b35c..52be1ddb15 100644 > > --- a/add-patch.c > > +++ b/add-patch.c > > @@ -1388,6 +1388,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 again the current hunk\n" > > "? - print help\n"); > > > > static int patch_update_file(struct add_p_state *s, > > @@ -1480,6 +1481,7 @@ static int patch_update_file(struct add_p_state *s, > > permitted |= ALLOW_EDIT; > > strbuf_addstr(&s->buf, ",e"); > > } > > + strbuf_addstr(&s->buf, ",p"); > > } > > if (file_diff->deleted) > > prompt_mode_type = PROMPT_DELETION; > > @@ -1658,6 +1660,8 @@ static int patch_update_file(struct add_p_state *s, > > hunk->use = USE_HUNK; > > goto soft_increment; > > } > > + } else if (s->answer.buf[0] == 'p') { > > + /* nothing to do */ > > This is not good. If we are taking a new input, why doesn't the > code already respond to it? "Showing it again" should be a separate > feature even if some other codepaths still do show when [2/2] would > prevent them to show, no? Doing nothing here produces, in the current implementation, the intended printing. Maybe the message needs to state so? > Also, in addition to the changes to the patch to unbreak it, we'd > need to update the git-add(1) manual page, especially its section on > the interactive mode. I think a single-line addition should suffice, > but it has to be there, added by the same patch as the one that > starts accepting 'p' and acting on that input. Oh, of course. I think this is the line needed: diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt index 14a371fff3..90b47927b2 100644 --- a/Documentation/git-add.txt +++ b/Documentation/git-add.txt @@ -348,6 +348,7 @@ patch:: K - leave this hunk undecided, see previous hunk s - split the current hunk into smaller hunks e - manually edit the current hunk + p - print again the current hunk ? - print help + After deciding the fate for all hunks, if there is any hunk
Rubén Justo <rjusto@gmail.com> writes: > Doing nothing here produces, in the current implementation, the intended > printing. Maybe the message needs to state so? Yeah, "/* Nothing special is needed */", or something. Thanks for a quick response.
diff --git a/add-patch.c b/add-patch.c index 68f525b35c..52be1ddb15 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1388,6 +1388,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 again the current hunk\n" "? - print help\n"); static int patch_update_file(struct add_p_state *s, @@ -1480,6 +1481,7 @@ static int patch_update_file(struct add_p_state *s, permitted |= ALLOW_EDIT; strbuf_addstr(&s->buf, ",e"); } + strbuf_addstr(&s->buf, ",p"); } if (file_diff->deleted) prompt_mode_type = PROMPT_DELETION; @@ -1658,6 +1660,8 @@ static int patch_update_file(struct add_p_state *s, hunk->use = USE_HUNK; goto soft_increment; } + } else if (s->answer.buf[0] == 'p') { + /* nothing to do */ } else { const char *p = _(help_patch_remainder), *eol = p; diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 0b5339ac6c..bc55255b0a 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -325,9 +325,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,?]? - (1/2) Stage mode change [y,n,q,a,d,j,J,g,/,?]? - (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,?]? + (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 ' @@ -514,13 +514,13 @@ test_expect_success 'split hunk setup' ' test_expect_success 'goto hunk' ' test_when_finished "git reset" && tr _ " " >expect <<-EOF && - (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,?]? + 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,?]?_ + (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 && @@ -530,11 +530,11 @@ test_expect_success 'goto hunk' ' test_expect_success 'navigate to hunk via regex' ' test_when_finished "git reset" && tr _ " " >expect <<-EOF && - (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,?]? @@ -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,?]?_ + (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 && @@ -715,21 +715,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,?]? <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,?]? <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,?]? <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,?]? <RESET> + <YELLOW>(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? <RESET> EOF test_cmp expect actual '
Shortly we're going make interactive-patch stop printing automatically the hunk under certain circumstances. Let's introduce a new option to allow the user to explicitly request the printing. Signed-off-by: Rubén Justo <rjusto@gmail.com> --- add-patch.c | 4 ++++ t/t3701-add-interactive.sh | 22 +++++++++++----------- 2 files changed, 15 insertions(+), 11 deletions(-)