From: Daniel Harding Date: Tue, 10 Jul 2018 13:24:19 GMT Subject: Re: [PATCH 0/2] Fix --rebase-merges with custom commentChar Message-ID: In-Reply-To: On Mon, 09 Jul 2018 at 10:53:14 +0300, Johannes Schindelin wrote> > On Sun, 8 Jul 2018, Daniel Harding wrote: > >> I have core.commentChar set in my .gitconfig, and when I tried to run >> git rebase -i -r, I received an error message like the following: >> >> error: invalid line 3: # Branch >> >> To fix this, I updated sequencer.c to use the configured commentChar >> for the Branch comments. I also tweaked the tests in t3430 to >> verify todo list generation with a custom commentChar. I'm not sure >> if I took the right approach with that, or if it would be better to >> add additional tests for that case, so feel free to >> tweak/replace/ignore the second commit as appropriate. > > Nothing is as powerful as an idea whose time has come. Or as a patch whose > time has come, I guess: > > https://public-inbox.org/git/20180628020414.25036-1-aaron@schrab.com/ Oops, I should have done a bit a searching before I tossed off a patch. Thanks Johannes for the pointer. > AFAICT the remaining task was to send a new revision of the patch, with > the commit message touched up, to reflect the analysis that it handles the > `auto` setting well. > > Your patch adds a regression test in addition, which is very nice. > > So maybe you can coordinate with Aaron about that first patch? I really > think that the commit message needs to explain why the `auto` setting is > not a problem here. Aaron, how would you like to move forward on this? I don't want to take credit from you since you were the first to post the patch. If you would like to post a new version of your patch with the commit message updated based on the feedback, I can then add my tests to go with it. Alternatively if you'd like me to run with this I can repost the patch with you as the author along with an updated commit message and my name in a "Commit-message-by:" line. Let me know your thoughts. If I don't hear from you in a couple of days, I'll go ahead and repost the patch as I described. Thanks, Daniel Harding