El 12/12/2007, a las 20:39, Junio C Hamano escribió:
I like the direction, but I think it would make much more sense if you
make check_whitespace() not about "a line in a patch that adds a
line",
but about "here is a line, check if that is acceptable". IOW, make
line
variable zero-based (and len = strlen(line)). The change would mean
that existing callers need to be modified to do something like:
if (line[0] == '+')
check_whitespace(line+1, len-1, ...);
Yes, good point. As you say, it can then potentially be used for any
line, not just lines in patches.
but at the same time we could conceivably teach "git show" to show
whitespace errors in a blob, i.e. "git show --show-ws-error HEAD:ws.c"
by using such a check_whitespace().
The highlighting code may need similar changes. I was actually hoping
you would consolidate the logic there that decides which segment of
the
string to highlight, and the logic in check_whitespace() to decide if
there is an error to begin with. Conceptually, if emit_line_with_ws()
decides there is nothing to highlight with DIFF_WHITESPACE color, that
means there is no whitespace error on the line and vice-versa, no?
You're dead right. As I said in my original message, there is a *lot*
of diff code and I basically only looked at enough of it to figure out
what changes would be necessary to implement what I wanted. I hadn't
looked inside emit_line_with_ws but it is an obvious site of
duplication and as you point out there is definitely scope for more
refactoring here; instead of doing the whitespace checking in three
places (the three that I know about so far) we can do it in just one
place. I'll see what I can cook up.
Cheers,
Wincent
-
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
