Re: Add comment lines to patch format

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, May 19, 2012 at 7:14 AM, Jeff King <peff@xxxxxxxx> wrote:
> 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.

> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]