René Scharfe <l.s.r@xxxxxx> writes:
> 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.
>>
>> 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.
As you are, I am fairly negative on the heuristic based on the
"non-blank" thing. We tried once with compaction-heuristics already
and it did not quite perform well. Let's not hardcode another one.
> 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.
The funcline regexp is used for two related but different purposes.
It identifies a single line to be placed on @@ ... @@ line before a
diff hunk. This line however does not have to be at the beginning
of a function. It has to be the line that conveys the most
significant information (e.g. the name of the function).
The way "diff -W" codepath used it as if it were always the very
first line of a function was bound to invite a patch like this, and
if we want to be extra elaborate, I agree that an extra mechanism to
say "the line the funcline regexp matches is not the beginning of a
function, but the beginning is a line that matches this other regexp
before that line" may help.
Do we really want to be that elaborate, though? I dunno.
I wonder if it would be sufficient to make -W take an optional
number, e.g. "git show -W4", to add extre context lines before the
funcline.