1675648 - moz-phab incorrectly handles commits with CARRIAGE RETURNs and creates unlandable diffs

Status: RESOLVED FIXED

(Conduit :: moz-phab, defect, P2)

Description

[Ian Moody [:Kwan] (UTC+0)]

Attachment: bad_carriage_return_diff.png

See D95451 (Lando) and the related #conduit:mozilla.org Matrix discussion.

I think it might be because it seems like moz-phab is processing the diffs with python's splitlines(), which includes lone \rs, instead of just doing split("\n"), which seems to be what arcanist does

The result is extra - prefixed lines that shouldn't be.
Attached picture shows what moz-phab diffs read from Phabricator looks like on the left, with the errors on lines 14 and 16, and the correct diff on the right.

Comment 1

[Zeid [:zeid]]

Attachment: Bug 1675648 - improve handling of line separators r=sheehan,dkl

WIP DO NOT LAND

This change improves line separator support on different OSes by:

• Determining linesep character based on current OS (e.g. \n or \r\n)
• Splitting lines that are generated by hg cat only on linesep instead
  of on any whitespace or newline character
  • This has the side-effect of preventing stray characters such as
    carriage returns from triggering a line split

This change also combines the functionality that is shared between
methods into a new _generate_lines static method.

TODO: add a test specific to _generate_lines.