sequencer: make it clearer that commit descriptions are just comments

PATCH Superseded Elijah Newren via GitGitGadget 6 participants 11 messages May 15, 2025 – May 16, 2025

lore

Share & tools

lore

Elijah Newren via GitGitGadget

author

2025-05-15 02:20 UTC #0 lore raw

PATCH 1/1 sequencer: make it clearer that commit descriptions are just comments

From: Elijah Newren <[email protected]>

Every once in a while, users report that editing the commit summaries in the todo list does not get reflected in the rebase operation, suggesting that users are (a) only using one-line commit messages, and
(b) not understanding that the commit summaries are merely helpful comments to help them find the right hashes.

It may be difficult to correct users' poor commit messages, but we can at least try to make it clearer that the commit summaries are not directives of some sort by inserting a comment character. Hopefully that leads to them looking a little further and noticing the hints at the bottom to use 'reword' or 'edit' directives.

Yes, this change may look funny at first since it hardcodes '#' rather than using comment_line_str. However:

* comment_line_str exists to allow disambiguation between lines in a commit message and lines that are instructions to users editing the commit message. No such disambiguation is needed for these comments that occur on the same line after existing directives
* the exact "comment" character(s) on regular pick lines used aren't actually important; I could have used anything, including completely random variable length text for each line and it'd work because we ignore everything after 'pick' and the hash.
* The whole point of this change is to signal to users that they should NOT be editing any part of the line after the hash (and if they do so, their edits will be ignored), while the whole point of comment_line_str is to allow highly flexible editing. So making it more general by using comment_line_str actually feels counterproductive.
* The character for merge directives absolutely must be '#'; that has been deeply hardcoded for a long time (see below), and will break if some other comment character is used instead. In a desire to have pick and merge directives be similar, I use the same comment character for both.
* Perhaps merge directives could be fixed to not be inflexible about the comment character used, if someone feels highly motivated, but I think that should be done in a separate follow-on patch.

Here are (some of?) the locations where '#' has already been hardcoded for a long time for merges:

1) In check_label_or_ref_arg():

	case TODO_LABEL:
		/*
		 * '#' is not a valid label as the merge command uses it to
		 * separate merge parents from the commit subject.
		 */

