On Tue, Mar 12, 2024 at 02:36:36PM +0000, phillip.wood123@xxxxxxxxx wrote: > > Leading whitespace actually does work, though I think you'd be slightly > > insane to use it. > > For "git rebase" in only works if you edit the todo list with "git rebase > --edit-todo" which calls strbuf_stripspace() and therefore parse_insn_line() > never sees the comments. If you edit the todo list directly then it will > error out. You can see this with > > git -c core.commentChar=' ' rebase -x 'echo " this is a comment" > >>"$(git rev-parse --git-path rebase-merge/git-rebase-todo)"' HEAD^ > > which successfully picks HEAD but then gives > > error: invalid command 'this' > > when it tries to parse the todo list after the exec command is run. Given it > is broken already I'm not sure we should worry about it here. In any case it > is not clear how much we should worry about problems caused by users editing > the todo list without using "git rebase --edit-todo". There is code in > parse_insn_line() which is clearly there to handle direct editing of the > file but I don't think it is tested and directly editing the file probably > bypasses the rebase.missingCommitsCheck checks as well. Ah, thanks for the example. I guess it's not too surprising that it can cause confusion. Given that it's an existing issue, I think my preference would be to leave it out of the series under discussion (given how long and complicated it is already), but I'd have no objection to tightening things further on top as a separate series. -Peff
