Re: [PATCH v2] rebase -i: do leave commit message intact in fixup! chains

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

 



Hi Martin,

On Tue, 12 Jan 2021, Martin Ågren wrote:

> Johannes Schindelin via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote:
> >
> > We actually need to actively suppress that clean-up lest a configured
> > `commit.cleanup` may interfere with what we want to do: leave the commit
> > message unchanged.
>
> >     Changes since v1:>
> >      * The fix now works even if commit.cleanup = commit
>
> Indeed. FWIW, this patch looks good to me.
>
> There is the lone remaining user of `CLEANUP_MSG` where we're handling
> the skipping of the final fixup in a chain and which is part of a larger
> block of code to handle various cases like that around `git rebase
> --skip`. I wrote some tests on top of your patch to see what happens and
> try to understand what's going on. The tests are below.
>
> I can't say I grok all that's going on in the implementation. By the
> time we're finalizing the commit message, it seems to me that we've lost
> track of which of the lines that look like comments are indeed comment
> lines added by us and which actually originate in the commit messages
> we're trying to rebase and which just happen to begin with a comment
> character.

Indeed, we lost that information. For what it's worth, that's totally in
line with how `--amend` works:

	git commit --allow-empty -m '#hash'
	git commit --allow-empty --amend

This will interpret the `#hash` line as a comment.

And since that's the case with `git commit`, I would contend that this is
a different issue than the one I am trying to address in this patch
series, and therefore should not be handled here (not even adding the
`test_expect_failure` cases you generously provided).

Ciao,
Dscho

[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