Re: [PATCH v3 2/6] commit: add amend suboption to --fixup to create amend! commit

On Wed, Mar 3, 2021 at 2:37 AM Charvi Mendiratta [off-list ref] wrote:

On Tue, 2 Mar 2021 at 03:45, Eric Sunshine [off-list ref] wrote:

quoted

quoted

+       if (starts_with(sb->buf, "amend! amend!"))

Is the content of the incoming strbuf created mechanically so that we
know that there will only ever be one space between the two "amend!"
literals? If not, then this starts_with() check feels fragile.

Yes, so for preparing each "amend!" commit we add prefix "amend! '' to
the subject of the specific commit. And further if we amend the
"amend!" commit then this above code is checked before creating a
"amend! amend!" commit for the user. So I think maybe we don't need to
check for multiple spaces ?

Okay, if this is guaranteed to be created mechanically, then what you
have should work, though it may be a good idea to add an in-code
comment stating the reason it is okay to expect just the single space.

The alternative would be to avoid having "amend! amend!" in the first
place. I didn't trace through the code carefully so I don't know if it
is possible, but would it make sense for the caller(s) to check before
adding a second "amend!", thus eliminating the need to do so here?
(Perhaps I'm misunderstanding, but the above code almost feels like a
case of "whoops, we did something undesirable, so let's undo it.".)