Re: [PATCH v2 0/2] status: improve rebase todo list parsing

[Phillip Wood <phillip.wood123@gmail.com>]

On 01/05/2026 16:16, Phillip Wood wrote:
> When there is rebase in progress "git status" displays the last couple
> of completed and the next couple of pending commands from the todo
> list. When it does this is tries to abbreviate the object ids of
> the commits to be picked. Unfortunately it does not abbreviate the
> object ids when the line starts with "fixup -C" or "merge -C". It
> also mistakenly replaces the refname in "reset main" and "update-ref
> refs/heads/main" with the object id that the ref points to.
> 
> This series fixes that. The first patch factors out the sequencer
> code that parses the command names in the todo list. The second patch
> uses that function in "git status" to parse the command names so that
> it knows whether the line may contain "-C" and whether there is an
> object id that should be abbreviated.
> 
> Thanks to Elijah and Patrick for their comments in V1.

Sorry Tian, I'd forgotten that you'd commented on these patches as well 
- thanks for your comments too.

Phillip

> 
> Changes since V1:
> 
> Patch 1 - Expanded commit message and added a code comment.
> 
> Patch 2 - Fixed some typos, added a code comment and clarified that -Wswitch
>            is included by -Wall.
> 
> Base-Commit: 8c9303b1ffae5b745d1b0a1f98330cf7944d8db0
> Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Fimprove-status-todo-list-parsing%2Fv2
> View-Changes-At: https://github.com/phillipwood/git/compare/8c9303b1f...b80bc1e0a
> Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/improve-status-todo-list-parsing/v2
> 
> 
> Phillip Wood (2):
>    sequencer: factor out parsing of todo commands
>    status: improve rebase todo list parsing
> 
>   sequencer.c            |  45 +++++++++-----
>   sequencer.h            |   8 +++
>   t/t7512-status-help.sh |  74 +++++++++++++++--------
>   wt-status.c            | 131 +++++++++++++++++++++++++++++++++--------
>   4 files changed, 191 insertions(+), 67 deletions(-)
> 
> Range-diff against v1:
> 1:  3d5135a719 ! 1:  d27dddff93 sequencer: factor out parsing of todo commands
>      @@ Metadata
>        ## Commit message ##
>           sequencer: factor out parsing of todo commands
>       
>      -    Move the code that parses todo commands into a separate function so that
>      -    it can be shared with "git status" in the next commit.
>      +    Move the code that parses todo commands into a separate function so
>      +    that it can be shared with "git status" in the next commit. As we
>      +    know the input is NUL terminated we do not pass a pointer to the end
>      +    of the line and instead test for a blank line by looking for NUL, CR
>      +    LF, or LF. We use starts_with() instead of starts_with_mem() for the
>      +    same reason. This results in slightly different behavior when there
>      +    a CR at the start of the line that is not followed by LF. Previously
>      +    such a line was treated as a comment rather than an invalid line.
>       
>           Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>       
>      @@ sequencer.h: int read_author_script(const char *path, char **name, char **email,
>        int write_basic_state(struct replay_opts *opts, const char *head_name,
>        		      struct commit *onto, const struct object_id *orig_head);
>        void sequencer_post_commit_cleanup(struct repository *r, int verbose);
>      ++
>      ++/*
>      ++ * Try to parse the todo command pointed to by *p. On success sets cmd,
>      ++ * advances p and returns true. On failure returns false, leaves p and
>      ++ * cmd unchanged.
>      ++ */
>       +bool sequencer_parse_todo_command(const char **p, enum todo_command *cmd);
>      ++
>        int sequencer_get_last_command(struct repository* r,
>        			       enum replay_action *action);
>        int sequencer_determine_whence(struct repository *r, enum commit_whence *whence);
> 2:  d20dc1f655 ! 2:  b80bc1e0a2 status: improve rebase todo list parsing
>      @@ Commit message
>       
>           When there is rebase in progress "git status" displays the last couple
>           of completed and the next couple of pending commands from the todo
>      -    list. When it does this is tries to abbreviate the object ids of
>      +    list. When it does this it tries to abbreviate the object ids of
>           the commits to be picked. Unfortunately it does not abbreviate the
>           object ids when the line starts with "fixup -C" or "merge -C". It
>           also mistakenly replaces the refname in "reset main" and "update-ref
>      @@ Commit message
>           wider variety of commands. Only the pending commands in the tests
>           are changed to avoid removing existing coverage.
>       
>      +    Helped-by: Elijah Newren <newren@gmail.com>
>           Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>       
>        ## t/t7512-status-help.sh ##
>      @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)
>        	return split_in_progress;
>       +}
>       +
>      ++/*
>      ++ * If the whitespace-delimited token starting at or just after *pp *
>      ++ * is a hex object id that is longer than its default abbreviation, *
>      ++ * abbreviate it in-place, shrinking `line` accordingly. On return
>      ++ * *pp points one past the (possibly abbreviated) token. Leaves both
>      ++ * `line` and *pp-advanced-past-the-token unchanged in all other cases
>      ++ * (non-hex token, unresolvable, or a refname that happens to consist
>      ++ * only of hex digits).
>      ++ */
>       +static void abbrev_oid_in_line(struct repository *r,
>       +			       struct strbuf *line, char **pp)
>       +{
>      @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)
>       +	p += strspn(p, " \t");
>       +	end_of_object_name = p + strcspn(p, " \t");
>       +	/*
>      -+	 * The for "merge" and "reset" the object name may be a label or
>      ++	 * For "merge" and "reset" the object name may be a label or
>       +	 * ref rather than a hex object id. Only abbreviate the object
>       +	 * name if it is a hex object id.
>       +	 */
>      @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)
>       +	*pp = end_of_object_name;
>       +}
>       +
>      -+static void skip_dash_c(char **pp) {
>      ++static void skip_dash_c(char **pp)
>      ++{
>       +	char *p = *pp;
>       +
>       +	p += strspn(p, " \t");
>      -+	/* The (void) cast is required to silence -Wunused_value */
>      ++	/* The (void) cast is required to silence -Wunused-value */
>       +	(void)(skip_prefix(p, "-C", &p) || skip_prefix(p, "-c", &p));
>       +	*pp = p;
>        }
>      @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)
>       +
>       +	/*
>       +	 * Avoid "default" and instead list all the other commands so
>      -+	 * that -Wswitch warns if a new command is added without handling
>      -+	 * it in this function.
>      ++	 * that -Wswitch (which is included in -Wall) warns if a new
>      ++	 * command is added without handling it in this function.
>       +	 */
>       +	case TODO_BREAK:
>       +	case TODO_EXEC: