On Tue, Sep 07 2021, Han-Wen Nienhuys wrote:
> On Tue, Sep 7, 2021 at 12:35 AM Ævar Arnfjörð Bjarmason
> <avarab@xxxxxxxxx> wrote:
>> > - printf("%s %s %s %"PRItime" %d %s\n",
>> > - oid_to_hex(old_oid), oid_to_hex(new_oid),
>> > - committer, timestamp, tz, msg);
>> > + printf("%s %s %s %" PRItime " %+05d\t%s\n", oid_to_hex(old_oid),
>> > + oid_to_hex(new_oid), committer, timestamp, tz, msg);
>>
>> Nit: Would be a more readable diff if this wasn't a
>> line-wrap-while-at-it change in addition to changing the format string.
>>
>> I.e. the last 4x parameters aren't changed, so leaving them on their own
>> line & just changing the string & the two oid_to_hex()...
>
> This is clang-format's output, and the new code takes up less lines vertically.
>
> If you think this is really, really important, I can change it, but I
> think it's a better use of everyone's time to leave mechanical tasks
> (like formatting) to the machines.
It's not "really, really important" or even "really important", just
notes while reading the series.
Patches are read N times, including during review. Since humans read
formatting changes, it's in general are not something we can leave to
machines. We're after all looking at the patch on-list, not diffing two
versions of the generated machine code, or of clang-format's output.
I don't think the intent of the clang-format target is to run it before
patch submission, but to serve as input on suggested formatting changes.
If on master you run:
git ls-files '*.[ch]' -z | xargs -n 1 -0 clang-format -i
You'll get:
631 files changed, 48246 insertions(+), 45061 deletions(-)
Which I think speaks for itself in the likelyhood that
"git-clang-format" is going to inject unnecessary churn into submitted
patches.
I think this case is quite small and not worth a re-roll.
