Re: [PATCH v3 01/10] xdiff: delete static forward declarations in xprepare

From: Elijah Newren <newren@gmail.com>
Date: Sep 20, 2025, 18:31 UTC
Message-ID: <CABPp-BFJPdVkA0U14_B_EzTj6rAi3jPFcQgW2uPqSz1ZBOdKAQ@mail.gmail.com>
In-Reply-To: <CAH=ZcbDzBvmzE75-7-BresWG0tYW05-1G-cnT+FLeuJe6JStcA@mail.gmail.com>

On Sat, Sep 20, 2025 at 10:41 AM Ezekiel Newren <ezekielnewren@gmail.com> wrote:

Show 12 quoted lines

>
> On Sat, Sep 20, 2025 at 11:16 AM Junio C Hamano <gitster@pobox.com> wrote:
> > > Best-viewed-with: --color-moved
> >
> > Two comments.
> >
> >  - This is a bit unusual to see in the trailer.
>
> I'm still not sure what the etiquette is for including those kinds of
> flags in a commit message. Could you show me my full commit message as
> a response with your preferred way of adding those flags in a commit
> message? please.

This command run in git.git will give lots of examples from various authors:
   git log --grep="est viewed with"

I didn't call out the trailer because I saw Ben suggest it in this thread, and he said he had seen Peff do this. But, searching right now, I can't actually find any such trailers from Peff or anyone else -- am I just searching wrong?

Show 9 quoted lines

> >  - It turned out that it was a very effective way to spot a typo for
> >    me.  You should try it yourself before you send out your patches
> >    ;-).
>
> Huh, I have viewed this patch with --color-moved dozens of times. I
> think CLion (My IDE of choice for C) "fixed" that for me, and I didn't
> notice until you pointed it out. Elijah missed it too. Maybe my
> terminal needs a more extreme contrast, so it's easier for me to spot
> things like that.

Yep, I did. In fact, in v1 of the series Ezekiel didn't call out the --color-moved thing, but I used it and mentioned it in my review, and Ezekiel decided to mention it in v2. When I looked at the --color-moved output, I saw the sea of purple and blue and skimmed quickly to verify it was all purple and blue -- and apparently didn't see the one red character amidst the purple and the one green character amidst the blue. Since the range-diff showed no differences to v1 and I had already thought this patch was fine from v1, I didn't look at it any closer.

So, not only did I miss it, but I missed it despite being the one to suggest that flag after using it myself. Oops.