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
