On Fri, Jul 17, 2020 at 4:47 PM Eric Sunshine [off-list ref] wrote:

Style: write `!ce` rather than `ce == NULL`:

OK, but I'll go with Junio's suggestion of getting `ce` once and
then checking `ce_staged` anyway.  (I'm used to a different
style guide that frowns on `if (ce)` and `if (!ce)`...)

As for bikeshedding the message itself, perhaps:

    _("conflicted");

Though, perhaps that's too succinct.

Maybe.  Succinct is usually good though.

quoted

+       touch conflicted &&

If the timestamp of the file is not relevant to the test -- as is the
case here -- then we avoid using `touch`. Instead:

    >conflicted &&

OK.

... use literal TABs and let the here-doc provide the newlines.

I personally hate depending on literal tabs, as they're really
hard to see.  If q_to_tab() is OK I'll use that.

I realize that this test script is already filled with this sort of
thing where actions are performed outside of tests, however, these
days we frown upon that, and there really isn't a good reason to avoid
taking care of this clean up via the modern idiom of using
test_when_finished(), which you would call immediately after creating
the file in the test. So:

    ...
    >conflicted &&
    test_when_finished "rm -f conflicted" &&
    ...

Indeed, that's where I copied it from.

Should I clean up other instances of separated-out `rm -f`s
in this file in a separate commit?

Chris