On Wed, 9 Jan 2008, Junio C Hamano wrote:

quoted text
> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > When I rebase series with bad whitespace, I end up with unhelpful messages 
> > like:
> >
> > .dotest/patch:412: trailing whitespace.
> > -- 
> > .dotest/patch:446: trailing whitespace.
> > -- 
> >
> > These line numbers obviously refer to lines in a file that's been removed 
> > by the time I can do anything about it.
> 
> The message is more appropriate for a workflow to "git apply --check"
> first, fix the patchfile and then applying for real.

Right; I'd keep it the same for all cases in which the patch is not 
applied: "git apply --check" (which doesn't apply it regardless) or when 
whitespace errors prevent application; so the message would be, in either 
case, appropriate to the workflow.

In fact, in your workflow, it wouldn't make any sense to give the 
resulting location of the whitespace, because that version of the file 
hasn't been created. I think that only one of the possible location 
reports is actually helpful: if it wasn't applied, the line in the patch 
file; if it was, the line in the working tree.

quoted text
> > ... if, in the case where it leaves the working tree 
> > modified with the non-compliant whitespace, it gave this location rather 
> > than the patch's location (because, even if you have the patch still, 
> > you'd need to revert it first in order to be able to apply a fixed version 
> > anyway).
> 
> In such a case, "git diff" will highlight the non-compliant whitespace.
> 
> More problematic is if you used whitespace=warn to let it commit anyway.
> You can use "git diff $beginning_of_series..HEAD" the same way
> to locate the breakages, but you then need to do "rebase -i" to
> fix it up (I personally would run "format-patch", fix the problems in
> the patch text, and run "am", instead of "rebase -i", mostly
> because I am used to working that way).

I should point out that, in this particular series, the non-compliant 
whitespace is in the "expected result" file for tests for email message 
generation, and it actually has to be like that. The annoying thing is 
that it's not clear what file has the trailing whitespace, so it's not 
clear that it is supposed to be that way from the message.

	-Daniel
*This .sig left intentionally blank*
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html