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

[Ramkumar Ramachandra <artagnon@gmail.com>]

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 sequencer
--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 sequencer
--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@pobox.com>
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 sequencer.c |   45 +++++++++++++++++++++++++++++++++------------
 1 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 02e58ad..cb93ee8 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -513,7 +513,22 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
 	return 0;
 }
 
-static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *opts)
+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);
+}
+
+int parse_insn_line(char *bol, char *eol, struct replay_opts *opts,
+		struct commit_list *list, int lineno)
 {
 	unsigned char commit_sha1[20];
 	enum replay_action action;
@@ -526,7 +541,8 @@ static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *
 		action = REPLAY_REVERT;
 		bol += strlen("revert");
 	} else
-		return NULL;
+		return parse_error(_("unrecognized action"),
+				git_path(SEQ_TODO_FILE), lineno, bol);
 
 	/* Eat up extra spaces/ tabs before object name */
 	bol += strspn(bol, " \t");
@@ -538,32 +554,36 @@ static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *
 	if (action != opts->action) {
 		const char *action_str;
 		action_str = action == REPLAY_REVERT ? "revert" : "cherry-pick";
-		error(_("Cannot %s during a %s"), action_str, action_name(opts));
-		return NULL;
+		return parse_error(_("invalid action"),
+				git_path(SEQ_TODO_FILE), lineno, bol);
 	}
 
 	namelen = strcspn(bol, " \t\n");
 	if (getn_sha1(bol, namelen, commit_sha1))
-		return NULL;
-
-	return lookup_commit_reference(commit_sha1);
+		return parse_error(_("malformed object name"),
+				git_path(SEQ_TODO_FILE), lineno, bol);
+
+	list->item = lookup_commit_reference(commit_sha1);
+	if (!list->item)
+		return parse_error(_("not a valid commit"),
+				git_path(SEQ_TODO_FILE), lineno, bol);
+	list->next = NULL;
+	return 0;
 }
 
 static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
 			struct replay_opts *opts)
 {
 	struct commit_list **next = todo_list;
-	struct commit *commit;
+	struct commit_list list;
 	char *p = buf;
 	int i;
 
 	for (i = 1; *p; i++) {
 		char *eol = strchrnul(p, '\n');
-		commit = parse_insn_line(p, eol, opts);
-		if (!commit)
-			return error(_("Could not parse line %d."), i);
-		next = commit_list_append(commit, next);
+		if (parse_insn_line(p, eol, opts, &list, i))
+			return -1;
+		next = commit_list_append(list.item, next);
 		p = *eol ? eol + 1 : eol;
 	}
 	if (!*todo_list)
-- 
1.7.8.1.362.g5d6df.dirty