Wincent Colaiuta <win@xxxxxxxxxxx> writes:
> diff --git a/ws.c b/ws.c
> index 52c10ca..884d373 100644
> --- a/ws.c
> +++ b/ws.c
> +unsigned check_whitespace(const char *line, int len, unsigned ws_rule)
> +{
> + ...
> + if (ws_rule & WS_TRAILING_SPACE) {
> + /* Lines start with "+" or "-" so length is at least 1 */
> + if (line[len - 1] == '\n') {
> + if (isspace(line[len - 2]))
> + result |= WS_TRAILING_SPACE;
> + }
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, ...);
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?
-
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
