Re: Add comment lines to patch format

From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: May 19, 2012, 04:54 UTC
Message-ID: <CACsJy8C-+VwpR4XEBqxhpwhtfZ=cxo6uKSp6Ov7K6aLGxqR=Sw@mail.gmail.com>
In-Reply-To: <20120519001410.GC765@sigill.intra.peff.net>

On Sat, May 19, 2012 at 7:14 AM, Jeff King <peff@peff.net> wrote:

Show 12 quoted lines

> On Fri, May 18, 2012 at 08:22:28PM +0700, Nguyen Thai Ngoc Duy wrote:
>
>> One thing is trailing space, like demonstrated in the patch below,
>> because trailing spaces may be intentional sometimes. But I'd like to
>> incorporate some word-diff goodness in patch format using this comment
>> line to spot few/single character addition/removal.
>> [...]
>> If anyone knows a tool with similar feature, I'd greatly appreciate it
>> (as the Internet taught me, everything I think of is already thought
>> of/implemented by someone)
>
> Have you looked at contrib/diff-highlight?

I didn't. Its output looks too packed though.

Show 15 quoted lines

> I think your approach is interesting because it can annotate much more
> than just "this part is interesting". Sometimes when I send a patch, I'd
> like to be able to comment in-line, like:
>
>  +       if (bar > 3)
>  +               foo(bar);
>  = Yeah, this magic "3" is ugly, and we should handle it
>  = through $whatever in the re-roll.
>  +       else
>  +               something_else();
>
> I usually just write something in the cover letter, or reply to myself
> with comments inline. I don't know how much I would use it in practice,
> but it might be a neat thing for "git apply" to simply ignore comment
> lines in the middle of a hunk.

Yeah. That's a good thing about in-patch comments.

One thing that's still bugging me is that we don't have a way to send machine-readable conflict resolutions in text form. But that's probably something else.

> Of course, this feature might not be worth breaking compatibility with
> every existing version of "git apply" and "patch" out there.

It's opt-in. And it's quite easy to convert back. A simple "sed -i '/^=/d'" would do.

-- 
Duy