Re: [PATCH 2/3] xdiff: -W: include immediately preceding non-empty lines in context
- From
Stefan Beller <sbeller@google.com>
- Date
- Jan 13, 2017, 18:44 UTC
- Message-ID
- <CAGZ79kamTgefk9RxUwFk7hgAQK7kEJJKS6RjNMbnv2=Vk=BTSQ@mail.gmail.com>
- In-Reply-To
- <e55dc4dd-768b-8c9b-e3b2-e850d5d521f5@web.de>
On Fri, Jan 13, 2017 at 10:19 AM, René Scharfe <l.s.r@web.de> wrote:
Show 6 quoted lines
> Am 13.01.2017 um 17:15 schrieb Vegard Nossum: >> >> When using -W to include the whole function in the diff context, you >> are typically doing this to be able to review the change in its entirety >> within the context of the function. It is therefore almost always >> desirable to include any comments that immediately precede the function.
Do we need a small comment in the actual code to hint at why we count upwards there?
Show 18 quoted lines
>>
>> This also the fixes the case for C where the declaration is split across
>> multiple lines (where the first line of the declaration would not be
>> included in the output), e.g.:
>>
>> void
>> dummy(void)
>> {
>> ...
>> }
>>
>
> That's true, but I'm not sure "non-empty line before function line" is good
> enough a definition for desirable lines. It wouldn't work for people who
> don't believe in empty lines. Or for those that put a blank line between
> comment and function. (I have an opinion on such habits, but git diff
> should probably stay neutral.) And that's just for C code; I have no idea
> how this heuristic would hold up for other file types like HTML.I think empty lines are "good as a first approach", see e.g. 433860f3d0beb0c6 the "compaction" heuristic for a similar thing (the compaction was introduced at d634d61ed), and then we can build a more elaborate thing on top.
Show 5 quoted lines
> > We can identify function lines with arbitrary precision (with a xfuncname > regex, if needed), but there is no accurate way to classify lines as > comments, or as the end of functions. Adding optional regexes for single- > and multi-line comments would help, at least for C.
That would cover Java and whole lot of other C like languages. So a good start as well IMHO.
> > René