On Mon, Nov 25, 2024, at 11:07, phillip.wood123@xxxxxxxxx wrote: > Hi Kristoffer > > Thanks for re-rolling, I've left some comments on the range-diff Hi Phillip, thanks for the review! I should be able to fix these and reroll today. > [...] > Stray "`" > >> + that hash3’s commit message should be used for the resulting commit. >> + So the user is presented with an editor where the two first commit >> + messages are commented out and the third is not. > > I'd perhaps say > > If there are conflicts when applying commit hash3 then the user is > presented ... > > as we only show all the messages to the user when there are conflicts. I use `fixup -c` for the third/last commit here. So I am prompted to edit the commit message. I tested this on this series: git checkout --detach kh/sequencer-comment-char git rebase -i fd3785337beb285ed7fd67ce6fc3d3bed2097b40 Which gave me [this] editor without these changes (with `core.commentChar` set to `%`). > >> However this does >> + not work if `core.commentChar`/`core.commentString` is in use since >> + the comment char is hardcoded (#) in this `sequencer.c` function. >> + As a result the first commit message will not be commented out. >> + >> + † 1: See 9e3cebd97cb (rebase -i: add fixup [-C | -c] command, >> + 2021-01-29) >> + >> + Signed-off-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> >> Co-authored-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> >> + Reported-by: Taylor Blau <me@xxxxxxxxxxxx> >> Signed-off-by: Kristoffer Haugsbakk <code@xxxxxxxxxxxxxxx> > > Thanks for updating the trailers, they look good to me > > Best Wishes > > Phillip † this: % This is a combination of 3 commits. % This is the 1st commit message: sequencer: comment checked-out branch properly `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 hardcoded (#). In turn the comment line gets interpreted as an invalid command when `core.commentChar`/ `core.commentString` is in use. † 1: See 900b50c242 (rebase: add --update-refs option, 2022-07-19) Signed-off-by: Kristoffer Haugsbakk <code@xxxxxxxxxxxxxxx> % The commit message #2 will be skipped: % sequencer: comment `--reference` subject line properly % % `git revert --reference <commit>` leaves behind a comment in the % first line:[1] % % # *** SAY WHY WE ARE REVERTING ON THE TITLE LINE *** % % Meaning that the commit will just consist of the next line if the user % exits the editor directly: % % This reverts commit <--format=reference commit> % % But the comment char here is hardcoded (#). Which means that the % comment line will inadvertently be included in the commit message if % `core.commentChar`/`core.commentString` is in use. % % † 1: See 43966ab3156 (revert: optionally refer to commit in the % "reference" format, 2022-05-26) % % Signed-off-by: Kristoffer Haugsbakk <code@xxxxxxxxxxxxxxx> % This is the commit message #3: sequencer: comment commit messages properly The rebase todo editor has commands like `fixup -c` which affects the commit messages of the rebased commits.[1] For example: pick hash1 <msg> fixup hash2 <msg> fixup -c hash3 <msg> This says that hash2` and hash3 should be squashed into hash1 and that hash3’s commit message should be used for the resulting commit. So the user is presented with an editor where the two first commit messages are commented out and the third is not. However this does not work if `core.commentChar`/`core.commentString` is in use since the comment char is hardcoded (#) in this `sequencer.c` function. As a result the first commit message will not be commented out. † 1: See 9e3cebd97cb (rebase -i: add fixup [-C | -c] command, 2021-01-29) Signed-off-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> Co-authored-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> Reported-by: Taylor Blau <me@xxxxxxxxxxxx> Signed-off-by: Kristoffer Haugsbakk <code@xxxxxxxxxxxxxxx> % Please enter the commit message for your changes. Lines starting