Am 15.01.2017 um 11:06 schrieb Vegard Nossum:
On 15/01/2017 03:39, Junio C Hamano wrote:
René Scharfe <l.s.r@xxxxxx> writes:
How about extending the context upward only up to and excluding a line
that is either empty *or* a function line? That would limit the extra
context to a single function in the worst case.
Reducing context at the bottom with the aim to remove comments for the
next section is more tricky as it could remove part of the function
that we'd like to show if we get the boundary wrong. How bad would it
be to keep the southern border unchanged?
I personally do not think there is any robust heuristic other than
Vegard's "a blank line may be a signal enough that lines before that
are not part of the beginning of the function", and I think your
"hence we look for a blank line but if there is a line that matches
the function header, stop there as we know we came too far back"
will be a good-enough safety measure.
I also agree with you that we probably do not want to futz with the
southern border.
You are right, trying to change the southern border in this way is not
quite reliable if there are no empty lines whatsoever and can
erroneously cause the function context to not include the bottom of the
function being changed.
I'm splitting the function boundary detection logic into separate
functions and trying to solve the above case without breaking the tests
(and adding a new test for the above case too).
I'll see if I can additionally provide some toggles (flags or config
variables) to control the new behaviour, what I had in mind was:
-W[=preamble,=no-preamble]
--function-context[=preamble,=no-preamble]
diff.functionContextPreamble = <bool>
(where the new logic is controlled by the new config variable and
overridden by the presence of =preamble or =no-preamble).
Adding comments before a function is useful, removing comments after a
function sounds to me as only nice to have (under the assumption that
they belong to the next function[*]). How bad would it be to only
implement the first part (as in the patch I just sent) without adding
new config settings or parameters?
Thanks,
René
[*] Silly counter-example (the #endif line):
#ifdef SOMETHING
int f(...) {
// implementation for SOMETHING
}
#else
inf f(...) {
// implementation without SOMETHING
}
#endif /* SOMETHING */