Re: [PATCH 5/6] whitespace: more accurate initial-indent highlighting

From: Wincent Colaiuta <win@wincent.com>
Date: Dec 17, 2007, 08:00 UTC
Message-ID: <ACA0791E-189F-4E19-AE87-C7D1163C0366@wincent.com>
In-Reply-To: <1197822702-5262-6-git-send-email-bfields@citi.umich.edu>

El 16/12/2007, a las 17:31, J. Bruce Fields escribió:

Show 7 quoted lines

> Instead of highlighting the entire initial indent, highlight only the
> problematic spaces.
>
> In the case of an indent like ' \t \t' there may be multiple  
> problematic
> ranges, so it's easiest to emit the highlighting as we go instead of
> trying rember disjoint ranges and do it all at the end.

I'm relatively opposed to mixing the "check" and the "emit" phases here because it will make further refactoring harder.

In the initial version of your series you forgot that there was another place in the codebase ("git apply --whitespace=fix") where whitespace errors are detected, and so you introduced an inconsistency which you later fixed up. To me this is indicative of the fact that we need to refactor further so that there really is only *one* place where the whitespace checking logic is implemented.

Basically I would have proposed extracting out each type of whitespace error into an inline function in ws.c, where it could be used by both check_and_emit_line() in ws.c and apply_one_fragment() in builtin- apply.c.

Unfortunately, mixing checking and emission phases makes this proposed refactoring a little bit ugly.

But I see it's already gone into master as ffe56885.

Wincent