2) In do_merge():

	/*
	 * For octopus merges, the arg starts with the list of revisions to be
	 * merged. The list is optionally followed by '#' and the oneline.
	 */
	merge_arg_len = oneline_offset = arg_len;
	for (p = arg; p - arg < arg_len; p += strspn(p, " \t\n")) {
		if (!*p)
			break;
		if (*p == '#' && (!p[1] || isspace(p[1]))) {

3) In label_oid():

		if ((buf->len == the_hash_algo->hexsz &&
		     !get_oid_hex(label, &dummy)) ||
		    (buf->len == 1 && *label == '#') ||
		    hashmap_get_from_hash(&state->labels,
					  strihash(label), label)) {
			/*
			 * If the label already exists, or if the label is a
			 * valid full OID, or the label is a '#' (which we use
			 * as a separator between merge heads and oneline), we
			 * append a dash and a number to make it unique.
			 */

Signed-off-by Elijah Newren <[email protected]>

---
sequencer: make it clearer that commit descriptions are just comments

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1923%2Fnewren%2Fsequencer-todo-commit-msgs-are-comments-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1923/newren/sequencer-todo-commit-msgs-are-comments-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1923

 sequencer.c                   | 16 +++++--
 t/t3404-rebase-interactive.sh | 54 +++++++++++-----------
 t/t3415-rebase-autosquash.sh  | 14 +++---
 t/t3430-rebase-merges.sh      | 10 ++--
 t/t5520-pull.sh               |  2 +-
 t/t7512-status-help.sh        | 86 +++++++++++++++++------------------
 6 files changed, 94 insertions(+), 88 deletions(-)

sequencer.c +11 -5

index b5c4043757e..2735966544b 100644
@@ -5901,11 +5901,11 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 
 		/* Create a label from the commit message */
 		strbuf_reset(&label_from_message);
-		if (skip_prefix(oneline.buf, "Merge ", &p1) &&
+		if (skip_prefix(oneline.buf, "# Merge ", &p1) &&
 		    (p1 = strchr(p1, '\'')) &&
 		    (p2 = strchr(++p1, '\'')))
 			strbuf_add(&label_from_message, p1, p2 - p1);
-		else if (skip_prefix(oneline.buf, "Merge pull request ",
+		else if (skip_prefix(oneline.buf, "# Merge pull request ",
 				     &p1) &&
 			 (p1 = strstr(p1, " from ")))
 			strbuf_addstr(&label_from_message, p1 + strlen(" from "));

@@ -5940,7 +5940,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 
 			strbuf_addstr(&buf, label_oid(oid, label, &state));
 		}
-		strbuf_addf(&buf, " # %s", oneline.buf);
+		strbuf_addf(&buf, " %s", oneline.buf);
 
 		FLEX_ALLOC_STR(entry, string, buf.buf);
 		oidcpy(&entry->entry.oid, &commit->object.oid);

@@ -6022,7 +6022,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 			else {
 				strbuf_reset(&oneline);
 				pretty_print_commit(pp, commit, &oneline);
-				strbuf_addf(out, "%s %s # %s\n",
+				strbuf_addf(out, "%s %s %s\n",
 					    cmd_reset, to, oneline.buf);
 			}
 		}

@@ -6090,8 +6090,14 @@ int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,
 	git_config_get_string("rebase.instructionFormat", &format);
 	if (!format || !*format) {
 		free(format);
-		format = xstrdup("%s");
+		format = xstrdup("# %s");
 	}
+	if (*format != '#') {
+		char *temp = format;
+		format = xstrfmt("# %s", temp);
+		free(temp);
+	}
+
 	get_commit_format(format, &revs);
 	free(format);
 	pp.fmt = revs.commit_format;

t/t3404-rebase-interactive.sh +27 ~~-27~~

116 lines not on this page · raw message

t/t3415-rebase-autosquash.sh +7 -7

index fcc40d6fe1f..26b42a526a1 100755
@@ -257,8 +257,8 @@ test_expect_success 'auto squash of fixup commit that matches branch name which
 	GIT_SEQUENCE_EDITOR="cat >tmp" git rebase --autosquash -i HEAD^^ &&
 	sed -ne "/^[^#]/{s/[0-9a-f]\{7,\}/HASH/g;p;}" tmp >actual &&
 	cat <<-EOF >expect &&
-	pick HASH second commit
-	pick HASH fixup! self-cycle # empty
+	pick HASH # second commit
+	pick HASH # fixup! self-cycle # empty
 	EOF
 	test_cmp expect actual
 '

@@ -311,10 +311,10 @@ test_auto_fixup_fixup () {
 		parent2=$(git rev-parse --short HEAD^^) &&
 		parent3=$(git rev-parse --short HEAD^^^) &&
 		cat >expected <<-EOF &&
-		pick $parent3 first commit
-		$1 $parent1 $1! first
-		$1 $head $1! $2! first
-		pick $parent2 second commit
+		pick $parent3 # first commit
+		$1 $parent1 # $1! first
+		$1 $head # $1! $2! first
+		pick $parent2 # second commit
 		EOF
 		test_cmp expected actual
 	) &&

@@ -389,7 +389,7 @@ test_expect_success 'autosquash with empty custom instructionFormat' '
 		set_cat_todo_editor &&
 		test_must_fail git -c rebase.instructionFormat= \
 			rebase --autosquash  --force-rebase -i HEAD^ >actual &&
-		git log -1 --format="pick %h %s" >expect &&
+		git log -1 --format="pick %h # %s" >expect &&
 		test_cmp expect actual
 	)
 '

t/t3430-rebase-merges.sh +5 -5

index b84d68c4b96..5f8fa05420c 100755
@@ -106,18 +106,18 @@ test_expect_success 'generate correct todo list' '
 	label onto
 
 	reset onto
-	pick $b B
+	pick $b # B
 	label first
 
 	reset onto
-	pick $c C
+	pick $c # C
 	label branch-point
-	pick $f F
-	pick $g G
+	pick $f # F
+	pick $g # G
 	label second
 
 	reset branch-point # C
-	pick $d D
+	pick $d # D
 	merge -C $e first # E
 	merge -C $h second # H

t/t5520-pull.sh +1 -1

index 47534f1062d..52a5e4c016b 100755
@@ -813,7 +813,7 @@ test_expect_success 'git pull --rebase does not reapply old patches' '
 		cd dst &&
 		test_must_fail git pull --rebase &&
 		cat .git/rebase-merge/done .git/rebase-merge/git-rebase-todo >work &&
-		grep -v -e \# -e ^$ work >patches &&
+		sed -e s/#.*// work | grep -v ^$ >patches &&
 		test_line_count = 1 patches &&
 		rm -f work
 	)

t/t7512-status-help.sh +43 ~~-43~~

203 lines not on this page · raw message

base-commit: 1a8a4971cc6c179c4dd711f4a7f5d7178f4b3ab7

Signature · 1 line

gitgitgadget

Reto

2025-05-15 04:48 UTC #1 lore raw

Quoted 5 lines · Elijah Newren via GitGitGadget

Every once in a while, users report that editing the commit summaries in the todo list does not get reflected in the rebase operation, suggesting that users are (a) only using one-line commit messages, and
(b) not understanding that the commit summaries are merely helpful comments to help them find the right hashes.

As with everything in git, you can learn that this is the case. You could as well just add a explanatory comment to the footer.

Quoted 5 lines

It may be difficult to correct users' poor commit messages, but we can at least try to make it clearer that the commit summaries are not directives of some sort by inserting a comment character. Hopefully that leads to them looking a little further and noticing the hints at the bottom to use 'reword' or 'edit' directives.

For fancy things/editors that recognize comments, this will *dim* the commit messages, to light grey or such.
This is decidedly not what I'd like to happen at least. The commit messages there are my primary way of navigating the commit, given that I'm not learning the commit shas by hard ;)

While I appreciate the motivation, I don't think the comment string is a good approach here.

Cheers,
Reto

Phillip Wood

2025-05-15 10:16 UTC #2 lore raw

Hi Reto

Quoted 8 lines · Reto

Quoted 5 lines · Elijah Newren via GitGitGadget
It may be difficult to correct users' poor commit messages, but we can at least try to make it clearer that the commit summaries are not directives of some sort by inserting a comment character. Hopefully that leads to them looking a little further and noticing the hints at the bottom to use 'reword' or 'edit' directives.

Quoted 5 lines

For fancy things/editors that recognize comments, this will *dim* the commit messages, to light grey or such.
This is decidedly not what I'd like to happen at least. The commit messages there are my primary way of navigating the commit, given that I'm not learning the commit shas by hard ;)

