diff mbox series

[2/3] wt-status: also abbreviate 'merge' and 'fixup -C' lines during rebase

Message ID e297b71ba123b642c2e724d7dda475fa52dfdeaa.1743181401.git.gitgitgadget@gmail.com (mailing list archive)
State New
Headers show
Series rebase -r: a bugfix and two status-related improvements | expand

Commit Message

Philippe Blain March 28, 2025, 5:03 p.m. UTC
From: Philippe Blain <levraiphilippeblain@gmail.com>

When "git status" is invoked during a rebase, we print the last commands
done and the next commands to do, and abbreviate commit hashes found in
those lines. However, we only abbreviate hashes in 'pick', 'squash' and
plain 'fixup' lines, not those in 'merge -C' and 'fixup -C' lines, as
the parsing done in wt-status.c::abbrev_oid_in_line is not prepared for
such lines.

Improve the parsing done by this function by special casing 'fixup' and
'merge' such that the hash to abbreviate is the string found in the
third field of 'split', instead of the second one for other commands.
Introduce a 'hash' strbuf pointer to point to the correct field in all
cases.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
 wt-status.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

Comments

Phillip Wood March 31, 2025, 3:37 p.m. UTC | #1
Hi Philippe

On 28/03/2025 17:03, Philippe Blain via GitGitGadget wrote:
> From: Philippe Blain <levraiphilippeblain@gmail.com>
> 
> When "git status" is invoked during a rebase, we print the last commands
> done and the next commands to do, and abbreviate commit hashes found in
> those lines. However, we only abbreviate hashes in 'pick', 'squash' and
> plain 'fixup' lines, not those in 'merge -C' and 'fixup -C' lines, as
> the parsing done in wt-status.c::abbrev_oid_in_line is not prepared for
> such lines.
> 
> Improve the parsing done by this function by special casing 'fixup' and
> 'merge' such that the hash to abbreviate is the string found in the
> third field of 'split', instead of the second one for other commands.
> Introduce a 'hash' strbuf pointer to point to the correct field in all
> cases.

Sounds good. It is a shame that the parsing here is not better 
integrated with the sequencer. I think that would be a much bigger task 
though. The patch looks good and is definitely an improvement on the 
status quo for the user.

I was going to ask about a test but it looks like one of the tests added 
in the next patch checks that we abbreviate "merge -C <oid>". It would 
be worth mentioning that in the commit message.

Thanks

Phillip

> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
> ---
>   wt-status.c | 31 ++++++++++++++++++++++---------
>   1 file changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/wt-status.c b/wt-status.c
> index 1da5732f57b..d11d9f9f142 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1342,9 +1342,11 @@ static int split_commit_in_progress(struct wt_status *s)
>   
>   /*
>    * Turn
> - * "pick d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some message"
> + * "pick d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some message" and
> + * "merge -C d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some-branch"
>    * into
> - * "pick d6a2f03 some message"
> + * "pick d6a2f03 some message" and
> + * "merge -C d6a2f03 some-branch"
>    *
>    * The function assumes that the line does not contain useless spaces
>    * before or after the command.
> @@ -1360,20 +1362,31 @@ static void abbrev_oid_in_line(struct strbuf *line)
>   	    starts_with(line->buf, "l "))
>   		return;
>   
> -	split = strbuf_split_max(line, ' ', 3);
> +	split = strbuf_split_max(line, ' ', 4);
>   	if (split[0] && split[1]) {
>   		struct object_id oid;
> -
> +		struct strbuf *hash;
> +
> +		if ((!strcmp(split[0]->buf, "merge ") ||
> +		     !strcmp(split[0]->buf, "m "    ) ||
> +		     !strcmp(split[0]->buf, "fixup ") ||
> +		     !strcmp(split[0]->buf, "f "    )) &&
> +		    (!strcmp(split[1]->buf, "-C ") ||
> +		     !strcmp(split[1]->buf, "-c "))) {
> +			hash = split[2];
> +		} else {
> +			hash = split[1];
> +		}
>   		/*
>   		 * strbuf_split_max left a space. Trim it and re-add
>   		 * it after abbreviation.
>   		 */
> -		strbuf_trim(split[1]);
> -		if (!repo_get_oid(the_repository, split[1]->buf, &oid)) {
> -			strbuf_reset(split[1]);
> -			strbuf_add_unique_abbrev(split[1], &oid,
> +		strbuf_trim(hash);
> +		if (!repo_get_oid(the_repository, hash->buf, &oid)) {
> +			strbuf_reset(hash);
> +			strbuf_add_unique_abbrev(hash, &oid,
>   						 DEFAULT_ABBREV);
> -			strbuf_addch(split[1], ' ');
> +			strbuf_addch(hash, ' ');
>   			strbuf_reset(line);
>   			for (i = 0; split[i]; i++)
>   				strbuf_addbuf(line, split[i]);
diff mbox series

Patch

diff --git a/wt-status.c b/wt-status.c
index 1da5732f57b..d11d9f9f142 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1342,9 +1342,11 @@  static int split_commit_in_progress(struct wt_status *s)
 
 /*
  * Turn
- * "pick d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some message"
+ * "pick d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some message" and
+ * "merge -C d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some-branch"
  * into
- * "pick d6a2f03 some message"
+ * "pick d6a2f03 some message" and
+ * "merge -C d6a2f03 some-branch"
  *
  * The function assumes that the line does not contain useless spaces
  * before or after the command.
@@ -1360,20 +1362,31 @@  static void abbrev_oid_in_line(struct strbuf *line)
 	    starts_with(line->buf, "l "))
 		return;
 
-	split = strbuf_split_max(line, ' ', 3);
+	split = strbuf_split_max(line, ' ', 4);
 	if (split[0] && split[1]) {
 		struct object_id oid;
-
+		struct strbuf *hash;
+
+		if ((!strcmp(split[0]->buf, "merge ") ||
+		     !strcmp(split[0]->buf, "m "    ) ||
+		     !strcmp(split[0]->buf, "fixup ") ||
+		     !strcmp(split[0]->buf, "f "    )) &&
+		    (!strcmp(split[1]->buf, "-C ") ||
+		     !strcmp(split[1]->buf, "-c "))) {
+			hash = split[2];
+		} else {
+			hash = split[1];
+		}
 		/*
 		 * strbuf_split_max left a space. Trim it and re-add
 		 * it after abbreviation.
 		 */
-		strbuf_trim(split[1]);
-		if (!repo_get_oid(the_repository, split[1]->buf, &oid)) {
-			strbuf_reset(split[1]);
-			strbuf_add_unique_abbrev(split[1], &oid,
+		strbuf_trim(hash);
+		if (!repo_get_oid(the_repository, hash->buf, &oid)) {
+			strbuf_reset(hash);
+			strbuf_add_unique_abbrev(hash, &oid,
 						 DEFAULT_ABBREV);
-			strbuf_addch(split[1], ' ');
+			strbuf_addch(hash, ' ');
 			strbuf_reset(line);
 			for (i = 0; split[i]; i++)
 				strbuf_addbuf(line, split[i]);