Re: [PATCH 5/8] revert: report fine-grained errors from insn parser
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:52:46
Ramkumar Ramachandra wrote:
quoted hunk ↗ jump to hunk
--- 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);
Since the snippet used in an error message is a single word, why is it called error_line? (And why is the signature written in a way that implies we might modify it, by the way?) Missing /* TRANSLATORS: ... */ comment. [...]
quoted hunk ↗ jump to hunk
@@ -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);
This is the message I'll get if I misspell "master" as "mister" or try to cherry-pick HEAD~100000 when the history is not that deep. When I read "malformed object name", I'll look for syntax errors and be confused. (They are valid syntax denoting commits that just happen not to exist.)