On 06.04.23 14:07, Phillip Wood wrote: > Hi Stefan > > On 06/04/2023 11:48, Stefan Haller wrote: >> On 06.04.23 11:49, Phillip Wood wrote: >>> Hi Stefan >>> >>> On 05/04/2023 18:36, Stefan Haller wrote: >>>> The git-rebase documentation has an example for a git-rebase-todo file >>>> when --rebase-merges is used; one of the lines in that file is >>>> >>>> reset refactor-button # Use the Button class for all buttons >>>> >>>> From reading the code that parses the file, this doesn't seem to be a >>>> valid line; as far as I can see, comments are not supported for >>>> reset or >>>> label. The label is the entire rest of the line after the command. >>> >>> You're right that comments are not supported for labels, but for the >>> reset command do_reset() ignores everything after the label so it does >>> effectively support comments. >> >> I don't follow; do_reset() simply uses whatever is stored in >> item->arg_len, and this is set to go until the end of the line for >> "label" and "reset" in parse_insn_line(). > > But it splits in line in do_reset() > > /* Determine the length of the label */ > for (i = 0; i < len; i++) > if (isspace(name[i])) > break; > len = i; > > commit = lookup_label(r, name, len, &ref_name); Ah, I missed that, thanks. But it does this only after special-casing the "[new root]" label, which does contain a space and needs to go to the end of the line. This is weird. This whole topic started because the todo parser that we are using in lazygit has a bug where it doesn't recognize the "[new root]" label properly, and this is because it does split the line at the first whitespace, but without special-casing "[new root]" like git does. How would you recommend we fix this? We could replicate git's current behavior exactly, but then this would break when you introduce comments later. Or we could anticipate that comments are added at some point, and already split the line at the first # (trimming whitespace before that). This would not exactly match git's current behavior, but it would be good enough because git never actually uses whitespace in label or reset lines except for the "[new root]" case. I'm leaning towards the second option myself. Any thoughts? -Stefan
