[PATCH 5/8] revert: report fine-grained errors from insn parser
From: Ramkumar Ramachandra <hidden>
Date: 2016-06-15 22:52:45
Subsystem:
the rest · Maintainer:
Linus Torvalds
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 [off-list ref] Helped-by: Jonathan Nieder [off-list ref] Signed-off-by: Ramkumar Ramachandra <redacted> --- 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