| Message ID | 752e13fb9fd9fd7930d83a0915dbbc0274a99908.1597670589.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | ce910287e727e5b534d806df725c68a8dfc4790a |
| Headers | show |
| Series | add p in C tweaks | expand |
Hi Phillip, On Mon, 17 Aug 2020, Phillip Wood via GitGitGadget wrote: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > When a file has been deleted the C version of add -p allows the user > to edit a hunk even though 'e' is not in the list of allowed > responses. (I think 'e' is disallowed because if the file is edited it > is no longer a deletion and we're not set up to rewrite the diff > header). > > The invalid response was allowed because the test that determines > whether to display 'e' was not duplicated correctly in the code that > processes the user's choice. Fix this by using flags that are set when > constructing the prompt and checked when processing the user's choice > rather than repeating the check itself. > > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> As I had mentioned on the PR, I would much rather have a bug-for-bug conversion to use the `enum`, and the fix for the `edit` case as a separate patch on top. Thank you, Dscho > --- > add-patch.c | 54 +++++++++++++++++++++++++++++++++++++---------------- > 1 file changed, 38 insertions(+), 16 deletions(-) > > diff --git a/add-patch.c b/add-patch.c > index a15fa407be..907c05b3c1 100644 > --- a/add-patch.c > +++ b/add-patch.c > @@ -1352,6 +1352,15 @@ 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; > + enum { > + ALLOW_GOTO_PREVIOUS_HUNK = 1 << 0, > + ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK = 1 << 1, > + ALLOW_GOTO_NEXT_HUNK = 1 << 2, > + ALLOW_GOTO_NEXT_UNDECIDED_HUNK = 1 << 3, > + ALLOW_SEARCH_AND_GOTO = 1 << 4, > + ALLOW_SPLIT = 1 << 5, > + ALLOW_EDIT = 1 << 6 > + } permitted = 0; > > if (!file_diff->hunk_nr) > return 0; > @@ -1388,22 +1397,35 @@ static int patch_update_file(struct add_p_state *s, > fputs(s->buf.buf, stdout); > > strbuf_reset(&s->buf); > - if (undecided_previous >= 0) > + if (undecided_previous >= 0) { > + permitted |= ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK; > strbuf_addstr(&s->buf, ",k"); > - if (hunk_index) > + } > + if (hunk_index) { > + permitted |= ALLOW_GOTO_PREVIOUS_HUNK; > strbuf_addstr(&s->buf, ",K"); > - if (undecided_next >= 0) > + } > + if (undecided_next >= 0) { > + permitted |= ALLOW_GOTO_NEXT_UNDECIDED_HUNK; > strbuf_addstr(&s->buf, ",j"); > - if (hunk_index + 1 < file_diff->hunk_nr) > + } > + if (hunk_index + 1 < file_diff->hunk_nr) { > + permitted |= ALLOW_GOTO_NEXT_HUNK; > strbuf_addstr(&s->buf, ",J"); > - if (file_diff->hunk_nr > 1) > + } > + if (file_diff->hunk_nr > 1) { > + permitted |= ALLOW_SEARCH_AND_GOTO; > strbuf_addstr(&s->buf, ",g,/"); > - if (hunk->splittable_into > 1) > + } > + if (hunk->splittable_into > 1) { > + permitted |= ALLOW_SPLIT; > strbuf_addstr(&s->buf, ",s"); > + } > if (hunk_index + 1 > file_diff->mode_change && > - !file_diff->deleted) > + !file_diff->deleted) { > + permitted |= ALLOW_EDIT; > strbuf_addstr(&s->buf, ",e"); > - > + } > if (file_diff->deleted) > prompt_mode_type = PROMPT_DELETION; > else if (file_diff->added) > @@ -1452,22 +1474,22 @@ static int patch_update_file(struct add_p_state *s, > break; > } > } else if (s->answer.buf[0] == 'K') { > - if (hunk_index) > + if (permitted & ALLOW_GOTO_PREVIOUS_HUNK) > hunk_index--; > else > err(s, _("No previous hunk")); > } else if (s->answer.buf[0] == 'J') { > - if (hunk_index + 1 < file_diff->hunk_nr) > + if (permitted & ALLOW_GOTO_NEXT_HUNK) > hunk_index++; > else > err(s, _("No next hunk")); > } else if (s->answer.buf[0] == 'k') { > - if (undecided_previous >= 0) > + if (permitted & ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK) > hunk_index = undecided_previous; > else > err(s, _("No previous hunk")); > } else if (s->answer.buf[0] == 'j') { > - if (undecided_next >= 0) > + if (permitted & ALLOW_GOTO_NEXT_UNDECIDED_HUNK) > hunk_index = undecided_next; > else > err(s, _("No next hunk")); > @@ -1475,7 +1497,7 @@ static int patch_update_file(struct add_p_state *s, > char *pend; > unsigned long response; > > - if (file_diff->hunk_nr < 2) { > + if (!(permitted & ALLOW_SEARCH_AND_GOTO)) { > err(s, _("No other hunks to goto")); > continue; > } > @@ -1512,7 +1534,7 @@ static int patch_update_file(struct add_p_state *s, > regex_t regex; > int ret; > > - if (file_diff->hunk_nr < 2) { > + if (!(permitted & ALLOW_SEARCH_AND_GOTO)) { > err(s, _("No other hunks to search")); > continue; > } > @@ -1557,7 +1579,7 @@ static int patch_update_file(struct add_p_state *s, > hunk_index = i; > } else if (s->answer.buf[0] == 's') { > size_t splittable_into = hunk->splittable_into; > - if (splittable_into < 2) > + if (!(permitted & ALLOW_SPLIT)) > err(s, _("Sorry, cannot split this hunk")); > else if (!split_hunk(s, file_diff, > hunk - file_diff->hunk)) > @@ -1565,7 +1587,7 @@ static int patch_update_file(struct add_p_state *s, > _("Split into %d hunks."), > (int)splittable_into); > } else if (s->answer.buf[0] == 'e') { > - if (hunk_index + 1 == file_diff->mode_change) > + if (!(permitted & ALLOW_EDIT)) > err(s, _("Sorry, cannot edit this hunk")); > else if (edit_hunk_loop(s, file_diff, hunk) >= 0) { > hunk->use = USE_HUNK; > -- > gitgitgadget >
diff --git a/add-patch.c b/add-patch.c index a15fa407be..907c05b3c1 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1352,6 +1352,15 @@ 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; + enum { + ALLOW_GOTO_PREVIOUS_HUNK = 1 << 0, + ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK = 1 << 1, + ALLOW_GOTO_NEXT_HUNK = 1 << 2, + ALLOW_GOTO_NEXT_UNDECIDED_HUNK = 1 << 3, + ALLOW_SEARCH_AND_GOTO = 1 << 4, + ALLOW_SPLIT = 1 << 5, + ALLOW_EDIT = 1 << 6 + } permitted = 0; if (!file_diff->hunk_nr) return 0; @@ -1388,22 +1397,35 @@ static int patch_update_file(struct add_p_state *s, fputs(s->buf.buf, stdout); strbuf_reset(&s->buf); - if (undecided_previous >= 0) + if (undecided_previous >= 0) { + permitted |= ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK; strbuf_addstr(&s->buf, ",k"); - if (hunk_index) + } + if (hunk_index) { + permitted |= ALLOW_GOTO_PREVIOUS_HUNK; strbuf_addstr(&s->buf, ",K"); - if (undecided_next >= 0) + } + if (undecided_next >= 0) { + permitted |= ALLOW_GOTO_NEXT_UNDECIDED_HUNK; strbuf_addstr(&s->buf, ",j"); - if (hunk_index + 1 < file_diff->hunk_nr) + } + if (hunk_index + 1 < file_diff->hunk_nr) { + permitted |= ALLOW_GOTO_NEXT_HUNK; strbuf_addstr(&s->buf, ",J"); - if (file_diff->hunk_nr > 1) + } + if (file_diff->hunk_nr > 1) { + permitted |= ALLOW_SEARCH_AND_GOTO; strbuf_addstr(&s->buf, ",g,/"); - if (hunk->splittable_into > 1) + } + if (hunk->splittable_into > 1) { + permitted |= ALLOW_SPLIT; strbuf_addstr(&s->buf, ",s"); + } if (hunk_index + 1 > file_diff->mode_change && - !file_diff->deleted) + !file_diff->deleted) { + permitted |= ALLOW_EDIT; strbuf_addstr(&s->buf, ",e"); - + } if (file_diff->deleted) prompt_mode_type = PROMPT_DELETION; else if (file_diff->added) @@ -1452,22 +1474,22 @@ static int patch_update_file(struct add_p_state *s, break; } } else if (s->answer.buf[0] == 'K') { - if (hunk_index) + if (permitted & ALLOW_GOTO_PREVIOUS_HUNK) hunk_index--; else err(s, _("No previous hunk")); } else if (s->answer.buf[0] == 'J') { - if (hunk_index + 1 < file_diff->hunk_nr) + if (permitted & ALLOW_GOTO_NEXT_HUNK) hunk_index++; else err(s, _("No next hunk")); } else if (s->answer.buf[0] == 'k') { - if (undecided_previous >= 0) + if (permitted & ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK) hunk_index = undecided_previous; else err(s, _("No previous hunk")); } else if (s->answer.buf[0] == 'j') { - if (undecided_next >= 0) + if (permitted & ALLOW_GOTO_NEXT_UNDECIDED_HUNK) hunk_index = undecided_next; else err(s, _("No next hunk")); @@ -1475,7 +1497,7 @@ static int patch_update_file(struct add_p_state *s, char *pend; unsigned long response; - if (file_diff->hunk_nr < 2) { + if (!(permitted & ALLOW_SEARCH_AND_GOTO)) { err(s, _("No other hunks to goto")); continue; } @@ -1512,7 +1534,7 @@ static int patch_update_file(struct add_p_state *s, regex_t regex; int ret; - if (file_diff->hunk_nr < 2) { + if (!(permitted & ALLOW_SEARCH_AND_GOTO)) { err(s, _("No other hunks to search")); continue; } @@ -1557,7 +1579,7 @@ static int patch_update_file(struct add_p_state *s, hunk_index = i; } else if (s->answer.buf[0] == 's') { size_t splittable_into = hunk->splittable_into; - if (splittable_into < 2) + if (!(permitted & ALLOW_SPLIT)) err(s, _("Sorry, cannot split this hunk")); else if (!split_hunk(s, file_diff, hunk - file_diff->hunk)) @@ -1565,7 +1587,7 @@ static int patch_update_file(struct add_p_state *s, _("Split into %d hunks."), (int)splittable_into); } else if (s->answer.buf[0] == 'e') { - if (hunk_index + 1 == file_diff->mode_change) + if (!(permitted & ALLOW_EDIT)) err(s, _("Sorry, cannot edit this hunk")); else if (edit_hunk_loop(s, file_diff, hunk) >= 0) { hunk->use = USE_HUNK;