All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ramkumar Ramachandra <artagnon@gmail.com>
To: Git List <git@vger.kernel.org>
Cc: Junio C Hamano <gitster@pobox.com>, Jonathan Nieder <jrnieder@gmail.com>
Subject: [PATCH 5/6] revert: report fine-grained error messages from insn parser
Date: Sun,  8 Jan 2012 17:57:32 +0530	[thread overview]
Message-ID: <1326025653-11922-6-git-send-email-artagnon@gmail.com> (raw)
In-Reply-To: <1326025653-11922-1-git-send-email-artagnon@gmail.com>

Three kinds of errors can arise from parsing '.git/sequencer/todo':
1. Unrecognized action
2. Malformed object name
3. Object name does not refer to a valid commit

Since we would like to make the instruction sheet user-editable in the
future (much like the 'rebase -i' sheet), report more fine-grained
parse errors prefixed with the filename and line number.

Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/revert.c |   45 +++++++++++++++++++++++++++------------------
 1 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 54ea394..35553e7 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -714,26 +714,29 @@ 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_insn_line(char *bol, char *eol,
+			struct replay_insn_list *item, int lineno)
 {
+	const char *todo_file = git_path(SEQ_TODO_FILE);
 	unsigned char commit_sha1[20];
 	char *end_of_object_name;
-	int saved, status, padding;
+	int saved, status;
+	size_t error_len;
 
-	if (!prefixcmp(bol, "pick")) {
+	if (!prefixcmp(bol, "pick ") || !prefixcmp(bol, "pick\t")) {
 		item->action = REPLAY_PICK;
-		bol += strlen("pick");
-	} else if (!prefixcmp(bol, "revert")) {
+		bol += strlen("pick ");
+	} else if (!prefixcmp(bol, "revert ") || !prefixcmp(bol, "revert\t")) {
 		item->action = REPLAY_REVERT;
-		bol += strlen("revert");
-	} else
-		return -1;
+		bol += strlen("revert ");
+	} else {
+		error_len = eol - bol > 255 ? 255 : eol - bol;
+		return error(_("%s:%d: Unrecognized action: %.*s"),
+			todo_file, lineno, (int)error_len, bol);
+	}
 
 	/* Eat up extra spaces/ tabs before object name */
-	padding = strspn(bol, " \t");
-	if (!padding)
-		return -1;
-	bol += padding;
+	bol += strspn(bol, " \t");
 
 	end_of_object_name = bol + strcspn(bol, " \t\n");
 	saved = *end_of_object_name;
@@ -741,12 +744,18 @@ static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
 	status = get_sha1(bol, commit_sha1);
 	*end_of_object_name = saved;
 
-	if (status < 0)
-		return -1;
+	if (status < 0) {
+		error_len = eol - bol > 255 ? 255 : eol - bol;
+		return error(_("%s:%d: Malformed object name: %.*s"),
+			todo_file, lineno, (int)error_len, bol);
+	}
 
 	item->operand = lookup_commit_reference(commit_sha1);
-	if (!item->operand)
-		return -1;
+	if (!item->operand) {
+		error_len = eol - bol > 255 ? 255 : eol - bol;
+		return error(_("%s:%d: Not a valid commit: %.*s"),
+			todo_file, lineno, (int)error_len, bol);
+	}
 
 	item->next = NULL;
 	return 0;
@@ -761,8 +770,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) < 0)
-			return error(_("Could not parse line %d."), i);
+		if (parse_insn_line(p, eol, &item, i) < 0)
+			return -1;
 		next = replay_insn_list_append(item.operand, item.action, next);
 		p = *eol ? eol + 1 : eol;
 	}
-- 
1.7.8.2

  parent reply	other threads:[~2012-01-08 12:29 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-08 12:27 [PATCH 0/6] The move to sequencer.c Ramkumar Ramachandra
2012-01-08 12:27 ` [PATCH 1/6] revert: move replay_action, replay_subcommand to header Ramkumar Ramachandra
2012-01-08 19:31   ` Jonathan Nieder
2012-01-08 12:27 ` [PATCH 2/6] revert: decouple sequencer actions from builtin commands Ramkumar Ramachandra
2012-01-08 19:34   ` Jonathan Nieder
2012-01-08 19:53     ` Ramkumar Ramachandra
2012-01-08 20:09       ` Jonathan Nieder
2012-01-08 20:07         ` Ramkumar Ramachandra
2012-01-08 20:48           ` Jonathan Nieder
2012-01-08 12:27 ` [PATCH 3/6] revert: don't let revert continue a cherry-pick Ramkumar Ramachandra
2012-01-08 19:37   ` Jonathan Nieder
2012-01-08 20:03     ` Ramkumar Ramachandra
2012-01-08 20:22       ` Jonathan Nieder
2012-01-08 20:28         ` Ramkumar Ramachandra
2012-01-08 20:45           ` Jonathan Nieder
2012-01-08 12:27 ` [PATCH 4/6] revert: allow mixing "pick" and "revert" actions Ramkumar Ramachandra
2012-01-08 19:40   ` Jonathan Nieder
2012-01-08 20:17     ` Ramkumar Ramachandra
2012-01-08 21:40       ` Jonathan Nieder
2012-01-08 21:55         ` Jonathan Nieder
2012-01-10  3:40           ` Ramkumar Ramachandra
2012-01-08 12:27 ` Ramkumar Ramachandra [this message]
2012-01-08 20:07   ` [PATCH 5/6] revert: report fine-grained error messages from insn parser Jonathan Nieder
2012-01-08 20:16     ` Ramkumar Ramachandra
2012-01-08 21:33       ` Jonathan Nieder
2012-01-10 15:24         ` Ramkumar Ramachandra
2012-01-08 12:27 ` [PATCH 6/6] sequencer: factor code out of revert builtin Ramkumar Ramachandra
2012-01-08 20:38   ` Jonathan Nieder
2012-01-10 15:21     ` Ramkumar Ramachandra
2012-01-08 19:28 ` [PATCH 0/6] The move to sequencer.c Jonathan Nieder
2012-01-08 19:51   ` Ramkumar Ramachandra
2012-01-08 20:43     ` Jonathan Nieder
2012-01-10 16:13 ` [PATCH v2 0/8] " Ramkumar Ramachandra
2012-01-10 16:13   ` [PATCH 1/8] revert: prepare to move replay_action to header Ramkumar Ramachandra
2012-01-10 18:27     ` Jonathan Nieder
2012-01-10 16:13   ` [PATCH 2/8] revert: decouple sequencer actions from builtin commands Ramkumar Ramachandra
2012-01-10 18:38     ` Jonathan Nieder
2012-01-11  4:02       ` Ramkumar Ramachandra
2012-01-11  4:17         ` Ramkumar Ramachandra
2012-01-11  5:04           ` Jonathan Nieder
2012-01-11  5:14             ` Ramkumar Ramachandra
2012-01-11  5:26               ` Ramkumar Ramachandra
2012-01-11  5:49                 ` Jonathan Nieder
2012-01-11  9:19                   ` Ramkumar Ramachandra
2012-01-11  9:52                     ` Jonathan Nieder
2012-01-11 10:11                       ` Ramkumar Ramachandra
2012-01-11 13:40                         ` Jonathan Nieder
2012-01-11 13:18               ` Jonathan Nieder
2012-01-11 16:39                 ` Ramkumar Ramachandra
2012-01-11 16:47                   ` Jonathan Nieder
2012-01-11 16:52                     ` Ramkumar Ramachandra
2012-01-11 18:15                     ` [PATCH v3 0/2] The move to sequencer.c Ramkumar Ramachandra
2012-01-11 18:15                       ` [PATCH 1/2] revert: prepare to move replay_action to header Ramkumar Ramachandra
2012-01-11 18:15                       ` [PATCH 2/2] sequencer: factor code out of revert builtin Ramkumar Ramachandra
2012-01-11 18:40                       ` [PATCH v3 0/2] The move to sequencer.c Jonathan Nieder
2012-01-10 16:13   ` [PATCH 3/8] revert: allow mixing "pick" and "revert" actions Ramkumar Ramachandra
2012-01-10 16:13   ` [PATCH 4/8] revert: separate out parse errors logically Ramkumar Ramachandra
2012-01-10 19:03     ` Jonathan Nieder
2012-01-11 12:38       ` Ramkumar Ramachandra
2012-01-10 16:13   ` [PATCH 5/8] revert: report fine-grained errors from insn parser Ramkumar Ramachandra
2012-01-11 12:44     ` Jonathan Nieder
2012-01-10 16:13   ` [PATCH 6/8] sha1_name: introduce getn_sha1() to take length Ramkumar Ramachandra
2012-01-10 16:13   ` [PATCH 7/8] revert: use getn_sha1() to simplify insn parsing Ramkumar Ramachandra
2012-01-10 16:13   ` [PATCH 8/8] sequencer: factor code out of revert builtin Ramkumar Ramachandra

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

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

  git send-email \
    --in-reply-to=1326025653-11922-6-git-send-email-artagnon@gmail.com \
    --to=artagnon@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.