As the commit message makes clear we already insert a '#' between the parents and subject line of a merge command. We also append '# empty' to empty commits. If there are editors that erroneously treat a '#' anywhere in the line as a comment they're already dimming things they shouldn't. A line in the todo list is only a comment if it starts with core.commentString which defaults to '#'. Editors that unconditionally treat '#' as the start of a comment are buggy.

Quoted 2 lines

While I appreciate the motivation, I don't think the comment string is a good approach here.

As the commit message points out strictly speaking we're not using the comment string we using a fixed single character.

Best Wishes

Phillip

Phillip Wood

2025-05-15 10:19 UTC #3 lore raw

Sorry I forgot to Cc the original author in the message below.

Quoted 28 lines · Phillip Wood

Hi Reto
Quoted 8 lines · Reto
Quoted 5 lines · Elijah Newren via GitGitGadget
It may be difficult to correct users' poor commit messages, but we can at least try to make it clearer that the commit summaries are not directives of some sort by inserting a comment character. Hopefully that leads to them looking a little further and noticing the hints at the bottom to use 'reword' or 'edit' directives.
Quoted 5 lines
For fancy things/editors that recognize comments, this will *dim* the commit messages, to light grey or such.
This is decidedly not what I'd like to happen at least. The commit messages there are my primary way of navigating the commit, given that I'm not learning the commit shas by hard ;)
As the commit message makes clear we already insert a '#' between the parents and subject line of a merge command. We also append '# empty' to empty commits. If there are editors that erroneously treat a '#' anywhere in the line as a comment they're already dimming things they shouldn't. A line in the todo list is only a comment if it starts with core.commentString which defaults to '#'. Editors that unconditionally treat '#' as the start of a comment are buggy.
Quoted 2 lines
While I appreciate the motivation, I don't think the comment string is a good approach here.
As the commit message points out strictly speaking we're not using the comment string we using a fixed single character.
Best Wishes
Phillip

Phillip Wood

2025-05-15 13:03 UTC #4 lore raw

In reply to Elijah Newren via GitGitGadget

Hi Elijah

