Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> The long-standing semantics of how -W works and interacts with -U<n>
> are rather easy to reason about:
>
> * -W extends the context line up to the start of the function. With
> userdiff this means the language-aware regex rules in userdiff.c,
> or user-supplied rules.
OK.
> * -U<n>, which defaults to -U3 shows at least <n> lines of context,
> if that's greater than what we'd extend the context to under -W
> then -U<n> wins.
Sorry, but I cannot quite guess what you mean here. Do you mean:
We first go back <n> lines due to -U<n>. If we have not found a
function header line that comes before the first changed line
within that <n> lines, we keep going back to satisfy -W (i.e. -W
"wins" if "-U<n>" is small). On the other hand, after going
back <n> lines, if we have already seen a function header before
the first changed line within these <n> lines, there is no need
to go back further to satisfy -W (i.e. -U<n> "wins" and makes -W
irrelevant).
If that is the case, I think I understand it.
> * When showing the hunk context we look up from the first line we
> show of the diff, and find whatever looks like useful context above
> that line.
Yes.
> Thus in e.g. the xdiff/xemit.c change being made in this commit we'll
> correctly show "xdl_emit_diff()" in the hunk context under default
> diff settings.
>
> But if we viewed it with the -W option we'd show "is_empty_rec()",
> because we'd first find the "xdl_emit_diff()" context line, extend the
> diff to that, and then would go look for context to show again.
Makes sense. As long as we apply this when "-W wins", i.e we extend
the precontext so that the hunk begins on the line that match the
function header pattern, there isn't much point in showing only the
name of the function that comes before this hunk.
On the other hand, if "-U<n> won", i.e. the first precontext line
that is shown in the hunk is _before_ the line -W chose to extend
the hunk to (in this case, where xdl_emit_diff begins) because -U<n>
gave a large enough number, I do want to see the name of the fuction
I am looking at the tail of in the precontext on the hunk header
line. E.g. "git diff -W -U120 xdiff/xemit.c" on this patch should
show is_empty_rec() as the hunk header.
> This new behavior does give us the edge case that if we e.g. view the
> diff here with "-U150 -W" we'd previously extend the context to the
> middle of the "is_func_rec()" function, and show that function in the
> hunk context. Now we'll show nothing.
To me, that sounds like a grave regression. Why lose the
information?
This may be coming from the difference between us, i.e. I read a lot
more patches written by other people than my own changes written for
my next commit, so every bit of hint helps, and the name of the
function I am seeing its latter half in the precontext is sometimes
a useful thing to see.
