[PATCH 1 of 2 STABLE] diffhelpers: add canstripcr=True to fix_newline

Phillip Cohen phillip at phillip.io
Mon Apr 24 17:53:44 EDT 2017


> Very minor nit: is that an extra space in front of '{'? Not sure about
our C-style.

Eep, you're right.

On Mon, Apr 24, 2017 at 2:45 PM, Sean Farley <sean at farley.io> wrote:
> Jun Wu <quark at fb.com> writes:
>
>> This fixes a 4-year bug. I think it is worth being merged.
>
> I would agree. The patch seems fine, safety-wise (but I'd appreciate
> another C person).
>
>> Excerpts from Phil Cohen's message of 2017-04-24 13:10:00 -0700:
>>> [...]
>>>
>>> @@ -97,8 +99,18 @@
>>>              x = PyFile_GetLine(fp, 0);
>>>              s = PyBytes_AsString(x);
>>>              c = *s;
>>> -            if (strcmp(s, "\\ No newline at end of file\n") == 0) {
>>> -                _fix_newline(hunk, a, b);
>>> +            char *prefix = "\\ No newline at end of file";
>>> +
>>> +            if (strncmp(s, prefix, strlen(prefix)) == 0) {
>>> +                bool canstripcr = true;
>>> +                size_t len = strlen(s);
>>> +                /* The "\\ No newline" line must end in CRLF to
>>> +                strip CRs -- see comment in diffhelpers.py */
>>> +                if (len > 2 && s[len - 2] != '\r') {
>>> +                    canstripcr = false;
>>> +                }
>>> +
>>> +                _fix_newline(hunk, a, b, canstripcr);
>>>                  continue;
>>>              }
>>
>> Ideally this chunk belongs to the second patch, and the first patch has a
>> one line diff here replacing "_fix_newline(hunk, a, b)" with
>> "_fix_newline(hunk, a, b, 1)". Since the final state is the same, I wouldn't
>> be too nitpicking on this.
>
> Could also be fixed in-flight (not too big a deal, either).
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>


More information about the Mercurial-devel mailing list