Quoted 67 lines · Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>
Every once in a while, users report that editing the commit summaries in the todo list does not get reflected in the rebase operation, suggesting that users are (a) only using one-line commit messages, and
(b) not understanding that the commit summaries are merely helpful comments to help them find the right hashes.
It may be difficult to correct users' poor commit messages, but we can at least try to make it clearer that the commit summaries are not directives of some sort by inserting a comment character. Hopefully that leads to them looking a little further and noticing the hints at the bottom to use 'reword' or 'edit' directives.
Yes, this change may look funny at first since it hardcodes '#' rather than using comment_line_str. However:
* comment_line_str exists to allow disambiguation between lines in a commit message and lines that are instructions to users editing the commit message. No such disambiguation is needed for these comments that occur on the same line after existing directives
* the exact "comment" character(s) on regular pick lines used aren't actually important; I could have used anything, including completely random variable length text for each line and it'd work because we ignore everything after 'pick' and the hash.
* The whole point of this change is to signal to users that they should NOT be editing any part of the line after the hash (and if they do so, their edits will be ignored), while the whole point of comment_line_str is to allow highly flexible editing. So making it more general by using comment_line_str actually feels counterproductive.
* The character for merge directives absolutely must be '#'; that has been deeply hardcoded for a long time (see below), and will break if some other comment character is used instead. In a desire to have pick and merge directives be similar, I use the same comment character for both.
* Perhaps merge directives could be fixed to not be inflexible about the comment character used, if someone feels highly motivated, but I think that should be done in a separate follow-on patch.
Here are (some of?) the locations where '#' has already been hardcoded for a long time for merges:
   1) In check_label_or_ref_arg():
	case TODO_LABEL:
		/*
		 * '#' is not a valid label as the merge command uses it to
		 * separate merge parents from the commit subject.
		 */

   2) In do_merge():

	/*
	 * For octopus merges, the arg starts with the list of revisions to be
	 * merged. The list is optionally followed by '#' and the oneline.
	 */
	merge_arg_len = oneline_offset = arg_len;
	for (p = arg; p - arg < arg_len; p += strspn(p, " \t\n")) {
		if (!*p)
			break;
		if (*p == '#' && (!p[1] || isspace(p[1]))) {

   3) In label_oid():

		if ((buf->len == the_hash_algo->hexsz &&
		     !get_oid_hex(label, &dummy)) ||
		    (buf->len == 1 && *label == '#') ||

Quoted 8 lines

		    hashmap_get_from_hash(&state->labels,
					  strihash(label), label)) {
			/*
			 * If the label already exists, or if the label is a
			 * valid full OID, or the label is a '#' (which we use
			 * as a separator between merge heads and oneline), we
			 * append a dash and a number to make it unique.
			 */

Thanks for the comprehensive commit message. I think this change is a very good idea. The code changes all look good to me. I've left one comment on the tests below though I should admit that I could feel my eyes glazing over by the time I got to the end of all the test changes. The test changes are pretty much all mechanical though so I'm reasonably confidant that the tests would fail if there was something wrong with the conversion.

t/t5520-pull.sh test_expect_success 'git pull --rebase does not reapply old patches' ' · line 816

Quoted 10 lines

t/t5520-pull.sh +1 -1

index 47534f1062d..52a5e4c016b 100755
@@ -813,7 +813,7 @@ test_expect_success 'git pull --rebase does not reapply old patches' '
  		cd dst &&
  		test_must_fail git pull --rebase &&
  		cat .git/rebase-merge/done .git/rebase-merge/git-rebase-todo >work &&
-		grep -v -e \# -e ^$ work >patches &&
+		sed -e s/#.*// work | grep -v ^$ >patches &&

I think we could just use 'sed -n "!/^#/p work >patches &&" here

Thanks

Phillip

Junio C Hamano

2025-05-15 13:58 UTC #9 lore raw

In reply to Elijah Newren via GitGitGadget

Quoted 9 lines · Elijah Newren via GitGitGadget

suggesting that users are (a) only using one-line commit messages, and
(b) not understanding that the commit summaries are merely helpful comments to help them find the right hashes.
It may be difficult to correct users' poor commit messages, but we can at least try to make it clearer that the commit summaries are not directives of some sort by inserting a comment character. Hopefully that leads to them looking a little further and noticing the hints at the bottom to use 'reword' or 'edit' directives.

Cute and clever.

t/t7512-status-help.sh test_expect_success 'status when splitting a commit' ' · line 240

Quoted 9 lines

Last commands done (2 commands done):

t/t7512-status-help.sh +3 -3

-   pick $COMMIT2 two_split
-   edit $COMMIT3 three_split
+   pick $COMMIT2 # two_split
+   edit $COMMIT3 # three_split
 Next command to do (1 remaining command):
-   pick $COMMIT4 four_split
+   pick $COMMIT4 # four_split
   (use "git rebase --edit-todo" to view and edit)

These show the intentions quite well. I think all places the addition of '#' is done at the output layer, and the matching of commits for the purpose of --autosquash should be done solely using the internal representation without '#', this change should not break anything, I would think, that we internally have.

Breaking custom todo-editors the users may have is a different story, but if they are somehow matching against the first line of the commit log message when there prominently is a short-but-unique commit object name, they are simply crazy and not worth worrying about, I would say ;-)

Junio C Hamano

2025-05-15 15:56 UTC #5 lore raw

In reply to Phillip Wood

Quoted 4 lines · Phillip Wood

t/t5520-pull.sh test_expect_success 'git pull --rebase does not reapply old patches' ' · line 816
Quoted 2 lines
t/t5520-pull.sh +1 -1
-		grep -v -e \# -e ^$ work >patches &&
+		sed -e s/#.*// work | grep -v ^$ >patches &&
I think we could just use 'sed -n "!/^#/p work >patches &&" here

Or even "sed -e '/^#/d' -e '/^$/d'" ?

Ben Knoble

2025-05-16 14:07 UTC #6 lore raw

Quoted 12 lines · Junio C Hamano

Quoted 4 lines · Phillip Wood
t/t5520-pull.sh test_expect_success 'git pull --rebase does not reapply old patches' ' · line 816
Quoted 2 lines
t/t5520-pull.sh +1 -1
-        grep -v -e \# -e ^$ work >patches &&
+        sed -e s/#.*// work | grep -v ^$ >patches &&
I think we could just use 'sed -n "!/^#/p work >patches &&" here
Or even "sed -e '/^#/d' -e '/^$/d'" ?

I’m probably missing something obvious, but why step away from "grep -v -e ^\# -e ^$"? A one character change from the original ;) If we must go with Sed, I’d prefer Junio’s construct (combining, with obvious semantics via "d") over the sed|grep pipeline.

