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

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

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.

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:
-- 
2.54.0.rc1.174.gd833f386ac5.dirty