From d258dc16c1bf92c01135c38130869da7808ed739 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Thu, 27 Jun 2019 07:12:44 -0700 Subject: [PATCH 1/3] sequencer: always allow tab after command name The code that parses the todo list allows an unabbreviated command name to be followed by a space or a tab, but if the command name is abbreviated it only allows a space after it. Fix this inconsistency. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- sequencer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sequencer.c b/sequencer.c index f88a97fb10..919e3153f5 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2100,7 +2100,7 @@ static int parse_insn_line(struct repository *r, struct todo_item *item, if (skip_prefix(bol, todo_command_info[i].str, &bol)) { item->command = i; break; - } else if ((bol + 1 == eol || bol[1] == ' ') && + } else if ((bol + 1 == eol || bol[1] == ' ' || bol[1] == '\t') && *bol == todo_command_info[i].c) { bol++; item->command = i; From 3e81bccdf3ceab531e95ba3846083dbc8ba0e319 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Thu, 27 Jun 2019 07:12:45 -0700 Subject: [PATCH 2/3] sequencer: factor out todo command name parsing Factor out the code that parses the name of the command at the start of each line in the todo file into its own function so that it can be used in the next commit. As I don't want to burden other callers with having to pass in a pointer to the end of the line the test for an abbreviated command is changed. This change should not affect the behavior. Instead of testing `eol == bol + 1` the new code checks for the end of the line by testing for '\n', '\r' or '\0' following the abbreviated name. To avoid reading past the end of an empty string it also checks that there is actually a single character abbreviation before testing if it matches. This prevents it from matching '\0' as the abbreviated name and then trying to read another character. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- sequencer.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/sequencer.c b/sequencer.c index 919e3153f5..793f86bf9a 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2076,6 +2076,18 @@ const char *todo_item_get_arg(struct todo_list *todo_list, return todo_list->buf.buf + item->arg_offset; } +static int is_command(enum todo_command command, const char **bol) +{ + const char *str = todo_command_info[command].str; + const char nick = todo_command_info[command].c; + const char *p = *bol + 1; + + return skip_prefix(*bol, str, bol) || + ((nick && **bol == nick) && + (*p == ' ' || *p == '\t' || *p == '\n' || *p == '\r' || !*p) && + (*bol = p)); +} + static int parse_insn_line(struct repository *r, struct todo_item *item, const char *buf, const char *bol, char *eol) { @@ -2097,12 +2109,7 @@ static int parse_insn_line(struct repository *r, struct todo_item *item, } for (i = 0; i < TODO_COMMENT; i++) - if (skip_prefix(bol, todo_command_info[i].str, &bol)) { - item->command = i; - break; - } else if ((bol + 1 == eol || bol[1] == ' ' || bol[1] == '\t') && - *bol == todo_command_info[i].c) { - bol++; + if (is_command(i, &bol)) { item->command = i; break; } From ed5b1ca10b2644f1af3bdfa3da32c67f4df1aa46 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Thu, 27 Jun 2019 07:12:46 -0700 Subject: [PATCH 3/3] status: do not report errors in sequencer/todo commit 4a72486de9 ("fix cherry-pick/revert status after commit", 2019-04-16) used parse_insn_line() to parse the first line of the todo list to check if it was a pick or revert. However if the todo list is left over from an old cherry-pick or revert and references a commit that no longer exists then parse_insn_line() prints an error message which is confusing for users [1]. Instead parse just the command name so that the user is alerted to the presence of stale sequencer state by status reporting that a cherry-pick or revert is in progress. Note that we should not be leaving stale sequencer state lying around (or at least not as often) after commit b07d9bfd17 ("commit/reset: try to clean up sequencer state", 2019-04-16). However the user may still have stale state that predates that commit. Also avoid printing an error message if for some reason the user has a file called `sequencer` in $GIT_DIR. [1] https://public-inbox.org/git/3bc58c33-4268-4e7c-bf1a-fe349b3cb037@www.fastmail.com/ Reported-by: Espen Antonsen Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- sequencer.c | 24 ++++++++---------------- t/t7512-status-help.sh | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/sequencer.c b/sequencer.c index 793f86bf9a..f8eab1697e 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2177,34 +2177,26 @@ static int parse_insn_line(struct repository *r, struct todo_item *item, int sequencer_get_last_command(struct repository *r, enum replay_action *action) { - struct todo_item item; - char *eol; - const char *todo_file; + const char *todo_file, *bol; struct strbuf buf = STRBUF_INIT; - int ret = -1; + int ret = 0; todo_file = git_path_todo_file(); if (strbuf_read_file(&buf, todo_file, 0) < 0) { - if (errno == ENOENT) + if (errno == ENOENT || errno == ENOTDIR) return -1; else return error_errno("unable to open '%s'", todo_file); } - eol = strchrnul(buf.buf, '\n'); - if (buf.buf != eol && eol[-1] == '\r') - eol--; /* strip Carriage Return */ - if (parse_insn_line(r, &item, buf.buf, buf.buf, eol)) - goto fail; - if (item.command == TODO_PICK) + bol = buf.buf + strspn(buf.buf, " \t\r\n"); + if (is_command(TODO_PICK, &bol) && (*bol == ' ' || *bol == '\t')) *action = REPLAY_PICK; - else if (item.command == TODO_REVERT) + else if (is_command(TODO_REVERT, &bol) && + (*bol == ' ' || *bol == '\t')) *action = REPLAY_REVERT; else - goto fail; + ret = -1; - ret = 0; - - fail: strbuf_release(&buf); return ret; diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index c1eb72555d..3c9308709a 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -798,6 +798,22 @@ EOF test_i18ncmp expected actual ' +test_expect_success 'status shows cherry-pick with invalid oid' ' + mkdir .git/sequencer && + test_write_lines "pick invalid-oid" >.git/sequencer/todo && + git status --untracked-files=no >actual 2>err && + git cherry-pick --quit && + test_must_be_empty err && + test_i18ncmp expected actual +' + +test_expect_success 'status does not show error if .git/sequencer is a file' ' + test_when_finished "rm .git/sequencer" && + test_write_lines hello >.git/sequencer && + git status --untracked-files=no 2>err && + test_must_be_empty err +' + test_expect_success 'status showing detached at and from a tag' ' test_commit atag tagging && git checkout atag &&