Re: [PATCH] sequencer: comment checked-out branch properly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 23, 2024 at 07:27:58PM +0200, kristofferhaugsbakk@xxxxxxxxxxxx wrote:
> From: Kristoffer Haugsbakk <code@xxxxxxxxxxxxxxx>
>
> `git rebase --update-ref` does not insert commands for dependent/sub-
> branches which are checked out.[1]  Instead it leaves a comment about
> that fact.  The comment char is hard-coded (#).  In turn the comment
> line gets interpreted as an invalid command when `core.commentChar`
> is in use.

Nice find. My first thought when reading was that this was a regression
from 8b311478ad (config: allow multi-byte core.commentChar, 2024-03-12).
But thinking about it for a moment that is definitely not true, as this
has probably never worked since core.commentChar was introduced, and has
nothing to do with what range of value(s) it does or doesn't support.

> † 1: 900b50c242 (rebase: add --update-refs option, 2022-07-19)
>
> Signed-off-by: Kristoffer Haugsbakk <code@xxxxxxxxxxxxxxx>
> ---
>  sequencer.c       |  5 +++--
>  t/t3400-rebase.sh | 16 ++++++++++++++++
>  2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 353d804999b..1b6fd86f70b 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -6382,8 +6382,9 @@ static int add_decorations_to_list(const struct commit *commit,
>  		/* If the branch is checked out, then leave a comment instead. */
>  		if ((path = branch_checked_out(decoration->name))) {
>  			item->command = TODO_COMMENT;
> -			strbuf_addf(ctx->buf, "# Ref %s checked out at '%s'\n",
> -				    decoration->name, path);
> +			strbuf_commented_addf(ctx->buf, comment_line_str,
> +					      "Ref %s checked out at '%s'\n",
> +					      decoration->name, path);

Makes sense, but the following command turns up a couple more results
even after applying:

    $ git grep -p 'strbuf_addf([^,]*, "#'
    sequencer.c=static void update_squash_message_for_fixup(struct strbuf *msg)
    sequencer.c:    strbuf_addf(&buf1, "# %s\n", _(first_commit_msg_str));
    sequencer.c:    strbuf_addf(&buf2, "# %s\n", _(skip_first_commit_msg_str));

I imagine that we would want similar treatment there as well, no?

> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
> index 09f230eefb2..f61a717b190 100755
> --- a/t/t3400-rebase.sh
> +++ b/t/t3400-rebase.sh
> @@ -456,4 +456,20 @@ test_expect_success 'rebase when inside worktree subdirectory' '
>  	)
>  '
>
> +test_expect_success 'git rebase --update-ref with core.commentChar and branch on worktree' '
> +	test_when_finished git branch -D base topic2 &&
> +	test_when_finished git checkout main &&
> +	test_when_finished git branch -D wt-topic &&
> +	test_when_finished git worktree remove wt-topic &&
> +	git checkout main &&
> +	git checkout -b base &&
> +	git checkout -b topic2 &&
> +	test_commit msg2 &&
> +	git worktree add wt-topic &&
> +	git checkout base &&
> +	test_commit msg3 &&
> +	git checkout topic2 &&
> +	git -c core.commentChar=% rebase --update-refs base
> +'
> +

Seems quite reasonable.

Thanks,
Taylor




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux