{"thread":{"id":"65520","subject":"[PATCH 0/2] status: improve rebase todo list parsing","startedAt":"2026-04-20T15:05:12Z","lastAt":"2026-06-23T15:55:01Z","messageCount":29,"participants":["Phillip Wood","Tian Yuchen","Elijah Newren","Patrick Steinhardt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"541976","messageId":"cover.1776697483.git.phillip.wood@dunelm.org.uk","threadId":"65520","inReplyTo":null,"subject":"[PATCH 0/2] status: improve rebase todo list parsing","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-04-20T15:04:42Z","receivedAt":"2026-04-20T15:05:12Z","isPatch":true,"body":"When there is rebase in progress \"git status\" displays the last couple\nof completed and the next couple of pending commands from the todo\nlist. When it does this is tries to abbreviate the object ids of\nthe commits to be picked. Unfortunately it does not abbreviate the\nobject ids when the line starts with \"fixup -C\" or \"merge -C\". It\nalso mistakenly replaces the refname in \"reset main\" and \"update-ref\nrefs/heads/main\" with the object id that the ref points to.\n\nThis series fixes that. The first patch factors out the sequencer\ncode that parses the command names in the todo list. The second patch\nuses that function in \"git status\" to parse the command names so that\nit knows whether the line may contain \"-C\" and whether there is an\nobject id that should be abbreviated.\n\nBase-Commit: 8c9303b1ffae5b745d1b0a1f98330cf7944d8db0\nPublished-As: https://github.com/phillipwood/git/releases/tag/pw%2Fimprove-status-todo-list-parsing%2Fv1\nView-Changes-At: https://github.com/phillipwood/git/compare/8c9303b1f...d20dc1f65\nFetch-It-Via: git fetch https://github.com/phillipwood/git pw/improve-status-todo-list-parsing/v1\n\n\nPhillip Wood (2):\n sequencer: factor out parsing of todo commands\n status: improve rebase todo list parsing\n\n sequencer.c | 45 ++++++++++-----\n sequencer.h | 1 +\n t/t7512-status-help.sh | 74 ++++++++++++++++---------\n wt-status.c | 121 ++++++++++++++++++++++++++++++++---------\n 4 files changed, 174 insertions(+), 67 deletions(-)\n\n-- \n2.54.0.rc1.174.gd833f386ac5.dirty\n\n"},{"id":"541977","messageId":"3d5135a719221031e50ad8067ff42740a3bbce0c.1776697483.git.phillip.wood@dunelm.org.uk","threadId":"65520","inReplyTo":"cover.1776697483.git.phillip.wood@dunelm.org.uk","subject":"[PATCH 1/2] sequencer: factor out parsing of todo commands","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-04-20T15:04:43Z","receivedAt":"2026-04-20T15:05:13Z","isPatch":true,"body":"From: Phillip Wood \n\nMove the code that parses todo commands into a separate function so that\nit can be shared with \"git status\" in the next commit.\n\nSigned-off-by: Phillip Wood \n---\n sequencer.c | 45 ++++++++++++++++++++++++++++++---------------\n sequencer.h | 1 +\n 2 files changed, 31 insertions(+), 15 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex b7d8dca47f..b8e860434a 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2625,6 +2625,27 @@ static int is_command(enum todo_command command, const char **bol)\n \t\treturn 1;\n \t}\n \treturn 0;\n+}\n+\n+bool sequencer_parse_todo_command(const char **p, enum todo_command *cmd)\n+{\n+\tconst char *s = *p;\n+\n+\tfor (int i = 0; i < TODO_COMMENT; i++)\n+\t\tif (is_command(i, p)) {\n+\t\t\t*cmd = i;\n+\t\t\treturn true;\n+\t\t}\n+\n+\tif (starts_with(s, comment_line_str)) {\n+\t\t*cmd = TODO_COMMENT;\n+\t\treturn true;\n+\t} else if (s[0] == '\\n' || (s[0] == '\\r' && s[1] == '\\n') || !s[0]) {\n+\t\t*cmd = TODO_COMMENT;\n+\t\treturn true;\n+\t}\n+\n+\treturn false;\n }\n \n static int check_label_or_ref_arg(enum todo_command command, const char *arg)\n@@ -2716,29 +2737,23 @@ static int parse_insn_line(struct repository *r, struct replay_opts *opts,\n {\n \tstruct object_id commit_oid;\n \tchar *end_of_object_name;\n-\tint i, saved, status, padding;\n+\tint saved, status, padding;\n \n \titem->flags = 0;\n \n \t/* left-trim */\n \tbol += strspn(bol, \" \\t\");\n \n-\tif (bol == eol || *bol == '\\r' || starts_with_mem(bol, eol - bol, comment_line_str)) {\n-\t\titem->command = TODO_COMMENT;\n-\t\titem->commit = NULL;\n-\t\titem->arg_offset = bol - buf;\n-\t\titem->arg_len = eol - bol;\n-\t\treturn 0;\n-\t}\n-\n-\tfor (i = 0; i < TODO_COMMENT; i++)\n-\t\tif (is_command(i, &bol)) {\n-\t\t\titem->command = i;\n-\t\t\tbreak;\n-\t\t}\n-\tif (i >= TODO_COMMENT)\n+\tif (!sequencer_parse_todo_command(&bol, &item->command))\n \t\treturn error(_(\"invalid command '%.*s'\"),\n \t\t\t (int)strcspn(bol, \" \\t\\r\\n\"), bol);\n+\n+\tif (item->command == TODO_COMMENT) {\n+\t\titem->commit = NULL;\n+\t\titem->arg_offset = bol - buf;\n+\t\titem->arg_len = eol - bol;\n+\t\treturn 0;\n+\t}\n \n \t/* Eat up extra spaces/ tabs before object name */\n \tpadding = strspn(bol, \" \\t\");\ndiff --git a/sequencer.h b/sequencer.h\nindex a6fa670c7c..20f6fac48a 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -262,6 +262,7 @@ int read_author_script(const char *path, char **name, char **email, char **date,\n int write_basic_state(struct replay_opts *opts, const char *head_name,\n \t\t struct commit *onto, const struct object_id *orig_head);\n void sequencer_post_commit_cleanup(struct repository *r, int verbose);\n+bool sequencer_parse_todo_command(const char **p, enum todo_command *cmd);\n int sequencer_get_last_command(struct repository* r,\n \t\t\t enum replay_action *action);\n int sequencer_determine_whence(struct repository *r, enum commit_whence *whence);\n-- \n2.54.0.rc1.174.gd833f386ac5.dirty\n\n"},{"id":"541978","messageId":"d20dc1f6550078883995ae963b91faaa00984c6e.1776697483.git.phillip.wood@dunelm.org.uk","threadId":"65520","inReplyTo":"cover.1776697483.git.phillip.wood@dunelm.org.uk","subject":"[PATCH 2/2] status: improve rebase todo list parsing","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-04-20T15:04:44Z","receivedAt":"2026-04-20T15:05:14Z","isPatch":true,"body":"From: Phillip Wood \n\nWhen there is rebase in progress \"git status\" displays the last couple\nof completed and the next couple of pending commands from the todo\nlist. When it does this is tries to abbreviate the object ids of\nthe commits to be picked. Unfortunately it does not abbreviate the\nobject ids when the line starts with \"fixup -C\" or \"merge -C\". It\nalso mistakenly replaces the refname in \"reset main\" and \"update-ref\nrefs/heads/main\" with the object id that the ref points to. Use\nthe function added in the last commit to parse the command name and\nonly try to abbreviate the argument for commands that take an object\nid. When trying to abbreviate an object id, only replace the object\nname if it starts with the abbreviated object id so that labels or\nbranch names that contain only hex digits are left unchanged.\n\nComments are now processed after stripping any leading\nwhitespace from the line. This matches what the sequencer does in\nparse_insn_line(). The existing test cases are updated to test a\nwider variety of commands. Only the pending commands in the tests\nare changed to avoid removing existing coverage.\n\nSigned-off-by: Phillip Wood \n---\n t/t7512-status-help.sh | 74 ++++++++++++++++---------\n wt-status.c | 121 ++++++++++++++++++++++++++++++++---------\n 2 files changed, 143 insertions(+), 52 deletions(-)\n\ndiff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh\nindex 08e82f7914..aca4b6d332 100755\n--- a/t/t7512-status-help.sh\n+++ b/t/t7512-status-help.sh\n@@ -224,7 +224,7 @@ test_expect_success 'status when splitting a commit' '\n \tCOMMIT3=$(git rev-parse --short split_commit) &&\n \ttest_commit four_split main.txt four &&\n \tCOMMIT4=$(git rev-parse --short split_commit) &&\n-\tFAKE_LINES=\"1 edit 2 3\" &&\n+\tFAKE_LINES=\"reword 1 edit 2 fixup_-C 3\" &&\n \texport FAKE_LINES &&\n \ttest_when_finished \"git rebase --abort\" &&\n \tONTO=$(git rev-parse --short HEAD~3) &&\n@@ -233,10 +233,10 @@ test_expect_success 'status when splitting a commit' '\n \tcat >expected <todo <<-EOF &&\n+\tedit several_edits^^ # two_edits\n+\tedit several_edits^ # three_edits\n+\tmerge $(git rev-parse main) $(git rev-parse several_edits)\n+\tEOF\n+\t(\n+\t\tset_replace_editor todo &&\n+\t\tgit rebase -i HEAD~3\n+\t) &&\n \tgit commit --amend -m \"c\" &&\n \tgit rebase --continue &&\n \tgit commit --amend -m \"d\" &&\n@@ -477,7 +483,7 @@ Last commands done (2 commands done):\n edit $COMMIT2 # two_edits\n edit $COMMIT3 # three_edits\n Next command to do (1 remaining command):\n- pick $COMMIT4 # four_edits\n+ merge $(git rev-parse --short main) $COMMIT4\n (use \"git rebase --edit-todo\" to view and edit)\n You are currently editing a commit while rebasing branch '\\''several_edits'\\'' on '\\''$ONTO'\\''.\n (use \"git commit --amend\" to amend the current commit)\n@@ -525,14 +531,21 @@ EOF\n \n test_expect_success 'status: (split first edit) second edit and split' '\n \tgit reset --hard several_edits &&\n-\tFAKE_LINES=\"edit 1 edit 2 3\" &&\n-\texport FAKE_LINES &&\n \ttest_when_finished \"git rebase --abort\" &&\n \tCOMMIT2=$(git rev-parse --short several_edits^^) &&\n \tCOMMIT3=$(git rev-parse --short several_edits^) &&\n \tCOMMIT4=$(git rev-parse --short several_edits) &&\n+\tcat >todo <<-EOF &&\n+\tedit several_edits^^ # two_edits\n+\tedit several_edits^ # three_edits\n+\treset $(git rev-parse main)\n+\tmerge -C several_edits topic # title\n+\tEOF\n \tONTO=$(git rev-parse --short HEAD~3) &&\n-\tgit rebase -i HEAD~3 &&\n+\t(\n+\t\tset_replace_editor todo &&\n+\t\tgit rebase -i HEAD~3\n+\t) &&\n \tgit reset HEAD^ &&\n \tgit add main.txt &&\n \tgit commit --amend -m \"f\" &&\n@@ -543,8 +556,9 @@ interactive rebase in progress; onto $ONTO\n Last commands done (2 commands done):\n edit $COMMIT2 # two_edits\n edit $COMMIT3 # three_edits\n-Next command to do (1 remaining command):\n- pick $COMMIT4 # four_edits\n+Next commands to do (2 remaining commands):\n+ reset $(git rev-parse --short main)\n+ merge -C $COMMIT4 topic # title\n (use \"git rebase --edit-todo\" to view and edit)\n You are currently splitting a commit while rebasing branch '\\''several_edits'\\'' on '\\''$ONTO'\\''.\n (Once your working directory is clean, run \"git rebase --continue\")\n@@ -563,14 +577,21 @@ EOF\n \n test_expect_success 'status: (split first edit) second edit and amend' '\n \tgit reset --hard several_edits &&\n-\tFAKE_LINES=\"edit 1 edit 2 3\" &&\n-\texport FAKE_LINES &&\n \ttest_when_finished \"git rebase --abort\" &&\n+\tgit branch cafe main &&\n \tCOMMIT2=$(git rev-parse --short several_edits^^) &&\n \tCOMMIT3=$(git rev-parse --short several_edits^) &&\n-\tCOMMIT4=$(git rev-parse --short several_edits) &&\n+\tcat >todo <<-EOF &&\n+\tedit several_edits^^ # two_edits\n+\tedit several_edits^ # three_edits\n+\tupdate-ref refs/heads/main\n+\treset cafe\n+\tEOF\n \tONTO=$(git rev-parse --short HEAD~3) &&\n-\tgit rebase -i HEAD~3 &&\n+\t(\n+\t\tset_replace_editor todo &&\n+\t\tgit rebase -i HEAD~3\n+\t) &&\n \tgit reset HEAD^ &&\n \tgit add main.txt &&\n \tgit commit --amend -m \"g\" &&\n@@ -581,8 +602,9 @@ interactive rebase in progress; onto $ONTO\n Last commands done (2 commands done):\n edit $COMMIT2 # two_edits\n edit $COMMIT3 # three_edits\n-Next command to do (1 remaining command):\n- pick $COMMIT4 # four_edits\n+Next commands to do (2 remaining commands):\n+ update-ref refs/heads/main\n+ reset cafe\n (use \"git rebase --edit-todo\" to view and edit)\n You are currently editing a commit while rebasing branch '\\''several_edits'\\'' on '\\''$ONTO'\\''.\n (use \"git commit --amend\" to amend the current commit)\ndiff --git a/wt-status.c b/wt-status.c\nindex 479ccc3304..ba93a18fc2 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1363,6 +1363,51 @@ static int split_commit_in_progress(struct wt_status *s)\n \tfree(rebase_orig_head);\n \n \treturn split_in_progress;\n+}\n+\n+static void abbrev_oid_in_line(struct repository *r,\n+\t\t\t struct strbuf *line, char **pp)\n+{\n+\tchar *p = *pp;\n+\tchar *end_of_object_name, saved;\n+\tconst char *abbrev;\n+\tstruct object_id oid;\n+\tbool have_oid;\n+\n+\tp += strspn(p, \" \\t\");\n+\tend_of_object_name = p + strcspn(p, \" \\t\");\n+\t/*\n+\t * The for \"merge\" and \"reset\" the object name may be a label or\n+\t * ref rather than a hex object id. Only abbreviate the object\n+\t * name if it is a hex object id.\n+\t */\n+\tfor (const char *q = p; q < end_of_object_name; q++) {\n+\t\tif (!isxdigit(*q))\n+\t\t\tgoto out;\n+\t}\n+\tsaved = *end_of_object_name;\n+\t*end_of_object_name = '\\0';\n+\thave_oid = !repo_get_oid(r, p, &oid);\n+\t*end_of_object_name = saved;\n+\tif (!have_oid)\n+\t\tgoto out; /* object name was a label */\n+\tabbrev = repo_find_unique_abbrev(r, &oid, DEFAULT_ABBREV);\n+\tif (!starts_with(p, abbrev))\n+\t\tgoto out; /* object name was a refname containing only xdigits */\n+\tp += strlen(abbrev);\n+\tstrbuf_remove(line, p - line->buf, end_of_object_name - p);\n+\tend_of_object_name = p;\n+out:\n+\t*pp = end_of_object_name;\n+}\n+\n+static void skip_dash_c(char **pp) {\n+\tchar *p = *pp;\n+\n+\tp += strspn(p, \" \\t\");\n+\t/* The (void) cast is required to silence -Wunused_value */\n+\t(void)(skip_prefix(p, \"-C\", &p) || skip_prefix(p, \"-c\", &p));\n+\t*pp = p;\n }\n \n /*\n@@ -1371,29 +1416,57 @@ static int split_commit_in_progress(struct wt_status *s)\n * into\n * \"pick d6a2f03 some message\"\n *\n- * The function assumes that the line does not contain useless spaces\n- * before or after the command.\n+ * Returns false on comment lines, true otherwise\n */\n-static void abbrev_oid_in_line(struct repository *r, struct strbuf *line)\n+static bool format_todo_line(struct repository *r, struct strbuf *line)\n {\n-\tstruct string_list split = STRING_LIST_INIT_DUP;\n-\tstruct object_id oid;\n-\n-\tif (starts_with(line->buf, \"exec \") ||\n-\t starts_with(line->buf, \"x \") ||\n-\t starts_with(line->buf, \"label \") ||\n-\t starts_with(line->buf, \"l \"))\n-\t\treturn;\n-\n-\tif ((2 <= string_list_split(&split, line->buf, \" \", 2)) &&\n-\t !repo_get_oid(r, split.items[1].string, &oid)) {\n-\t\tstrbuf_reset(line);\n-\t\tstrbuf_addf(line, \"%s \", split.items[0].string);\n-\t\tstrbuf_add_unique_abbrev(line, &oid, DEFAULT_ABBREV);\n-\t\tfor (size_t i = 2; i < split.nr; i++)\n-\t\t\tstrbuf_addf(line, \" %s\", split.items[i].string);\n+\tenum todo_command cmd;\n+\tchar *p = line->buf;\n+\n+\tif (!sequencer_parse_todo_command((const char**)&p, &cmd))\n+\t\treturn true; /* keep invalid lines */\n+\n+\tswitch (cmd) {\n+\tcase TODO_COMMENT:\n+\t\treturn false;\n+\n+\tcase TODO_MERGE:\n+\t\tskip_dash_c(&p);\n+\t\twhile (true) {\n+\t\t\tp += strspn(p, \" \\t\");\n+\t\t\tif (!p[0] || (p[0] == '#' && (!p[1] || isspace(p[1]))))\n+\t\t\t\tbreak;\n+\t\t\tabbrev_oid_in_line(r, line, &p);\n+\t\t}\n+\t\tbreak;\n+\n+\tcase TODO_FIXUP:\n+\t\tskip_dash_c(&p);\n+\t\t/* fallthrough */\n+\tcase TODO_DROP:\n+\tcase TODO_EDIT:\n+\tcase TODO_PICK:\n+\tcase TODO_RESET:\n+\tcase TODO_REVERT:\n+\tcase TODO_REWORD:\n+\tcase TODO_SQUASH:\n+\t\tabbrev_oid_in_line(r, line, &p);\n+\t\tbreak;\n+\n+\t/*\n+\t * Avoid \"default\" and instead list all the other commands so\n+\t * that -Wswitch warns if a new command is added without handling\n+\t * it in this function.\n+\t */\n+\tcase TODO_BREAK:\n+\tcase TODO_EXEC:\n+\tcase TODO_LABEL:\n+\tcase TODO_NOOP:\n+\tcase TODO_UPDATE_REF:\n+\t\tbreak;\n \t}\n-\tstring_list_clear(&split, 0);\n+\n+\treturn true;\n }\n \n static int read_rebase_todolist(struct repository *r, const char *fname, struct string_list *lines)\n@@ -1411,13 +1484,9 @@ static int read_rebase_todolist(struct repository *r, const char *fname, struct\n \t\t\t repo_git_path_replace(r, &buf, \"%s\", fname));\n \t}\n \twhile (!strbuf_getline_lf(&buf, f)) {\n-\t\tif (starts_with(buf.buf, comment_line_str))\n-\t\t\tcontinue;\n \t\tstrbuf_trim(&buf);\n-\t\tif (!buf.len)\n-\t\t\tcontinue;\n-\t\tabbrev_oid_in_line(r, &buf);\n-\t\tstring_list_append(lines, buf.buf);\n+\t\tif (format_todo_line(r, &buf))\n+\t\t\tstring_list_append(lines, buf.buf);\n \t}\n \tfclose(f);\n \n-- \n2.54.0.rc1.174.gd833f386ac5.dirty\n\n"},{"id":"541985","messageId":"b62a96c5-fab4-4c6d-9768-ade48a8476ca@malon.dev","threadId":"65520","inReplyTo":"d20dc1f6550078883995ae963b91faaa00984c6e.1776697483.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH 2/2] status: improve rebase todo list parsing","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-04-20T16:38:54Z","receivedAt":"2026-04-20T16:39:02Z","isPatch":true,"body":"Hi Phillip,\n\nOn 4/20/26 23:04, Phillip Wood wrote:\n\n> +\tif (!starts_with(p, abbrev))\n> +\t\tgoto out; /* object name was a refname containing only xdigits */\n> +\tp += strlen(abbrev);\n> +\tstrbuf_remove(line, p - line->buf, end_of_object_name - p);\n> +\tend_of_object_name = p;\n\n\n> -\tif ((2 <= string_list_split(&split, line->buf, \" \", 2)) &&\n> -\t !repo_get_oid(r, split.items[1].string, &oid)) {\n> -\t\tstrbuf_reset(line);\n> -\t\tstrbuf_addf(line, \"%s \", split.items[0].string);\n> -\t\tstrbuf_add_unique_abbrev(line, &oid, DEFAULT_ABBREV);\n> -\t\tfor (size_t i = 2; i < split.nr; i++)\n> -\t\t\tstrbuf_addf(line, \" %s\", split.items[i].string);\n\nI noticed that after this patch, refnames shorter than seven characters \nare no longer standardised to the standard seven-character length, \nbecause the 'start_with()' function always returns FALSE. The code jumps \ndirectly to 'out', without completing or cutting the refname.\n\nI’m not sure if this was your intention, but I just want to point it out \nfor your information.\n\n(Also noted that there is a very rare scenario where the OID of a \nrefname longer than 7 characters happens to begin with the refname \nitself; in this case, 'start_with' returns TRUE and the string is cut \nincorrectly. However, I think we can safely ignore this.)\n\nRegards, Yuchen\n"},{"id":"542046","messageId":"9b239518-7b1c-4125-939e-38ebe69ed004@gmail.com","threadId":"65520","inReplyTo":"b62a96c5-fab4-4c6d-9768-ade48a8476ca@malon.dev","subject":"Re: [PATCH 2/2] status: improve rebase todo list parsing","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-04-21T16:03:35Z","receivedAt":"2026-04-21T16:03:39Z","isPatch":true,"body":"Hi Tian\n\nOn 20/04/2026 17:38, Tian Yuchen wrote:\n> On 4/20/26 23:04, Phillip Wood wrote:\n> \n>> +    if (!starts_with(p, abbrev))\n>> +        goto out; /* object name was a refname containing only \n>> xdigits */\n>> +    p += strlen(abbrev);\n>> +    strbuf_remove(line, p - line->buf, end_of_object_name - p);\n>> +    end_of_object_name = p;\n> \n> \n>> -    if ((2 <= string_list_split(&split, line->buf, \" \", 2)) &&\n>> -        !repo_get_oid(r, split.items[1].string, &oid)) {\n>> -        strbuf_reset(line);\n>> -        strbuf_addf(line, \"%s \", split.items[0].string);\n>> -        strbuf_add_unique_abbrev(line, &oid, DEFAULT_ABBREV);\n>> -        for (size_t i = 2; i < split.nr; i++)\n>> -            strbuf_addf(line, \" %s\", split.items[i].string);\n> \n> I noticed that after this patch, refnames shorter than seven characters \n> are no longer standardised to the standard seven-character length, \n> because the 'start_with()' function always returns FALSE. The code jumps \n> directly to 'out', without completing or cutting the refname.\n\nDo you mean object id's rather than refnames? We do not want to alter \nrefnames, but we do want to abbreviate object ids. You're right that if \nthe todo list contains an object id that is shorter than the default \nabbreviation length it will not be expanded but \"git rebase\" rewrites \nthe todo list to contain full length object ids after the user has \nedited it. If a user may edit the todo list directly without using \"git \nrebase --edit-todo\" and then run \"git status\" before \"git rebase \n--continue\" the todo list could contain shorter object ids but that \nseems pretty unlikely.\n> \n> I’m not sure if this was your intention, but I just want to point it out \n> for your information.\n> \n> (Also noted that there is a very rare scenario where the OID of a \n> refname longer than 7 characters happens to begin with the refname \n> itself; in this case, 'start_with' returns TRUE and the string is cut \n> incorrectly. However, I think we can safely ignore this.)\n\nYes, I think that's unlikely to happen in practice.\n\nThanks\n\nPhillip\n> Regards, Yuchen\n\n"},{"id":"542092","messageId":"CABPp-BGrRehYkox__=VYVqgViuyjRft_VGiaST+nrjbV7H8PPA@mail.gmail.com","threadId":"65520","inReplyTo":"3d5135a719221031e50ad8067ff42740a3bbce0c.1776697483.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH 1/2] sequencer: factor out parsing of todo commands","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-04-22T00:32:16Z","receivedAt":"2026-04-22T00:32:28Z","isPatch":true,"body":"On Mon, Apr 20, 2026 at 8:41 AM Phillip Wood wrote:\n>\n> From: Phillip Wood \n>\n> Move the code that parses todo commands into a separate function so that\n> it can be shared with \"git status\" in the next commit.\n>\n> Signed-off-by: Phillip Wood \n> ---\n> sequencer.c | 45 ++++++++++++++++++++++++++++++---------------\n> sequencer.h | 1 +\n> 2 files changed, 31 insertions(+), 15 deletions(-)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index b7d8dca47f..b8e860434a 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -2625,6 +2625,27 @@ static int is_command(enum todo_command command, const char **bol)\n> return 1;\n> }\n> return 0;\n> +}\n> +\n> +bool sequencer_parse_todo_command(const char **p, enum todo_command *cmd)\n> +{\n> + const char *s = *p;\n> +\n> + for (int i = 0; i < TODO_COMMENT; i++)\n> + if (is_command(i, p)) {\n> + *cmd = i;\n> + return true;\n> + }\n> +\n> + if (starts_with(s, comment_line_str)) {\n> + *cmd = TODO_COMMENT;\n> + return true;\n> + } else if (s[0] == '\\n' || (s[0] == '\\r' && s[1] == '\\n') || !s[0]) {\n> + *cmd = TODO_COMMENT;\n> + return true;\n> + }\n> +\n> + return false;\n> }\n\nSince *p will be advanced by this function, perhaps it's worth a quick\ncomment before the function explaining how it advances?\n\n>\n> static int check_label_or_ref_arg(enum todo_command command, const char *arg)\n> @@ -2716,29 +2737,23 @@ static int parse_insn_line(struct repository *r, struct replay_opts *opts,\n> {\n> struct object_id commit_oid;\n> char *end_of_object_name;\n> - int i, saved, status, padding;\n> + int saved, status, padding;\n>\n> item->flags = 0;\n>\n> /* left-trim */\n> bol += strspn(bol, \" \\t\");\n>\n> - if (bol == eol || *bol == '\\r' || starts_with_mem(bol, eol - bol, comment_line_str)) {\n> - item->command = TODO_COMMENT;\n> - item->commit = NULL;\n> - item->arg_offset = bol - buf;\n> - item->arg_len = eol - bol;\n> - return 0;\n> - }\n> -\n> - for (i = 0; i < TODO_COMMENT; i++)\n> - if (is_command(i, &bol)) {\n> - item->command = i;\n> - break;\n> - }\n> - if (i >= TODO_COMMENT)\n> + if (!sequencer_parse_todo_command(&bol, &item->command))\n> return error(_(\"invalid command '%.*s'\"),\n> (int)strcspn(bol, \" \\t\\r\\n\"), bol);\n> +\n> + if (item->command == TODO_COMMENT) {\n> + item->commit = NULL;\n> + item->arg_offset = bol - buf;\n> + item->arg_len = eol - bol;\n> + return 0;\n> + }\n\nThe commit message says it's just moving code, but are there some\nsubtle changes? It switches the order of parsing commands and\ncomments, which I think doesn't matter. I think the previous code\nwould treat a bare carriage return not immediately followed by a\nnewline as the start of a comment, whereas the new code only handles\ncarriage return immediately followed by a newline as the start of a\ncomment. I don't think that matters in practice, and was potentially\nbuggy before, but feels like the commit message isn't quite telling\nthe whole story. Maybe it's worth calling out the minor but harmless\ntweaks in the commit message?\n\n\n> /* Eat up extra spaces/ tabs before object name */\n> padding = strspn(bol, \" \\t\");\n> diff --git a/sequencer.h b/sequencer.h\n> index a6fa670c7c..20f6fac48a 100644\n> --- a/sequencer.h\n> +++ b/sequencer.h\n> @@ -262,6 +262,7 @@ int read_author_script(const char *path, char **name, char **email, char **date,\n> int write_basic_state(struct replay_opts *opts, const char *head_name,\n> struct commit *onto, const struct object_id *orig_head);\n> void sequencer_post_commit_cleanup(struct repository *r, int verbose);\n> +bool sequencer_parse_todo_command(const char **p, enum todo_command *cmd);\n> int sequencer_get_last_command(struct repository* r,\n> enum replay_action *action);\n> int sequencer_determine_whence(struct repository *r, enum commit_whence *whence);\n> --\n> 2.54.0.rc1.174.gd833f386ac5.dirty\n\nOtherwise, looks good.\n"},{"id":"542093","messageId":"CABPp-BFziRXjuMKqf=RHgCwuCcujXSSrz0f+BS4pvE6EUbk-WQ@mail.gmail.com","threadId":"65520","inReplyTo":"d20dc1f6550078883995ae963b91faaa00984c6e.1776697483.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH 2/2] status: improve rebase todo list parsing","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-04-22T00:32:21Z","receivedAt":"2026-04-22T00:32:33Z","isPatch":true,"body":"On Mon, Apr 20, 2026 at 8:25 AM Phillip Wood wrote:\n>\n> From: Phillip Wood \n>\n> When there is rebase in progress \"git status\" displays the last couple\n> of completed and the next couple of pending commands from the todo\n> list. When it does this is tries to abbreviate the object ids of\n\nis tries => it tries ?\n\n[...]\n> @@ -1363,6 +1363,51 @@ static int split_commit_in_progress(struct wt_status *s)\n> free(rebase_orig_head);\n>\n> return split_in_progress;\n> +}\n> +\n> +static void abbrev_oid_in_line(struct repository *r,\n> + struct strbuf *line, char **pp)\n> +{\n> + char *p = *pp;\n> + char *end_of_object_name, saved;\n> + const char *abbrev;\n> + struct object_id oid;\n> + bool have_oid;\n\nI'll put \"thinking out loud\" text in square brackets below...\n\n> +\n> + p += strspn(p, \" \\t\");\n> + end_of_object_name = p + strcspn(p, \" \\t\");\n\n[Advances p after whitespace, marks the end of the object with the\nnext whitespace after that.]\n\n> + /*\n> + * The for \"merge\" and \"reset\" the object name may be a label or\n\nThe for => For ?\n\n> + * ref rather than a hex object id. Only abbreviate the object\n> + * name if it is a hex object id.\n> + */\n> + for (const char *q = p; q < end_of_object_name; q++) {\n> + if (!isxdigit(*q))\n> + goto out;\n> + }\n\n\n\n> + saved = *end_of_object_name;\n> + *end_of_object_name = '\\0';\n> + have_oid = !repo_get_oid(r, p, &oid);\n> + *end_of_object_name = saved;\n\n[Tries to resolve the token, doing NUL-termination and restore dance.]\n\n> + if (!have_oid)\n> + goto out; /* object name was a label */\n\n\n> + abbrev = repo_find_unique_abbrev(r, &oid, DEFAULT_ABBREV);\n> + if (!starts_with(p, abbrev))\n> + goto out; /* object name was a refname containing only xdigits */\n\n[Ensures what we have is an oid rather than a branch name that can be\nresolved to an oid]\n\n> + p += strlen(abbrev);\n> + strbuf_remove(line, p - line->buf, end_of_object_name - p);\n> + end_of_object_name = p;\n\n[Splice out a bunch of characters in the middle?]\n\n> +out:\n> + *pp = end_of_object_name;\n> +}\n\nI had a hard time following the logic in the function and trying to\nfigure out what it was doing. I went line by line but had no mental\nmodel to follow. When I got to the comment that is now above\nformat_todo_line(), I suddenly understood, but without it, all the\ncode was hard to follow. Maybe a small comment at the beginning of\nthe function along the lines of\n\n /*\n * If the whitespace-delimited token starting at or just after *pp is a\n * full hex object id that resolves uniquely, rewrite it in place to\n * its default abbreviation, shrinking `line` accordingly. On return\n * *pp points one past the (possibly abbreviated) token. Leaves both\n * `line` and *pp-advanced-past-the-token unchanged in all other cases\n * (non-hex token, unresolvable, or a refname that happens to consist\n * only of hex digits).\n */\n\n? (Assuming I'm understanding correctly, of course.)\n\n> +\n> +static void skip_dash_c(char **pp) {\n\nMove the brace to the next line?\n\n> + char *p = *pp;\n> +\n> + p += strspn(p, \" \\t\");\n> + /* The (void) cast is required to silence -Wunused_value */\n\n-Wunused_value => -Wunused-value ?\n\n> + (void)(skip_prefix(p, \"-C\", &p) || skip_prefix(p, \"-c\", &p));\n> + *pp = p;\n> }\n>\n> /*\n> @@ -1371,29 +1416,57 @@ static int split_commit_in_progress(struct wt_status *s)\n> * into\n> * \"pick d6a2f03 some message\"\n> *\n> - * The function assumes that the line does not contain useless spaces\n> - * before or after the command.\n> + * Returns false on comment lines, true otherwise\n> */\n> -static void abbrev_oid_in_line(struct repository *r, struct strbuf *line)\n> +static bool format_todo_line(struct repository *r, struct strbuf *line)\n> {\n> - struct string_list split = STRING_LIST_INIT_DUP;\n> - struct object_id oid;\n> -\n> - if (starts_with(line->buf, \"exec \") ||\n> - starts_with(line->buf, \"x \") ||\n> - starts_with(line->buf, \"label \") ||\n> - starts_with(line->buf, \"l \"))\n> - return;\n> -\n> - if ((2 <= string_list_split(&split, line->buf, \" \", 2)) &&\n> - !repo_get_oid(r, split.items[1].string, &oid)) {\n> - strbuf_reset(line);\n> - strbuf_addf(line, \"%s \", split.items[0].string);\n> - strbuf_add_unique_abbrev(line, &oid, DEFAULT_ABBREV);\n> - for (size_t i = 2; i < split.nr; i++)\n> - strbuf_addf(line, \" %s\", split.items[i].string);\n> + enum todo_command cmd;\n> + char *p = line->buf;\n> +\n> + if (!sequencer_parse_todo_command((const char**)&p, &cmd))\n> + return true; /* keep invalid lines */\n> +\n> + switch (cmd) {\n> + case TODO_COMMENT:\n> + return false;\n> +\n> + case TODO_MERGE:\n> + skip_dash_c(&p);\n> + while (true) {\n> + p += strspn(p, \" \\t\");\n> + if (!p[0] || (p[0] == '#' && (!p[1] || isspace(p[1]))))\n> + break;\n> + abbrev_oid_in_line(r, line, &p);\n> + }\n> + break;\n> +\n> + case TODO_FIXUP:\n> + skip_dash_c(&p);\n> + /* fallthrough */\n> + case TODO_DROP:\n> + case TODO_EDIT:\n> + case TODO_PICK:\n> + case TODO_RESET:\n> + case TODO_REVERT:\n> + case TODO_REWORD:\n> + case TODO_SQUASH:\n> + abbrev_oid_in_line(r, line, &p);\n> + break;\n> +\n> + /*\n> + * Avoid \"default\" and instead list all the other commands so\n> + * that -Wswitch warns if a new command is added without handling\n> + * it in this function.\n> + */\n\nNice. :-)\n\n> + case TODO_BREAK:\n> + case TODO_EXEC:\n> + case TODO_LABEL:\n> + case TODO_NOOP:\n> + case TODO_UPDATE_REF:\n> + break;\n> }\n> - string_list_clear(&split, 0);\n> +\n> + return true;\n> }\n>\n> static int read_rebase_todolist(struct repository *r, const char *fname, struct string_list *lines)\n> @@ -1411,13 +1484,9 @@ static int read_rebase_todolist(struct repository *r, const char *fname, struct\n> repo_git_path_replace(r, &buf, \"%s\", fname));\n> }\n> while (!strbuf_getline_lf(&buf, f)) {\n> - if (starts_with(buf.buf, comment_line_str))\n> - continue;\n> strbuf_trim(&buf);\n> - if (!buf.len)\n> - continue;\n> - abbrev_oid_in_line(r, &buf);\n> - string_list_append(lines, buf.buf);\n> + if (format_todo_line(r, &buf))\n> + string_list_append(lines, buf.buf);\n> }\n> fclose(f);\n>\n> --\n> 2.54.0.rc1.174.gd833f386ac5.dirty\n\nOther than the minor comments above, this looks like a nice cleanup.\n"},{"id":"542130","messageId":"aejM4EY29MGht5or@pks.im","threadId":"65520","inReplyTo":"CABPp-BFziRXjuMKqf=RHgCwuCcujXSSrz0f+BS4pvE6EUbk-WQ@mail.gmail.com","subject":"Re: [PATCH 2/2] status: improve rebase todo list parsing","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-04-22T13:28:00Z","receivedAt":"2026-04-22T13:28:07Z","isPatch":true,"body":"On Tue, Apr 21, 2026 at 05:32:21PM -0700, Elijah Newren wrote:\n> On Mon, Apr 20, 2026 at 8:25 AM Phillip Wood wrote:\n> > + /*\n> > + * Avoid \"default\" and instead list all the other commands so\n> > + * that -Wswitch warns if a new command is added without handling\n> > + * it in this function.\n> > + */\n> \n> Nice. :-)\n\nDo we actually use -Wswitch anywhere? A quick grep in our code base\ndidn't surface it, so I'm a bit sceptical that we would actually detect\nany missing cases via CI.\n\nThanks!\n\nPatrick\n"},{"id":"542134","messageId":"95b91177-a4a8-4039-bf37-b4ce8d2477bf@gmail.com","threadId":"65520","inReplyTo":"aejM4EY29MGht5or@pks.im","subject":"Re: [PATCH 2/2] status: improve rebase todo list parsing","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-04-22T14:14:09Z","receivedAt":"2026-04-22T14:14:12Z","isPatch":true,"body":"On 22/04/2026 14:28, Patrick Steinhardt wrote:\n> On Tue, Apr 21, 2026 at 05:32:21PM -0700, Elijah Newren wrote:\n>> On Mon, Apr 20, 2026 at 8:25 AM Phillip Wood wrote:\n>>> + /*\n>>> + * Avoid \"default\" and instead list all the other commands so\n>>> + * that -Wswitch warns if a new command is added without handling\n>>> + * it in this function.\n>>> + */\n>>\n>> Nice. :-)\n> \n> Do we actually use -Wswitch anywhere? A quick grep in our code base\n> didn't surface it, so I'm a bit sceptical that we would actually detect\n> any missing cases via CI.\n\nI've just double checked the gcc docs and it's included in -Wall (I'm \npretty sure I tried deleting one of the case statements to check before \nI submitted the patch)\n\nThanks\n\nPhillip\n\n"},{"id":"542135","messageId":"7e44dfab-cd46-4907-b96c-58bada33b663@gmail.com","threadId":"65520","inReplyTo":"CABPp-BFziRXjuMKqf=RHgCwuCcujXSSrz0f+BS4pvE6EUbk-WQ@mail.gmail.com","subject":"Re: [PATCH 2/2] status: improve rebase todo list parsing","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-04-22T14:15:19Z","receivedAt":"2026-04-22T14:15:22Z","isPatch":true,"body":"Hi Elijah\n\nThanks for the review, all you suggestions look sensible to me, I'll \nsend a re-roll.\n\nPhillip\n\nOn 22/04/2026 01:32, Elijah Newren wrote:\n> On Mon, Apr 20, 2026 at 8:25 AM Phillip Wood wrote:\n>>\n>> From: Phillip Wood \n>>\n>> When there is rebase in progress \"git status\" displays the last couple\n>> of completed and the next couple of pending commands from the todo\n>> list. When it does this is tries to abbreviate the object ids of\n> \n> is tries => it tries ?\n> \n> [...]\n>> @@ -1363,6 +1363,51 @@ static int split_commit_in_progress(struct wt_status *s)\n>> free(rebase_orig_head);\n>>\n>> return split_in_progress;\n>> +}\n>> +\n>> +static void abbrev_oid_in_line(struct repository *r,\n>> + struct strbuf *line, char **pp)\n>> +{\n>> + char *p = *pp;\n>> + char *end_of_object_name, saved;\n>> + const char *abbrev;\n>> + struct object_id oid;\n>> + bool have_oid;\n> \n> I'll put \"thinking out loud\" text in square brackets below...\n> \n>> +\n>> + p += strspn(p, \" \\t\");\n>> + end_of_object_name = p + strcspn(p, \" \\t\");\n> \n> [Advances p after whitespace, marks the end of the object with the\n> next whitespace after that.]\n> \n>> + /*\n>> + * The for \"merge\" and \"reset\" the object name may be a label or\n> \n> The for => For ?\n> \n>> + * ref rather than a hex object id. Only abbreviate the object\n>> + * name if it is a hex object id.\n>> + */\n>> + for (const char *q = p; q < end_of_object_name; q++) {\n>> + if (!isxdigit(*q))\n>> + goto out;\n>> + }\n> \n> \n> \n>> + saved = *end_of_object_name;\n>> + *end_of_object_name = '\\0';\n>> + have_oid = !repo_get_oid(r, p, &oid);\n>> + *end_of_object_name = saved;\n> \n> [Tries to resolve the token, doing NUL-termination and restore dance.]\n> \n>> + if (!have_oid)\n>> + goto out; /* object name was a label */\n> \n> \n>> + abbrev = repo_find_unique_abbrev(r, &oid, DEFAULT_ABBREV);\n>> + if (!starts_with(p, abbrev))\n>> + goto out; /* object name was a refname containing only xdigits */\n> \n> [Ensures what we have is an oid rather than a branch name that can be\n> resolved to an oid]\n> \n>> + p += strlen(abbrev);\n>> + strbuf_remove(line, p - line->buf, end_of_object_name - p);\n>> + end_of_object_name = p;\n> \n> [Splice out a bunch of characters in the middle?]\n> \n>> +out:\n>> + *pp = end_of_object_name;\n>> +}\n> \n> I had a hard time following the logic in the function and trying to\n> figure out what it was doing. I went line by line but had no mental\n> model to follow. When I got to the comment that is now above\n> format_todo_line(), I suddenly understood, but without it, all the\n> code was hard to follow. Maybe a small comment at the beginning of\n> the function along the lines of\n> \n> /*\n> * If the whitespace-delimited token starting at or just after *pp is a\n> * full hex object id that resolves uniquely, rewrite it in place to\n> * its default abbreviation, shrinking `line` accordingly. On return\n> * *pp points one past the (possibly abbreviated) token. Leaves both\n> * `line` and *pp-advanced-past-the-token unchanged in all other cases\n> * (non-hex token, unresolvable, or a refname that happens to consist\n> * only of hex digits).\n> */\n> \n> ? (Assuming I'm understanding correctly, of course.)\n> \n>> +\n>> +static void skip_dash_c(char **pp) {\n> \n> Move the brace to the next line?\n> \n>> + char *p = *pp;\n>> +\n>> + p += strspn(p, \" \\t\");\n>> + /* The (void) cast is required to silence -Wunused_value */\n> \n> -Wunused_value => -Wunused-value ?\n> \n>> + (void)(skip_prefix(p, \"-C\", &p) || skip_prefix(p, \"-c\", &p));\n>> + *pp = p;\n>> }\n>>\n>> /*\n>> @@ -1371,29 +1416,57 @@ static int split_commit_in_progress(struct wt_status *s)\n>> * into\n>> * \"pick d6a2f03 some message\"\n>> *\n>> - * The function assumes that the line does not contain useless spaces\n>> - * before or after the command.\n>> + * Returns false on comment lines, true otherwise\n>> */\n>> -static void abbrev_oid_in_line(struct repository *r, struct strbuf *line)\n>> +static bool format_todo_line(struct repository *r, struct strbuf *line)\n>> {\n>> - struct string_list split = STRING_LIST_INIT_DUP;\n>> - struct object_id oid;\n>> -\n>> - if (starts_with(line->buf, \"exec \") ||\n>> - starts_with(line->buf, \"x \") ||\n>> - starts_with(line->buf, \"label \") ||\n>> - starts_with(line->buf, \"l \"))\n>> - return;\n>> -\n>> - if ((2 <= string_list_split(&split, line->buf, \" \", 2)) &&\n>> - !repo_get_oid(r, split.items[1].string, &oid)) {\n>> - strbuf_reset(line);\n>> - strbuf_addf(line, \"%s \", split.items[0].string);\n>> - strbuf_add_unique_abbrev(line, &oid, DEFAULT_ABBREV);\n>> - for (size_t i = 2; i < split.nr; i++)\n>> - strbuf_addf(line, \" %s\", split.items[i].string);\n>> + enum todo_command cmd;\n>> + char *p = line->buf;\n>> +\n>> + if (!sequencer_parse_todo_command((const char**)&p, &cmd))\n>> + return true; /* keep invalid lines */\n>> +\n>> + switch (cmd) {\n>> + case TODO_COMMENT:\n>> + return false;\n>> +\n>> + case TODO_MERGE:\n>> + skip_dash_c(&p);\n>> + while (true) {\n>> + p += strspn(p, \" \\t\");\n>> + if (!p[0] || (p[0] == '#' && (!p[1] || isspace(p[1]))))\n>> + break;\n>> + abbrev_oid_in_line(r, line, &p);\n>> + }\n>> + break;\n>> +\n>> + case TODO_FIXUP:\n>> + skip_dash_c(&p);\n>> + /* fallthrough */\n>> + case TODO_DROP:\n>> + case TODO_EDIT:\n>> + case TODO_PICK:\n>> + case TODO_RESET:\n>> + case TODO_REVERT:\n>> + case TODO_REWORD:\n>> + case TODO_SQUASH:\n>> + abbrev_oid_in_line(r, line, &p);\n>> + break;\n>> +\n>> + /*\n>> + * Avoid \"default\" and instead list all the other commands so\n>> + * that -Wswitch warns if a new command is added without handling\n>> + * it in this function.\n>> + */\n> \n> Nice. :-)\n> \n>> + case TODO_BREAK:\n>> + case TODO_EXEC:\n>> + case TODO_LABEL:\n>> + case TODO_NOOP:\n>> + case TODO_UPDATE_REF:\n>> + break;\n>> }\n>> - string_list_clear(&split, 0);\n>> +\n>> + return true;\n>> }\n>>\n>> static int read_rebase_todolist(struct repository *r, const char *fname, struct string_list *lines)\n>> @@ -1411,13 +1484,9 @@ static int read_rebase_todolist(struct repository *r, const char *fname, struct\n>> repo_git_path_replace(r, &buf, \"%s\", fname));\n>> }\n>> while (!strbuf_getline_lf(&buf, f)) {\n>> - if (starts_with(buf.buf, comment_line_str))\n>> - continue;\n>> strbuf_trim(&buf);\n>> - if (!buf.len)\n>> - continue;\n>> - abbrev_oid_in_line(r, &buf);\n>> - string_list_append(lines, buf.buf);\n>> + if (format_todo_line(r, &buf))\n>> + string_list_append(lines, buf.buf);\n>> }\n>> fclose(f);\n>>\n>> --\n>> 2.54.0.rc1.174.gd833f386ac5.dirty\n> \n> Other than the minor comments above, this looks like a nice cleanup.\n\n"},{"id":"542549","messageId":"cover.1777648598.git.phillip.wood@dunelm.org.uk","threadId":"65520","inReplyTo":"cover.1776697483.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v2 0/2] status: improve rebase todo list parsing","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-05-01T15:16:37Z","receivedAt":"2026-05-01T15:16:56Z","isPatch":true,"body":"When there is rebase in progress \"git status\" displays the last couple\nof completed and the next couple of pending commands from the todo\nlist. When it does this is tries to abbreviate the object ids of\nthe commits to be picked. Unfortunately it does not abbreviate the\nobject ids when the line starts with \"fixup -C\" or \"merge -C\". It\nalso mistakenly replaces the refname in \"reset main\" and \"update-ref\nrefs/heads/main\" with the object id that the ref points to.\n\nThis series fixes that. The first patch factors out the sequencer\ncode that parses the command names in the todo list. The second patch\nuses that function in \"git status\" to parse the command names so that\nit knows whether the line may contain \"-C\" and whether there is an\nobject id that should be abbreviated.\n\nThanks to Elijah and Patrick for their comments in V1.\n\nChanges since V1:\n\nPatch 1 - Expanded commit message and added a code comment.\n\nPatch 2 - Fixed some typos, added a code comment and clarified that -Wswitch\n is included by -Wall.\n\nBase-Commit: 8c9303b1ffae5b745d1b0a1f98330cf7944d8db0\nPublished-As: https://github.com/phillipwood/git/releases/tag/pw%2Fimprove-status-todo-list-parsing%2Fv2\nView-Changes-At: https://github.com/phillipwood/git/compare/8c9303b1f...b80bc1e0a\nFetch-It-Via: git fetch https://github.com/phillipwood/git pw/improve-status-todo-list-parsing/v2\n\n\nPhillip Wood (2):\n sequencer: factor out parsing of todo commands\n status: improve rebase todo list parsing\n\n sequencer.c | 45 +++++++++-----\n sequencer.h | 8 +++\n t/t7512-status-help.sh | 74 +++++++++++++++--------\n wt-status.c | 131 +++++++++++++++++++++++++++++++++--------\n 4 files changed, 191 insertions(+), 67 deletions(-)\n\nRange-diff against v1:\n1: 3d5135a719 ! 1: d27dddff93 sequencer: factor out parsing of todo commands\n @@ Metadata\n ## Commit message ##\n sequencer: factor out parsing of todo commands\n \n - Move the code that parses todo commands into a separate function so that\n - it can be shared with \"git status\" in the next commit.\n + Move the code that parses todo commands into a separate function so\n + that it can be shared with \"git status\" in the next commit. As we\n + know the input is NUL terminated we do not pass a pointer to the end\n + of the line and instead test for a blank line by looking for NUL, CR\n + LF, or LF. We use starts_with() instead of starts_with_mem() for the\n + same reason. This results in slightly different behavior when there\n + a CR at the start of the line that is not followed by LF. Previously\n + such a line was treated as a comment rather than an invalid line.\n \n Signed-off-by: Phillip Wood \n \n @@ sequencer.h: int read_author_script(const char *path, char **name, char **email,\n int write_basic_state(struct replay_opts *opts, const char *head_name,\n \t\t struct commit *onto, const struct object_id *orig_head);\n void sequencer_post_commit_cleanup(struct repository *r, int verbose);\n ++\n ++/*\n ++ * Try to parse the todo command pointed to by *p. On success sets cmd,\n ++ * advances p and returns true. On failure returns false, leaves p and\n ++ * cmd unchanged.\n ++ */\n +bool sequencer_parse_todo_command(const char **p, enum todo_command *cmd);\n ++\n int sequencer_get_last_command(struct repository* r,\n \t\t\t enum replay_action *action);\n int sequencer_determine_whence(struct repository *r, enum commit_whence *whence);\n2: d20dc1f655 ! 2: b80bc1e0a2 status: improve rebase todo list parsing\n @@ Commit message\n \n When there is rebase in progress \"git status\" displays the last couple\n of completed and the next couple of pending commands from the todo\n - list. When it does this is tries to abbreviate the object ids of\n + list. When it does this it tries to abbreviate the object ids of\n the commits to be picked. Unfortunately it does not abbreviate the\n object ids when the line starts with \"fixup -C\" or \"merge -C\". It\n also mistakenly replaces the refname in \"reset main\" and \"update-ref\n @@ Commit message\n wider variety of commands. Only the pending commands in the tests\n are changed to avoid removing existing coverage.\n \n + Helped-by: Elijah Newren \n Signed-off-by: Phillip Wood \n \n ## t/t7512-status-help.sh ##\n @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)\n \treturn split_in_progress;\n +}\n +\n ++/*\n ++ * If the whitespace-delimited token starting at or just after *pp *\n ++ * is a hex object id that is longer than its default abbreviation, *\n ++ * abbreviate it in-place, shrinking `line` accordingly. On return\n ++ * *pp points one past the (possibly abbreviated) token. Leaves both\n ++ * `line` and *pp-advanced-past-the-token unchanged in all other cases\n ++ * (non-hex token, unresolvable, or a refname that happens to consist\n ++ * only of hex digits).\n ++ */\n +static void abbrev_oid_in_line(struct repository *r,\n +\t\t\t struct strbuf *line, char **pp)\n +{\n @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)\n +\tp += strspn(p, \" \\t\");\n +\tend_of_object_name = p + strcspn(p, \" \\t\");\n +\t/*\n -+\t * The for \"merge\" and \"reset\" the object name may be a label or\n ++\t * For \"merge\" and \"reset\" the object name may be a label or\n +\t * ref rather than a hex object id. Only abbreviate the object\n +\t * name if it is a hex object id.\n +\t */\n @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)\n +\t*pp = end_of_object_name;\n +}\n +\n -+static void skip_dash_c(char **pp) {\n ++static void skip_dash_c(char **pp)\n ++{\n +\tchar *p = *pp;\n +\n +\tp += strspn(p, \" \\t\");\n -+\t/* The (void) cast is required to silence -Wunused_value */\n ++\t/* The (void) cast is required to silence -Wunused-value */\n +\t(void)(skip_prefix(p, \"-C\", &p) || skip_prefix(p, \"-c\", &p));\n +\t*pp = p;\n }\n @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)\n +\n +\t/*\n +\t * Avoid \"default\" and instead list all the other commands so\n -+\t * that -Wswitch warns if a new command is added without handling\n -+\t * it in this function.\n ++\t * that -Wswitch (which is included in -Wall) warns if a new\n ++\t * command is added without handling it in this function.\n +\t */\n +\tcase TODO_BREAK:\n +\tcase TODO_EXEC:\n-- \n2.54.0.rc1.174.gd833f386ac5.dirty\n\n"},{"id":"542550","messageId":"d27dddff93144f7b6d7fc89719bdf53b6856c9fc.1777648598.git.phillip.wood@dunelm.org.uk","threadId":"65520","inReplyTo":"cover.1777648598.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v2 1/2] sequencer: factor out parsing of todo commands","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-05-01T15:16:38Z","receivedAt":"2026-05-01T15:16:57Z","isPatch":true,"body":"From: Phillip Wood \n\nMove the code that parses todo commands into a separate function so\nthat it can be shared with \"git status\" in the next commit. As we\nknow the input is NUL terminated we do not pass a pointer to the end\nof the line and instead test for a blank line by looking for NUL, CR\nLF, or LF. We use starts_with() instead of starts_with_mem() for the\nsame reason. This results in slightly different behavior when there\na CR at the start of the line that is not followed by LF. Previously\nsuch a line was treated as a comment rather than an invalid line.\n\nSigned-off-by: Phillip Wood \n---\n sequencer.c | 45 ++++++++++++++++++++++++++++++---------------\n sequencer.h | 8 ++++++++\n 2 files changed, 38 insertions(+), 15 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex b7d8dca47f..b8e860434a 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2625,6 +2625,27 @@ static int is_command(enum todo_command command, const char **bol)\n \t\treturn 1;\n \t}\n \treturn 0;\n+}\n+\n+bool sequencer_parse_todo_command(const char **p, enum todo_command *cmd)\n+{\n+\tconst char *s = *p;\n+\n+\tfor (int i = 0; i < TODO_COMMENT; i++)\n+\t\tif (is_command(i, p)) {\n+\t\t\t*cmd = i;\n+\t\t\treturn true;\n+\t\t}\n+\n+\tif (starts_with(s, comment_line_str)) {\n+\t\t*cmd = TODO_COMMENT;\n+\t\treturn true;\n+\t} else if (s[0] == '\\n' || (s[0] == '\\r' && s[1] == '\\n') || !s[0]) {\n+\t\t*cmd = TODO_COMMENT;\n+\t\treturn true;\n+\t}\n+\n+\treturn false;\n }\n \n static int check_label_or_ref_arg(enum todo_command command, const char *arg)\n@@ -2716,29 +2737,23 @@ static int parse_insn_line(struct repository *r, struct replay_opts *opts,\n {\n \tstruct object_id commit_oid;\n \tchar *end_of_object_name;\n-\tint i, saved, status, padding;\n+\tint saved, status, padding;\n \n \titem->flags = 0;\n \n \t/* left-trim */\n \tbol += strspn(bol, \" \\t\");\n \n-\tif (bol == eol || *bol == '\\r' || starts_with_mem(bol, eol - bol, comment_line_str)) {\n-\t\titem->command = TODO_COMMENT;\n-\t\titem->commit = NULL;\n-\t\titem->arg_offset = bol - buf;\n-\t\titem->arg_len = eol - bol;\n-\t\treturn 0;\n-\t}\n-\n-\tfor (i = 0; i < TODO_COMMENT; i++)\n-\t\tif (is_command(i, &bol)) {\n-\t\t\titem->command = i;\n-\t\t\tbreak;\n-\t\t}\n-\tif (i >= TODO_COMMENT)\n+\tif (!sequencer_parse_todo_command(&bol, &item->command))\n \t\treturn error(_(\"invalid command '%.*s'\"),\n \t\t\t (int)strcspn(bol, \" \\t\\r\\n\"), bol);\n+\n+\tif (item->command == TODO_COMMENT) {\n+\t\titem->commit = NULL;\n+\t\titem->arg_offset = bol - buf;\n+\t\titem->arg_len = eol - bol;\n+\t\treturn 0;\n+\t}\n \n \t/* Eat up extra spaces/ tabs before object name */\n \tpadding = strspn(bol, \" \\t\");\ndiff --git a/sequencer.h b/sequencer.h\nindex a6fa670c7c..28fabef926 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -262,6 +262,14 @@ int read_author_script(const char *path, char **name, char **email, char **date,\n int write_basic_state(struct replay_opts *opts, const char *head_name,\n \t\t struct commit *onto, const struct object_id *orig_head);\n void sequencer_post_commit_cleanup(struct repository *r, int verbose);\n+\n+/*\n+ * Try to parse the todo command pointed to by *p. On success sets cmd,\n+ * advances p and returns true. On failure returns false, leaves p and\n+ * cmd unchanged.\n+ */\n+bool sequencer_parse_todo_command(const char **p, enum todo_command *cmd);\n+\n int sequencer_get_last_command(struct repository* r,\n \t\t\t enum replay_action *action);\n int sequencer_determine_whence(struct repository *r, enum commit_whence *whence);\n-- \n2.54.0.rc1.174.gd833f386ac5.dirty\n\n"},{"id":"542551","messageId":"b80bc1e0a298e2773a2fdab3e73651d59b8d39b7.1777648598.git.phillip.wood@dunelm.org.uk","threadId":"65520","inReplyTo":"cover.1777648598.git.phillip.wood@dunelm.org.uk","subject":"[PATCH v2 2/2] status: improve rebase todo list parsing","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-05-01T15:16:39Z","receivedAt":"2026-05-01T15:16:58Z","isPatch":true,"body":"From: Phillip Wood \n\nWhen there is rebase in progress \"git status\" displays the last couple\nof completed and the next couple of pending commands from the todo\nlist. When it does this it tries to abbreviate the object ids of\nthe commits to be picked. Unfortunately it does not abbreviate the\nobject ids when the line starts with \"fixup -C\" or \"merge -C\". It\nalso mistakenly replaces the refname in \"reset main\" and \"update-ref\nrefs/heads/main\" with the object id that the ref points to. Use\nthe function added in the last commit to parse the command name and\nonly try to abbreviate the argument for commands that take an object\nid. When trying to abbreviate an object id, only replace the object\nname if it starts with the abbreviated object id so that labels or\nbranch names that contain only hex digits are left unchanged.\n\nComments are now processed after stripping any leading\nwhitespace from the line. This matches what the sequencer does in\nparse_insn_line(). The existing test cases are updated to test a\nwider variety of commands. Only the pending commands in the tests\nare changed to avoid removing existing coverage.\n\nHelped-by: Elijah Newren \nSigned-off-by: Phillip Wood \n---\n t/t7512-status-help.sh | 74 +++++++++++++++--------\n wt-status.c | 131 +++++++++++++++++++++++++++++++++--------\n 2 files changed, 153 insertions(+), 52 deletions(-)\n\ndiff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh\nindex 08e82f7914..aca4b6d332 100755\n--- a/t/t7512-status-help.sh\n+++ b/t/t7512-status-help.sh\n@@ -224,7 +224,7 @@ test_expect_success 'status when splitting a commit' '\n \tCOMMIT3=$(git rev-parse --short split_commit) &&\n \ttest_commit four_split main.txt four &&\n \tCOMMIT4=$(git rev-parse --short split_commit) &&\n-\tFAKE_LINES=\"1 edit 2 3\" &&\n+\tFAKE_LINES=\"reword 1 edit 2 fixup_-C 3\" &&\n \texport FAKE_LINES &&\n \ttest_when_finished \"git rebase --abort\" &&\n \tONTO=$(git rev-parse --short HEAD~3) &&\n@@ -233,10 +233,10 @@ test_expect_success 'status when splitting a commit' '\n \tcat >expected <todo <<-EOF &&\n+\tedit several_edits^^ # two_edits\n+\tedit several_edits^ # three_edits\n+\tmerge $(git rev-parse main) $(git rev-parse several_edits)\n+\tEOF\n+\t(\n+\t\tset_replace_editor todo &&\n+\t\tgit rebase -i HEAD~3\n+\t) &&\n \tgit commit --amend -m \"c\" &&\n \tgit rebase --continue &&\n \tgit commit --amend -m \"d\" &&\n@@ -477,7 +483,7 @@ Last commands done (2 commands done):\n edit $COMMIT2 # two_edits\n edit $COMMIT3 # three_edits\n Next command to do (1 remaining command):\n- pick $COMMIT4 # four_edits\n+ merge $(git rev-parse --short main) $COMMIT4\n (use \"git rebase --edit-todo\" to view and edit)\n You are currently editing a commit while rebasing branch '\\''several_edits'\\'' on '\\''$ONTO'\\''.\n (use \"git commit --amend\" to amend the current commit)\n@@ -525,14 +531,21 @@ EOF\n \n test_expect_success 'status: (split first edit) second edit and split' '\n \tgit reset --hard several_edits &&\n-\tFAKE_LINES=\"edit 1 edit 2 3\" &&\n-\texport FAKE_LINES &&\n \ttest_when_finished \"git rebase --abort\" &&\n \tCOMMIT2=$(git rev-parse --short several_edits^^) &&\n \tCOMMIT3=$(git rev-parse --short several_edits^) &&\n \tCOMMIT4=$(git rev-parse --short several_edits) &&\n+\tcat >todo <<-EOF &&\n+\tedit several_edits^^ # two_edits\n+\tedit several_edits^ # three_edits\n+\treset $(git rev-parse main)\n+\tmerge -C several_edits topic # title\n+\tEOF\n \tONTO=$(git rev-parse --short HEAD~3) &&\n-\tgit rebase -i HEAD~3 &&\n+\t(\n+\t\tset_replace_editor todo &&\n+\t\tgit rebase -i HEAD~3\n+\t) &&\n \tgit reset HEAD^ &&\n \tgit add main.txt &&\n \tgit commit --amend -m \"f\" &&\n@@ -543,8 +556,9 @@ interactive rebase in progress; onto $ONTO\n Last commands done (2 commands done):\n edit $COMMIT2 # two_edits\n edit $COMMIT3 # three_edits\n-Next command to do (1 remaining command):\n- pick $COMMIT4 # four_edits\n+Next commands to do (2 remaining commands):\n+ reset $(git rev-parse --short main)\n+ merge -C $COMMIT4 topic # title\n (use \"git rebase --edit-todo\" to view and edit)\n You are currently splitting a commit while rebasing branch '\\''several_edits'\\'' on '\\''$ONTO'\\''.\n (Once your working directory is clean, run \"git rebase --continue\")\n@@ -563,14 +577,21 @@ EOF\n \n test_expect_success 'status: (split first edit) second edit and amend' '\n \tgit reset --hard several_edits &&\n-\tFAKE_LINES=\"edit 1 edit 2 3\" &&\n-\texport FAKE_LINES &&\n \ttest_when_finished \"git rebase --abort\" &&\n+\tgit branch cafe main &&\n \tCOMMIT2=$(git rev-parse --short several_edits^^) &&\n \tCOMMIT3=$(git rev-parse --short several_edits^) &&\n-\tCOMMIT4=$(git rev-parse --short several_edits) &&\n+\tcat >todo <<-EOF &&\n+\tedit several_edits^^ # two_edits\n+\tedit several_edits^ # three_edits\n+\tupdate-ref refs/heads/main\n+\treset cafe\n+\tEOF\n \tONTO=$(git rev-parse --short HEAD~3) &&\n-\tgit rebase -i HEAD~3 &&\n+\t(\n+\t\tset_replace_editor todo &&\n+\t\tgit rebase -i HEAD~3\n+\t) &&\n \tgit reset HEAD^ &&\n \tgit add main.txt &&\n \tgit commit --amend -m \"g\" &&\n@@ -581,8 +602,9 @@ interactive rebase in progress; onto $ONTO\n Last commands done (2 commands done):\n edit $COMMIT2 # two_edits\n edit $COMMIT3 # three_edits\n-Next command to do (1 remaining command):\n- pick $COMMIT4 # four_edits\n+Next commands to do (2 remaining commands):\n+ update-ref refs/heads/main\n+ reset cafe\n (use \"git rebase --edit-todo\" to view and edit)\n You are currently editing a commit while rebasing branch '\\''several_edits'\\'' on '\\''$ONTO'\\''.\n (use \"git commit --amend\" to amend the current commit)\ndiff --git a/wt-status.c b/wt-status.c\nindex 479ccc3304..94c159d9d4 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1363,6 +1363,61 @@ static int split_commit_in_progress(struct wt_status *s)\n \tfree(rebase_orig_head);\n \n \treturn split_in_progress;\n+}\n+\n+/*\n+ * If the whitespace-delimited token starting at or just after *pp *\n+ * is a hex object id that is longer than its default abbreviation, *\n+ * abbreviate it in-place, shrinking `line` accordingly. On return\n+ * *pp points one past the (possibly abbreviated) token. Leaves both\n+ * `line` and *pp-advanced-past-the-token unchanged in all other cases\n+ * (non-hex token, unresolvable, or a refname that happens to consist\n+ * only of hex digits).\n+ */\n+static void abbrev_oid_in_line(struct repository *r,\n+\t\t\t struct strbuf *line, char **pp)\n+{\n+\tchar *p = *pp;\n+\tchar *end_of_object_name, saved;\n+\tconst char *abbrev;\n+\tstruct object_id oid;\n+\tbool have_oid;\n+\n+\tp += strspn(p, \" \\t\");\n+\tend_of_object_name = p + strcspn(p, \" \\t\");\n+\t/*\n+\t * For \"merge\" and \"reset\" the object name may be a label or\n+\t * ref rather than a hex object id. Only abbreviate the object\n+\t * name if it is a hex object id.\n+\t */\n+\tfor (const char *q = p; q < end_of_object_name; q++) {\n+\t\tif (!isxdigit(*q))\n+\t\t\tgoto out;\n+\t}\n+\tsaved = *end_of_object_name;\n+\t*end_of_object_name = '\\0';\n+\thave_oid = !repo_get_oid(r, p, &oid);\n+\t*end_of_object_name = saved;\n+\tif (!have_oid)\n+\t\tgoto out; /* object name was a label */\n+\tabbrev = repo_find_unique_abbrev(r, &oid, DEFAULT_ABBREV);\n+\tif (!starts_with(p, abbrev))\n+\t\tgoto out; /* object name was a refname containing only xdigits */\n+\tp += strlen(abbrev);\n+\tstrbuf_remove(line, p - line->buf, end_of_object_name - p);\n+\tend_of_object_name = p;\n+out:\n+\t*pp = end_of_object_name;\n+}\n+\n+static void skip_dash_c(char **pp)\n+{\n+\tchar *p = *pp;\n+\n+\tp += strspn(p, \" \\t\");\n+\t/* The (void) cast is required to silence -Wunused-value */\n+\t(void)(skip_prefix(p, \"-C\", &p) || skip_prefix(p, \"-c\", &p));\n+\t*pp = p;\n }\n \n /*\n@@ -1371,29 +1426,57 @@ static int split_commit_in_progress(struct wt_status *s)\n * into\n * \"pick d6a2f03 some message\"\n *\n- * The function assumes that the line does not contain useless spaces\n- * before or after the command.\n+ * Returns false on comment lines, true otherwise\n */\n-static void abbrev_oid_in_line(struct repository *r, struct strbuf *line)\n+static bool format_todo_line(struct repository *r, struct strbuf *line)\n {\n-\tstruct string_list split = STRING_LIST_INIT_DUP;\n-\tstruct object_id oid;\n-\n-\tif (starts_with(line->buf, \"exec \") ||\n-\t starts_with(line->buf, \"x \") ||\n-\t starts_with(line->buf, \"label \") ||\n-\t starts_with(line->buf, \"l \"))\n-\t\treturn;\n-\n-\tif ((2 <= string_list_split(&split, line->buf, \" \", 2)) &&\n-\t !repo_get_oid(r, split.items[1].string, &oid)) {\n-\t\tstrbuf_reset(line);\n-\t\tstrbuf_addf(line, \"%s \", split.items[0].string);\n-\t\tstrbuf_add_unique_abbrev(line, &oid, DEFAULT_ABBREV);\n-\t\tfor (size_t i = 2; i < split.nr; i++)\n-\t\t\tstrbuf_addf(line, \" %s\", split.items[i].string);\n+\tenum todo_command cmd;\n+\tchar *p = line->buf;\n+\n+\tif (!sequencer_parse_todo_command((const char**)&p, &cmd))\n+\t\treturn true; /* keep invalid lines */\n+\n+\tswitch (cmd) {\n+\tcase TODO_COMMENT:\n+\t\treturn false;\n+\n+\tcase TODO_MERGE:\n+\t\tskip_dash_c(&p);\n+\t\twhile (true) {\n+\t\t\tp += strspn(p, \" \\t\");\n+\t\t\tif (!p[0] || (p[0] == '#' && (!p[1] || isspace(p[1]))))\n+\t\t\t\tbreak;\n+\t\t\tabbrev_oid_in_line(r, line, &p);\n+\t\t}\n+\t\tbreak;\n+\n+\tcase TODO_FIXUP:\n+\t\tskip_dash_c(&p);\n+\t\t/* fallthrough */\n+\tcase TODO_DROP:\n+\tcase TODO_EDIT:\n+\tcase TODO_PICK:\n+\tcase TODO_RESET:\n+\tcase TODO_REVERT:\n+\tcase TODO_REWORD:\n+\tcase TODO_SQUASH:\n+\t\tabbrev_oid_in_line(r, line, &p);\n+\t\tbreak;\n+\n+\t/*\n+\t * Avoid \"default\" and instead list all the other commands so\n+\t * that -Wswitch (which is included in -Wall) warns if a new\n+\t * command is added without handling it in this function.\n+\t */\n+\tcase TODO_BREAK:\n+\tcase TODO_EXEC:\n+\tcase TODO_LABEL:\n+\tcase TODO_NOOP:\n+\tcase TODO_UPDATE_REF:\n+\t\tbreak;\n \t}\n-\tstring_list_clear(&split, 0);\n+\n+\treturn true;\n }\n \n static int read_rebase_todolist(struct repository *r, const char *fname, struct string_list *lines)\n@@ -1411,13 +1494,9 @@ static int read_rebase_todolist(struct repository *r, const char *fname, struct\n \t\t\t repo_git_path_replace(r, &buf, \"%s\", fname));\n \t}\n \twhile (!strbuf_getline_lf(&buf, f)) {\n-\t\tif (starts_with(buf.buf, comment_line_str))\n-\t\t\tcontinue;\n \t\tstrbuf_trim(&buf);\n-\t\tif (!buf.len)\n-\t\t\tcontinue;\n-\t\tabbrev_oid_in_line(r, &buf);\n-\t\tstring_list_append(lines, buf.buf);\n+\t\tif (format_todo_line(r, &buf))\n+\t\t\tstring_list_append(lines, buf.buf);\n \t}\n \tfclose(f);\n \n-- \n2.54.0.rc1.174.gd833f386ac5.dirty\n\n"},{"id":"542556","messageId":"5256b235-b173-4804-aef0-752eac81d618@gmail.com","threadId":"65520","inReplyTo":"cover.1777648598.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH v2 0/2] status: improve rebase todo list parsing","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-05-01T18:19:05Z","receivedAt":"2026-05-01T18:19:06Z","isPatch":true,"body":"On 01/05/2026 16:16, Phillip Wood wrote:\n> When there is rebase in progress \"git status\" displays the last couple\n> of completed and the next couple of pending commands from the todo\n> list. When it does this is tries to abbreviate the object ids of\n> the commits to be picked. Unfortunately it does not abbreviate the\n> object ids when the line starts with \"fixup -C\" or \"merge -C\". It\n> also mistakenly replaces the refname in \"reset main\" and \"update-ref\n> refs/heads/main\" with the object id that the ref points to.\n> \n> This series fixes that. The first patch factors out the sequencer\n> code that parses the command names in the todo list. The second patch\n> uses that function in \"git status\" to parse the command names so that\n> it knows whether the line may contain \"-C\" and whether there is an\n> object id that should be abbreviated.\n> \n> Thanks to Elijah and Patrick for their comments in V1.\n\nSorry Tian, I'd forgotten that you'd commented on these patches as well \n- thanks for your comments too.\n\nPhillip\n\n> \n> Changes since V1:\n> \n> Patch 1 - Expanded commit message and added a code comment.\n> \n> Patch 2 - Fixed some typos, added a code comment and clarified that -Wswitch\n> is included by -Wall.\n> \n> Base-Commit: 8c9303b1ffae5b745d1b0a1f98330cf7944d8db0\n> Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Fimprove-status-todo-list-parsing%2Fv2\n> View-Changes-At: https://github.com/phillipwood/git/compare/8c9303b1f...b80bc1e0a\n> Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/improve-status-todo-list-parsing/v2\n> \n> \n> Phillip Wood (2):\n> sequencer: factor out parsing of todo commands\n> status: improve rebase todo list parsing\n> \n> sequencer.c | 45 +++++++++-----\n> sequencer.h | 8 +++\n> t/t7512-status-help.sh | 74 +++++++++++++++--------\n> wt-status.c | 131 +++++++++++++++++++++++++++++++++--------\n> 4 files changed, 191 insertions(+), 67 deletions(-)\n> \n> Range-diff against v1:\n> 1: 3d5135a719 ! 1: d27dddff93 sequencer: factor out parsing of todo commands\n> @@ Metadata\n> ## Commit message ##\n> sequencer: factor out parsing of todo commands\n> \n> - Move the code that parses todo commands into a separate function so that\n> - it can be shared with \"git status\" in the next commit.\n> + Move the code that parses todo commands into a separate function so\n> + that it can be shared with \"git status\" in the next commit. As we\n> + know the input is NUL terminated we do not pass a pointer to the end\n> + of the line and instead test for a blank line by looking for NUL, CR\n> + LF, or LF. We use starts_with() instead of starts_with_mem() for the\n> + same reason. This results in slightly different behavior when there\n> + a CR at the start of the line that is not followed by LF. Previously\n> + such a line was treated as a comment rather than an invalid line.\n> \n> Signed-off-by: Phillip Wood \n> \n> @@ sequencer.h: int read_author_script(const char *path, char **name, char **email,\n> int write_basic_state(struct replay_opts *opts, const char *head_name,\n> \t\t struct commit *onto, const struct object_id *orig_head);\n> void sequencer_post_commit_cleanup(struct repository *r, int verbose);\n> ++\n> ++/*\n> ++ * Try to parse the todo command pointed to by *p. On success sets cmd,\n> ++ * advances p and returns true. On failure returns false, leaves p and\n> ++ * cmd unchanged.\n> ++ */\n> +bool sequencer_parse_todo_command(const char **p, enum todo_command *cmd);\n> ++\n> int sequencer_get_last_command(struct repository* r,\n> \t\t\t enum replay_action *action);\n> int sequencer_determine_whence(struct repository *r, enum commit_whence *whence);\n> 2: d20dc1f655 ! 2: b80bc1e0a2 status: improve rebase todo list parsing\n> @@ Commit message\n> \n> When there is rebase in progress \"git status\" displays the last couple\n> of completed and the next couple of pending commands from the todo\n> - list. When it does this is tries to abbreviate the object ids of\n> + list. When it does this it tries to abbreviate the object ids of\n> the commits to be picked. Unfortunately it does not abbreviate the\n> object ids when the line starts with \"fixup -C\" or \"merge -C\". It\n> also mistakenly replaces the refname in \"reset main\" and \"update-ref\n> @@ Commit message\n> wider variety of commands. Only the pending commands in the tests\n> are changed to avoid removing existing coverage.\n> \n> + Helped-by: Elijah Newren \n> Signed-off-by: Phillip Wood \n> \n> ## t/t7512-status-help.sh ##\n> @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)\n> \treturn split_in_progress;\n> +}\n> +\n> ++/*\n> ++ * If the whitespace-delimited token starting at or just after *pp *\n> ++ * is a hex object id that is longer than its default abbreviation, *\n> ++ * abbreviate it in-place, shrinking `line` accordingly. On return\n> ++ * *pp points one past the (possibly abbreviated) token. Leaves both\n> ++ * `line` and *pp-advanced-past-the-token unchanged in all other cases\n> ++ * (non-hex token, unresolvable, or a refname that happens to consist\n> ++ * only of hex digits).\n> ++ */\n> +static void abbrev_oid_in_line(struct repository *r,\n> +\t\t\t struct strbuf *line, char **pp)\n> +{\n> @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)\n> +\tp += strspn(p, \" \\t\");\n> +\tend_of_object_name = p + strcspn(p, \" \\t\");\n> +\t/*\n> -+\t * The for \"merge\" and \"reset\" the object name may be a label or\n> ++\t * For \"merge\" and \"reset\" the object name may be a label or\n> +\t * ref rather than a hex object id. Only abbreviate the object\n> +\t * name if it is a hex object id.\n> +\t */\n> @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)\n> +\t*pp = end_of_object_name;\n> +}\n> +\n> -+static void skip_dash_c(char **pp) {\n> ++static void skip_dash_c(char **pp)\n> ++{\n> +\tchar *p = *pp;\n> +\n> +\tp += strspn(p, \" \\t\");\n> -+\t/* The (void) cast is required to silence -Wunused_value */\n> ++\t/* The (void) cast is required to silence -Wunused-value */\n> +\t(void)(skip_prefix(p, \"-C\", &p) || skip_prefix(p, \"-c\", &p));\n> +\t*pp = p;\n> }\n> @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)\n> +\n> +\t/*\n> +\t * Avoid \"default\" and instead list all the other commands so\n> -+\t * that -Wswitch warns if a new command is added without handling\n> -+\t * it in this function.\n> ++\t * that -Wswitch (which is included in -Wall) warns if a new\n> ++\t * command is added without handling it in this function.\n> +\t */\n> +\tcase TODO_BREAK:\n> +\tcase TODO_EXEC:\n\n"},{"id":"544313","messageId":"xmqqbjdwcsno.fsf@gitster.g","threadId":"65520","inReplyTo":"b80bc1e0a298e2773a2fdab3e73651d59b8d39b7.1777648598.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH v2 2/2] status: improve rebase todo list parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-05-31T00:46:35Z","receivedAt":"2026-05-31T00:46:38Z","isPatch":true,"body":"Phillip Wood writes:\n\n> +static void abbrev_oid_in_line(struct repository *r,\n> +\t\t\t struct strbuf *line, char **pp)\n> +{\n> ...\n> +\thave_oid = !repo_get_oid(r, p, &oid);\n> +\t*end_of_object_name = saved;\n> +\tif (!have_oid)\n> +\t\tgoto out; /* object name was a label */\n\nCan there be a label \"deadbeef123\" that is unrelated to an object whose\nobject name happens to abbreviate to \"deadbeef123\"?\n\n> +\tcase TODO_MERGE:\n> +\t\tskip_dash_c(&p);\n> +\t\twhile (true) {\n> +\t\t\tp += strspn(p, \" \\t\");\n> +\t\t\tif (!p[0] || (p[0] == '#' && (!p[1] || isspace(p[1]))))\n> +\t\t\t\tbreak;\n> +\t\t\tabbrev_oid_in_line(r, line, &p);\n> +\t\t}\n> +\t\tbreak;\n\nWhat does this loop do? A \"merge\" command may look like \"merge\n[[-C|-c] ]