Detect trailing spaces and tabs
ClosedPublic
Actions

Authored by johnny-bit on May 2 2017, 5:49 PM.

Details

Reviewers

epriestley

Group Reviewers

Blessed Reviewers

Maniphest Tasks

T12655: ArcanistTextLinter: Detect trailing tabs

Commits

rARC20ad47f27331: (stable) Promote 2017 Week 18
rARC3c4735795a29: Detect trailing spaces and tabs

Summary

Ref T12655 - this change properly detects trailing spaces or tabs (or combinations of thereof) on end of lines.

Test Plan

Use Text lint with trailing whitespace rule on files with spaces, tabs or combinations of thereof. Should properly detect and fix all those.

Diff Detail

Repository

rARC Arcanist

Branch

text-lint-tab (branched from master)

Lint

Lint Skipped

Unit

Test Failures

Build Status

Buildable 16765
Build 22366: arc lint + arc unit

Unit TestsFailed
View All

	Time	Test
	6 ms	ArcanistTextLinterTestCase::Unknown Unit Message ("") In 'trailing-whitespace-4.lint-test', expected lint to raise autofix on line 1 at char 28, but no autofix was raised. Actually raised: error at line 1, char 1: TXT1 DOS Newlines error at line 1, char 28: TXT2 Tab Literal
	8 ms	ArcanistChmodLinterTestCase::Unknown Unit Message ("") 5 assertions passed.
	0 ms	ArcanistClosureLinterTestCase::Unknown Unit Message ("") 1 assertion passed.
	0 ms	ArcanistCpplintLinterTestCase::Unknown Unit Message ("") 1 assertion passed.
	6 ms	ArcanistFilenameLinterTestCase::Unknown Unit Message ("") 5 assertions passed.
		View Full Test Results (1 Failed · 24 Passed · 29 Skipped)

Event Timeline

johnny-bit created this revision.May 2 2017, 5:49 PM

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptMay 2 2017, 5:49 PM

Herald added a subscriber: Korvin. · View Herald Transcript

johnny-bit mentioned this in T12655: ArcanistTextLinter: Detect trailing tabs.May 2 2017, 5:51 PM

These tests seem to pass on my system -- can you describe the \r\n issue you're running into in more detail? (Or just include the test case which isn't working, if it isn't one of these?)

I had 4th test file for cr/lf lineendings but couldn't get past

In 'trailing-whitespace-4.lint-test', expected lint to raise autofix on line 1 at char 28, but no autofix was raised. Actually raised:
  error at line 1, char 1: TXT1 DOS Newlines
  error at line 1, char 28: TXT2 Tab Literal

Whole file was identical to trailing-whitespace-3.lint-test but 3 first lines had cr/lf line endings (last one left only lf to properly detect ~~~~~~~~~~ plus had 2 error lines:

error:1:1
error:1:28

but generally seems to work while preserving line endings.
If you'd like I can send testfile to paste or add to this diff for checking.

Yeah, just go ahead and add it to the diff.

add failing test case

This is significantly involved because ArcanistLintPatcher, and other callsites elsewhere, hard-code "\n" line endings (e.g., in AcanistLintPatcher->getCharacterOffset()). This essentially prevents files with "\r\n" line endings from being patched correctly, at least in cases where patches interact with line endings.

I spent ~10 minutes poking at it without much success.

Let's not poke it further then. May we agree to remove trailing-whitespace-4.lint-test and say that "\r\n" line endings are generally no-go? This would slash troubleshooting this and all other callsites that have hardcoded line endings 😉