Re: [PATCH 3/3] Color support for "git-add -i"
- From
Wincent Colaiuta <win@wincent.com>
- Date
- Dec 7, 2007, 12:34 UTC
- Message-ID
- <AEA9FB5E-1ABA-42D6-8FC5-95BAAA9B6BC8@wincent.com>
- In-Reply-To
- <7vejdzd1aw.fsf@gitster.siamese.dyndns.org>
El 6/12/2007, a las 21:18, Junio C Hamano escribió:
Show 25 quoted lines
> Wincent Colaiuta <win@wincent.com> writes: > >> What are the other options? >> >> - Make git-add--interactive part of builtin-add so as to be able to >> use the core.whitespace code directly? (ideally yes and at some point >> in the future it seems inevitable that this will happen, but it will >> require a fair bit of work) >> >> - Fork a second "git diff-files" process to capture the colorized >> version of the output? (may set off the "kludge" alarm) >> >> - Something else? > > - Realize that whitespace clean-up can be risky and change semantics > depending on the material you are editing. Do not do the clean-up > during "add -i", but before. IOW, add an alias that does an > equivalent of: > > git diff HEAD >tmp > git apply -R <tmp > git apply --whitespace=fix <tmp > > then encourage users to clean-up their whitespace gotchas early (and > test the result!), before even attempting to run "git add -i".
In principle I agree with what you're saying but I think I didn't make myself clear; I wasn't talking about actually *cleaning-up* whitespace problems from inside "git add -i", I was talking about *display* of whitespace problems only. In other words, I don't want the situation where I think it's ok to stage a hunk only because I mistakenly think it's free of whitespace problems. In other words, if one part of Git ("git diff") says the diff is "this" and another part ("git add -i") says the diff is "that" and there's a discrepancy, then that sounds like a bug to me.
Seeing as speed is not a concern here (when displaying diffs the bottleneck is really how fast the user can read, not how fast git emits the text), correctness is important, and git-add--interactive will eventually become part of builtin-add anyway, I personally think the optimum solution is this one: run the diff tools twice, once without color and once with; the former is used for internal processing as before and the later is used for display only. This approach has the advantage that the whitespace colorization provided by "git diff-files" can evolve and become more sophisticated (per-path attributes etc) and "git add -i" will automatically benefit from it.
But I wanted to throw the question out there for input first, in any case, I have a patch implementing my proposal and I'll send it out in a minute.
Cheers, Wincent