Re: [PATCH v2] git-am: Ignore whitespace before patches

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 12, 2010 at 19:13, Jay Soffian <jaysoffian@xxxxxxxxx> wrote:
> On Wed, Aug 11, 2010 at 3:57 PM, Ævar Arnfjörð Bjarmason
> <avarab@xxxxxxxxx> wrote:
>>> Actually cut-and-paste is often a major source of whitespace breakage
>>> (including tabs silently being expanded), and I personally think a patch
>>> like this to encourage the practice is going in a wrong direction.
>>
>> I disagree and think git-am should be smarter. Any human looking at
>> something like a GMail mail.txt download will clearly see that it's a
>> patch, but git-am is pedantic and doesn't skip past whitespace at the
>> beginning of the file.
>
> The point of git-am being pedantic is to prevent the original patch
> from being applied w/silent corruption (e.g., tabs-to-spaces).

The git-am code doesn't strike me as particularly pedantic. It'll fail
to spot any number of common pitfalls like patches being pasted inline
(recently discussed on list), double encoding, invalid author names /
E-Mail addresses etc. (which can e.g. happen when applying patches
from RT).

The parsing code just didn't think of this issue, I'm not aware of any
corruption that begins with whitespace being added to the beginning of
a patch, but I am aware of a non-corruption (GMail) that does that.

> Perhaps, before making git-am less strict

I don't think it's less strict with this patch, just more intelligent.

> we should modify format-patch to include a sha1 of the diff output
> so that corruption can be reliably detected by git-am.

There's a lot we could do in this department, and there was a previous
discussion on list about schemas like that (can't find it now).

We could do an ad-hoc checksum, but including more than the SHA would
be better, e.g.:

    --
    cs:<7 char SHA1> t:<NUM TABS> s:<NUM SPACES> c:<NUM CHARS ([^
\t])> ln:<NUM LINES>

That'd allow git-am to print more intelligent error messages than just
"ok/not ok", e.g.:

    * "your patch is $x lines, but the patch thinks it's $y, something
       may have gone wrong with wrapping"

    * "You have 0 tabs, but the patch thinks it has 20"

etc., but that's a project for another day.
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]