Elijah Newren

2025-05-16 15:22 UTC #7 lore raw

Quoted 5 lines · Ben Knoble

Quoted 5 lines · Junio C Hamano
Phillip Wood <[email protected]> writes:

Quoted 8 lines

Quoted 6 lines
Quoted 4 lines
t/t5520-pull.sh test_expect_success 'git pull --rebase does not reapply old patches' ' · line 816
Quoted 2 lines
t/t5520-pull.sh +1 -1
-        grep -v -e \# -e ^$ work >patches &&
+        sed -e s/#.*// work | grep -v ^$ >patches &&
I think we could just use 'sed -n "!/^#/p work >patches &&" here
Or even "sed -e '/^#/d' -e '/^$/d'" ?
I’m probably missing something obvious, but why step away from "grep -v -e ^\# -e ^$"? A one character change from the original ;) If we must go with Sed, I’d prefer Junio’s construct (combining, with obvious semantics via "d") over the sed|grep pipeline.

:-) I made this exact change to make it be a one-character modification of the original, and pushed it up yesterday
(https://github.com/gitgitgadget/git/pull/1923); I just hadn't /submit'ted it to the list yet. I agree that every suggestion folks have made so far is better than my original proposal, but I kind of prefer this latest suggestion of just making a one-character modification. So, unless folks object, I'll indeed go with this one.

Junio C Hamano

2025-05-16 16:21 UTC #8 lore raw

In reply to Ben Knoble

Quoted 4 lines · Ben Knoble

Quoted 4 lines · Junio C Hamano
Phillip Wood <[email protected]> writes:

Quoted 8 lines

Quoted 6 lines
Quoted 4 lines
t/t5520-pull.sh test_expect_success 'git pull --rebase does not reapply old patches' ' · line 816
Quoted 2 lines
t/t5520-pull.sh +1 -1
-        grep -v -e \# -e ^$ work >patches &&
+        sed -e s/#.*// work | grep -v ^$ >patches &&
I think we could just use 'sed -n "!/^#/p work >patches &&" here
Or even "sed -e '/^#/d' -e '/^$/d'" ?
I’m probably missing something obvious, but why step away from "grep -v -e ^\# -e ^$"? A one character change from the original ;) If we must go with Sed, I’d prefer Junio’s construct (combining, with obvious semantics via "d") over the sed|grep pipeline.

