[PATCH 5/8] revert: report fine-grained errors from insn parser

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The infrastructure that parses '.git/sequencer/todo' is meant to
handle arbitrary user input some day, so it can be used as the
implementation of 'git rebase --interactive' and 'git sequence
--edit'.  It is currently sub-optimal for that purpose because the
parse error messages just say:

  error: Could not parse line 5.

This patch shifts responsibility to parse_insn_line(), which can come
up with a more detailed message like:

  error: .git/sequencer/todo:5: unrecognized action: frobnicate

Once the operator is allowed to edit the sequence, the message might
be adjusted to something like:

  error: <sequence you just gave me>:5: unrecognized action: frobnicate

instead of exposing an implementation detail. Some day "git sequence
--edit" could even re-launch the editor with an error message in a
comment before the problematic line and the cursor pointing there.
For now, pointing to the explicit filename is useful since this should
only happen if there was filesystem corruption, tampering, or a git
bug.

Helped-by: Junio C Hamano <gitster@xxxxxxxxx>
Helped-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
Signed-off-by: Ramkumar Ramachandra <artagnon@xxxxxxxxx>
---
 builtin/revert.c |   30 ++++++++++++++++++++++++------
 1 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 9a09471..0954d22 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -732,7 +732,22 @@ static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)
 	return 0;
 }
 
-static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
+static int parse_error(const char *message, const char *file,
+		int lineno, char *error_line)
+{
+	const char *suffix = "";
+	int error_len = strcspn(error_line, " \t\n");
+
+	if (error_len > 20) {
+		error_len = 20;
+		suffix = "...";
+	}
+	return error(_("%s:%d: %s: %.*s%s"), file, lineno, message,
+		error_len, error_line, suffix);
+}
+
+static int parse_insn_line(char *bol, char *eol,
+			struct replay_insn_list *item, int lineno)
 {
 	unsigned char commit_sha1[20];
 	char *end_of_object_name;
@@ -745,7 +760,8 @@ static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
 		item->action = REPLAY_REVERT;
 		bol += strlen("revert ");
 	} else
-		return -1;
+		return parse_error(_("unrecognized action"),
+				git_path(SEQ_TODO_FILE), lineno, bol);
 
 	/* Eat up extra spaces/ tabs before object name */
 	bol += strspn(bol, " \t");
@@ -757,11 +773,13 @@ static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
 	*end_of_object_name = saved;
 
 	if (status < 0)
-		return -1;
+		return parse_error(_("malformed object name"),
+				git_path(SEQ_TODO_FILE), lineno, bol);
 
 	item->operand = lookup_commit_reference(commit_sha1);
 	if (!item->operand)
-		return -1;
+		return parse_error(_("not a valid commit"),
+				git_path(SEQ_TODO_FILE), lineno, bol);
 
 	item->next = NULL;
 	return 0;
@@ -776,8 +794,8 @@ static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)
 
 	for (i = 1; *p; i++) {
 		char *eol = strchrnul(p, '\n');
-		if (parse_insn_line(p, eol, &item))
-			return error(_("Could not parse line %d."), i);
+		if (parse_insn_line(p, eol, &item, i))
+			return -1;
 		next = replay_insn_list_append(item.operand, item.action, next);
 		p = *eol ? eol + 1 : eol;
 	}
-- 
1.7.8.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]