git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Phillip Wood <phillip.wood@dunelm.org.uk>,
	Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: [PATCH 2/2] add -p: fix checking of user input
Date: Mon, 17 Aug 2020 13:23:08 +0000	[thread overview]
Message-ID: <752e13fb9fd9fd7930d83a0915dbbc0274a99908.1597670589.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.702.git.1597670589.gitgitgadget@gmail.com>

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>
---
 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

  parent reply	other threads:[~2020-08-17 13:23 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-17 13:23 [PATCH 0/2] add p in C tweaks Phillip Wood via GitGitGadget
2020-08-17 13:23 ` [PATCH 1/2] add -p: use ALLOC_GROW_BY instead of ALLOW_GROW Phillip Wood via GitGitGadget
2020-08-17 18:35   ` Junio C Hamano
2020-08-17 13:23 ` Phillip Wood via GitGitGadget [this message]
2020-08-18  6:38   ` [PATCH 2/2] add -p: fix checking of user input Johannes Schindelin
2020-08-18  6:39 ` [PATCH 0/2] add p in C tweaks Johannes Schindelin
2020-08-18 19:44 ` Junio C Hamano
2020-08-19  9:59   ` Phillip Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=752e13fb9fd9fd7930d83a0915dbbc0274a99908.1597670589.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).