Hi Michael, On Mon, 18 Dec 2023, Michael Lohmann wrote: > I wanted to align rebase and revert/cherry-pick a bit more (for the > latter I am currently finishing my patch for --show-current-patch and > then looked into possibly implementing --edit-todo). To avoid code > duplication I wanted to reuse the existing interactive-rebase code as > much as possible and ended up at the todo script parsing in the > sequencer. I was a bit surprised to find that the file could already > handle the command `revert`, even though it isn't documented in > `append_todo_help` of rebase-interactive.c - is that by choice or just > missing documentation? The reason that it is not documented, and that it has no single-letter "short command", is that it is more of a historic accident than design that interactive rebases support the "revert" command: In 004fefa754a4 (sequencer: completely revamp the "todo" script parsing, 2016-10-21), I revamped sequencer's parsing of the "todo script", in preparation for teaching it the trick to parse full-blown todo scripts of interactive rebases in addition to parsing the (hitherto quite limited) `cherry-pick` and `revert` "scripts", a trick that was completed with cca3be6ea15b (Merge branch 'js/prepare-sequencer', 2016-10-27). These days, `git rebase --interactive` uses that code to parse todo scripts. Naturally, to be able to continue parsing the "revert scripts", the `revert` command needed to be supported, and I never thought of disabling it specifically for interactive rebases. > Whenever I wanted to achieve this I used `break` and then manually did > the revert, which obviously works fine, but it is much nicer to put the > command in the todo file... (Now that I think about it I could also have > done it with `exec`, but that is also not the nicest solution :D ). Right. I often find myself adding commands like this one: x git revert -n <reverse-fixup> && git commit --amend --no-edit to amend a commit with a reversal of another commit, most prominently when I squashed a fixup into another commit than I had originally intended and now need to fix that. > The only other command not mentioned is `noop` which is obviously not > too useful apart from distinguishing an empty list and aborting, so I > totally understand it missing. Correct, that one is intentionally not described, for the reasons you described. > Yes - in contrast to all the other options it does not have a single > char notation (and 'r' is obviously already taken und 'u' for undo as > well or 't' for the last letter), but why not show it in the list > without it? Or maybe add 'v' for "reVert"? Sure ;-) Ciao, Johannes
