Page MenuHomePhabricator
Differential D10472

Improve `LINT_DOS_NEWLINE` behavior
ClosedPublic
Actions

Authored by joshuaspence on Sep 11 2014, 12:24 PM.
Tags
None
Referenced Files
F14117654: D10472.id25315.diff
Thu, Nov 28, 7:51 PM
F14117653: D10472.id25314.diff
Thu, Nov 28, 7:51 PM
F14117651: D10472.id25348.diff
Thu, Nov 28, 7:50 PM
F14117650: D10472.id25184.diff
Thu, Nov 28, 7:50 PM
F14117649: D10472.id25349.diff
Thu, Nov 28, 7:50 PM
F14117648: D10472.id.diff
Thu, Nov 28, 7:50 PM
F14117614: D10472.diff
Thu, Nov 28, 7:32 PM
Unknown Object (File)
Tue, Nov 26, 2:38 AM
View All Files
Subscribers
epriestley
Korvin

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Commits
rARC888ada2f380c: Improve `LINT_DOS_NEWLINE` behavior
Summary

Improve the behavior of ArcanistTextLinter in dealing with LINT_DOS_NEWLINE. In particular, provide replacement text so that these linter issues can be autofixed.

Test Plan

Added a unit test.

Diff Detail

Repository
rARC Arcanist
Branch
arcpatch-D10472
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 2604
Build 2608: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

joshuaspence updated this revision to Diff 25184.Sep 11 2014, 12:24 PM
joshuaspence retitled this revision from to Improve `LINT_DOS_NEWLINE` behavior.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.
Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptSep 11 2014, 12:24 PM
Herald added subscribers: Korvin, epriestley. · View Herald Transcript
Harbormaster completed remote builds in B2508: Diff 25184.Sep 11 2014, 12:25 PM
epriestley requested changes to this revision.Sep 11 2014, 12:38 PM
epriestley edited edge metadata.

If the file has OS9 newlines (\r only), this will completely destroy them and mangle the file. We should only remove \r if the next character is \n.

Also, maybe it's better to emit one giant whole-file patch than one patch for each line?

This revision now requires changes to proceed.Sep 11 2014, 12:38 PM
joshuaspence updated this revision to Diff 25314.Sep 22 2014, 11:14 AM
joshuaspence edited edge metadata.
  • Don't suggest a fix if the file has OS9 newlines
  • Suggest one patch per file rather than one patch per line
Harbormaster completed remote builds in B2603: Diff 25314.Sep 22 2014, 11:14 AM
joshuaspence updated this object.Sep 22 2014, 11:20 AM
joshuaspence edited edge metadata.
joshuaspence updated this revision to Diff 25315.Sep 22 2014, 11:23 AM

Rebase

Harbormaster completed remote builds in B2604: Diff 25315.Sep 22 2014, 11:24 AM
epriestley accepted this revision.Sep 24 2014, 8:59 PM
epriestley edited edge metadata.

We could also suggest one patch per contiguous block of lines with bad newlines as an ideal behavior, but this should usually accomplish the same thing.

This revision is now accepted and ready to land.Sep 24 2014, 8:59 PM
joshuaspence updated this revision to Diff 25348.Sep 24 2014, 9:16 PM
joshuaspence edited edge metadata.

Rebase

Closed by commit rARC888ada2f380c: Improve `LINT_DOS_NEWLINE` behavior (authored by joshuaspence, committed by joshuaspence).Sep 24 2014, 9:16 PM
This revision was automatically updated to reflect the committed changes.
Harbormaster completed remote builds in B2626: Diff 25348.Sep 24 2014, 9:17 PM

Revision Contents
Changeset List

PathSize
src/
lint/
linter/
11 lines
__tests__/
text/
7 lines

Diff 25315

View Options

src/lint/linter/ArcanistTextLinter.php

Loading...
View Options

src/lint/linter/__tests__/text/dos-newline.lint-test

Loading...
Phabricator · Built by Phacility