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

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

 



Hi Kristoffer

On 25/11/2024 10:52, Kristoffer Haugsbakk wrote:
On Mon, Nov 25, 2024, at 11:07, phillip.wood123@xxxxxxxxx wrote:

Hi Phillip, thanks for the review!

You're welcome, thanks for fixing this

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

Oh, I see the same thing, I was sure we only showed the final message unless there were conflicts. I wonder if I've misremembered or the behavior has changed in any case that's outside the scope of this series.

Thanks

Phillip


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





[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