Re: [PATCH] sequencer: comment checked-out branch properly
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Oct 23, 2024, 18:44 UTC
- Message-ID
- <ZxlEJ+44M8z03VOj@nand.local>
- In-Reply-To
- <5267b9a9c8cc5cc66979117dc4c1e4d7329e2a03.1729704370.git.code@khaugsbakk.name>
On Wed, Oct 23, 2024 at 07:27:58PM +0200, kristofferhaugsbakk@fastmail.com wrote:
Show 7 quoted lines
> From: Kristoffer Haugsbakk <code@khaugsbakk.name> > > `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.
Show 21 quoted lines
> † 1: 900b50c242 (rebase: add --update-refs option, 2022-07-19)
>
> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
> ---
> 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?
Show 24 quoted lines
> 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