Don't choke on blame of files with whitespace-only trailing lines
ClosedPublic
Actions

Authored by epriestley on Jan 27 2014, 6:52 PM.

Details

Reviewers

btrahan
durham

Maniphest Tasks

T4349: arc cover in Mercurial fails for files with trailing whitespace

Commits

rARC1daa719acecb: Don't choke on blame of files with whitespace-only trailing lines

Summary

Fixes T4349. Two issues:

As discussed in T4349, we would trim the entire output and then require spaces when matching. This choked incorrectly if the last line of a file contained only whitespace. Use phutil_split_lines() instead, and regexp things more reasonably.
We were capturing the line text, not the commit, as "revision". This isn't actually used elsewhere, but was obviously wrong. Make this consistent with Git/SVN.

Test Plan

Rigged a call up and saw reasonable output after the patch, on a working copy which threw before the patch.

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

durham accepted this revision.Jan 27 2014, 7:00 PM

durham added inline comments.

src/repository/api/ArcanistMercurialAPI.php
326	Why bother with $retain_line_endings if the only thing lines are used for is to check against the regex?

Mostly just force of habit, at this point. Generally, in the past, we did a lot of newline stripping and eventually hit bugs where "\r\n" newlines were getting corrupted or not handled correctly. Retaining the endings is more correct, in that if we were actually capturing the text we should capture the endings as they are printed, so callers can retain unusual endings.

Closed by commit rARC1daa719acecb (authored by @epriestley).

Revision Contents
Changeset List

Path

Size

src/

repository/

api/

ArcanistMercurialAPI.php

6 lines

Diff 18277

View Options

src/repository/api/ArcanistMercurialAPI.php

Don't choke on blame of files with whitespace-only trailing linesClosedPublicActions