Mine was a spinal reflex against piping sed and grep; I do not see a reason why "grep -v" with two patterns like you gave should not work---that is a good way to remove the pipe, too.

Thanks.

Elijah Newren via GitGitGadget

author

2025-05-16 16:26 UTC #10 lore raw

PATCH v2 1/1 sequencer: make it clearer that commit descriptions are just comments

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

Yes, this change may look funny at first since it hardcodes '#' rather than using comment_line_str. However:

Here are (some of?) the locations where '#' has already been hardcoded for a long time for merges:

1) In check_label_or_ref_arg():

	case TODO_LABEL:
		/*
		 * '#' is not a valid label as the merge command uses it to
		 * separate merge parents from the commit subject.
		 */

2) In do_merge():

	/*
	 * For octopus merges, the arg starts with the list of revisions to be
	 * merged. The list is optionally followed by '#' and the oneline.
	 */
	merge_arg_len = oneline_offset = arg_len;
	for (p = arg; p - arg < arg_len; p += strspn(p, " \t\n")) {
		if (!*p)
			break;
		if (*p == '#' && (!p[1] || isspace(p[1]))) {

3) In label_oid():

		if ((buf->len == the_hash_algo->hexsz &&
		     !get_oid_hex(label, &dummy)) ||
		    (buf->len == 1 && *label == '#') ||
		    hashmap_get_from_hash(&state->labels,
					  strihash(label), label)) {
			/*
			 * If the label already exists, or if the label is a
			 * valid full OID, or the label is a '#' (which we use
			 * as a separator between merge heads and oneline), we
			 * append a dash and a number to make it unique.
			 */

Signed-off-by Elijah Newren <[email protected]>

---
sequencer: make it clearer that commit descriptions are just comments

Changes since v1:

* Simplified the change to t5520 down to a single character change

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1923%2Fnewren%2Fsequencer-todo-commit-msgs-are-comments-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1923/newren/sequencer-todo-commit-msgs-are-comments-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1923

Range-diff vs v1:

1: e5f3d7059e4 ! 1: 5d290203d62 sequencer: make it clearer that commit descriptions are just comments

     @@ t/t5520-pull.sh: test_expect_success 'git pull --rebase does not reapply old pat
       		test_must_fail git pull --rebase &&
       		cat .git/rebase-merge/done .git/rebase-merge/git-rebase-todo >work &&
      -		grep -v -e \# -e ^$ work >patches &&
     -+		sed -e s/#.*// work | grep -v ^$ >patches &&
     ++		grep -v -e ^\# -e ^$ work >patches &&
       		test_line_count = 1 patches &&
       		rm -f work
       	)

 sequencer.c                   | 16 +++++--
 t/t3404-rebase-interactive.sh | 54 +++++++++++-----------
 t/t3415-rebase-autosquash.sh  | 14 +++---
 t/t3430-rebase-merges.sh      | 10 ++--
 t/t5520-pull.sh               |  2 +-
 t/t7512-status-help.sh        | 86 +++++++++++++++++------------------
 6 files changed, 94 insertions(+), 88 deletions(-)

sequencer.c +11 -5

index b5c4043757e..2735966544b 100644
@@ -5901,11 +5901,11 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 
 		/* Create a label from the commit message */
 		strbuf_reset(&label_from_message);
-		if (skip_prefix(oneline.buf, "Merge ", &p1) &&
+		if (skip_prefix(oneline.buf, "# Merge ", &p1) &&
 		    (p1 = strchr(p1, '\'')) &&
 		    (p2 = strchr(++p1, '\'')))
 			strbuf_add(&label_from_message, p1, p2 - p1);
-		else if (skip_prefix(oneline.buf, "Merge pull request ",
+		else if (skip_prefix(oneline.buf, "# Merge pull request ",
 				     &p1) &&
 			 (p1 = strstr(p1, " from ")))
 			strbuf_addstr(&label_from_message, p1 + strlen(" from "));

@@ -5940,7 +5940,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 
 			strbuf_addstr(&buf, label_oid(oid, label, &state));
 		}
-		strbuf_addf(&buf, " # %s", oneline.buf);
+		strbuf_addf(&buf, " %s", oneline.buf);
 
 		FLEX_ALLOC_STR(entry, string, buf.buf);
 		oidcpy(&entry->entry.oid, &commit->object.oid);

@@ -6022,7 +6022,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 			else {
 				strbuf_reset(&oneline);
 				pretty_print_commit(pp, commit, &oneline);
-				strbuf_addf(out, "%s %s # %s\n",
+				strbuf_addf(out, "%s %s %s\n",
 					    cmd_reset, to, oneline.buf);
 			}
 		}

@@ -6090,8 +6090,14 @@ int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,
 	git_config_get_string("rebase.instructionFormat", &format);
 	if (!format || !*format) {
 		free(format);
-		format = xstrdup("%s");
+		format = xstrdup("# %s");
 	}
+	if (*format != '#') {
+		char *temp = format;
+		format = xstrfmt("# %s", temp);
+		free(temp);
+	}
+
 	get_commit_format(format, &revs);
 	free(format);
 	pp.fmt = revs.commit_format;

t/t3404-rebase-interactive.sh +27 ~~-27~~

116 lines not on this page · raw message

t/t3415-rebase-autosquash.sh +7 -7

index fcc40d6fe1f..26b42a526a1 100755
@@ -257,8 +257,8 @@ test_expect_success 'auto squash of fixup commit that matches branch name which
 	GIT_SEQUENCE_EDITOR="cat >tmp" git rebase --autosquash -i HEAD^^ &&
 	sed -ne "/^[^#]/{s/[0-9a-f]\{7,\}/HASH/g;p;}" tmp >actual &&
 	cat <<-EOF >expect &&
-	pick HASH second commit
-	pick HASH fixup! self-cycle # empty
+	pick HASH # second commit
+	pick HASH # fixup! self-cycle # empty
 	EOF
 	test_cmp expect actual
 '

@@ -311,10 +311,10 @@ test_auto_fixup_fixup () {
 		parent2=$(git rev-parse --short HEAD^^) &&
 		parent3=$(git rev-parse --short HEAD^^^) &&
 		cat >expected <<-EOF &&
-		pick $parent3 first commit
-		$1 $parent1 $1! first
-		$1 $head $1! $2! first
-		pick $parent2 second commit
+		pick $parent3 # first commit
+		$1 $parent1 # $1! first
+		$1 $head # $1! $2! first
+		pick $parent2 # second commit
 		EOF
 		test_cmp expected actual
 	) &&

@@ -389,7 +389,7 @@ test_expect_success 'autosquash with empty custom instructionFormat' '
 		set_cat_todo_editor &&
 		test_must_fail git -c rebase.instructionFormat= \
 			rebase --autosquash  --force-rebase -i HEAD^ >actual &&
-		git log -1 --format="pick %h %s" >expect &&
+		git log -1 --format="pick %h # %s" >expect &&
 		test_cmp expect actual
 	)
 '

t/t3430-rebase-merges.sh +5 -5

index b84d68c4b96..5f8fa05420c 100755
@@ -106,18 +106,18 @@ test_expect_success 'generate correct todo list' '
 	label onto
 
 	reset onto
-	pick $b B
+	pick $b # B
 	label first
 
 	reset onto
-	pick $c C
+	pick $c # C
 	label branch-point
-	pick $f F
-	pick $g G
+	pick $f # F
+	pick $g # G
 	label second
 
 	reset branch-point # C
-	pick $d D
+	pick $d # D
 	merge -C $e first # E
 	merge -C $h second # H

t/t5520-pull.sh +1 -1

index 47534f1062d..63c9a8f04b1 100755
@@ -813,7 +813,7 @@ test_expect_success 'git pull --rebase does not reapply old patches' '
 		cd dst &&
 		test_must_fail git pull --rebase &&
 		cat .git/rebase-merge/done .git/rebase-merge/git-rebase-todo >work &&
-		grep -v -e \# -e ^$ work >patches &&
+		grep -v -e ^\# -e ^$ work >patches &&
 		test_line_count = 1 patches &&
 		rm -f work
 	)

t/t7512-status-help.sh +43 ~~-43~~

203 lines not on this page · raw message

base-commit: 1a8a4971cc6c179c4dd711f4a7f5d7178f4b3ab7

Signature · 1 line

gitgitgadget