Re: [PATCH v3 0/3] sequencer: comment out properly in todo list

From: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
Date: Nov 25, 2024, 10:52 UTC
Message-ID: <3bd7cd08-61b6-4b57-a293-1c94eb3529d7@app.fastmail.com>
In-Reply-To: <7739a6e2-8758-4d0f-b1d6-f0879a89590f@gmail.com>

On Mon, Nov 25, 2024, at 11:07, phillip.wood123@gmail.com 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.

Show 13 quoted lines

> [...]
> 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 `%`).

Show 19 quoted lines

>
>> 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@dunelm.org.uk>
>>           Co-authored-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>>      +    Reported-by: Taylor Blau <me@ttaylorr.com>
>>           Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> 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@khaugsbakk.name>

    % 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@khaugsbakk.name>

    % 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@dunelm.org.uk>
    Co-authored-by: Phillip Wood <phillip.wood@dunelm.org.uk>
    Reported-by: Taylor Blau <me@ttaylorr.com>
    Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>

    % Please enter the commit message for your changes. Lines starting