Page MenuHomePhabricator

Remove all whitespace options/configuration everywhere
ClosedPublic

Authored by epriestley on Feb 16 2019, 2:16 PM.

Details

Summary

Depends on D20181. Depends on D20182. Fixes T3498. Ref T13161. My claim, at least, is that D20181 can be tweaked to be good enough to throw away this "feature" completely.

I think this feature was sort of a mistake, where the ease of access to diff -bw shaped behavior a very long time ago and then the train just ran a long way down the tracks in the same direction.

Test Plan

Grepped for whitespace, deleted almost everything. Poked around the UI a bit. I'm expecting the whitespace changes to get some more iteration this week so I not being hugely pedantic about testing this stuff exhaustively.

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Feb 16 2019, 2:16 PM
Harbormaster returned this revision to the author for changes because remote builds failed.Feb 16 2019, 2:18 PM
Harbormaster failed remote builds in B22039: Diff 48192!
epriestley added inline comments.Feb 16 2019, 2:49 PM
src/applications/differential/parser/DifferentialChangesetParser.php
718–734

See T13161. This is basically the piece I'm going to restore in the next diff to improve line alignment when lines changes only by changing whitespace, so this is coming back in some form soon.

src/applications/differential/parser/DifferentialHunkParser.php
318–331

This piece may need to come back, too.

amckinley accepted this revision.Tue, Feb 19, 8:21 PM
amckinley added inline comments.
src/applications/differential/__tests__/data/generated.diff.one.unshielded
1–5

This isn't really a generated file; it's just designed to appear as one, right? Presumably there isn't a very good way to actually test this stuff without also incidentally adding a "This is a generated file which doesn't need to be reviewed" message when reviewing revisions that touch these files.

This revision is now accepted and ready to land.Tue, Feb 19, 8:21 PM
epriestley added inline comments.Tue, Feb 19, 8:38 PM
src/applications/differential/__tests__/data/generated.diff.one.unshielded
1–5

Right, this is testing the rendering of generated files. We could work around this with some mangling in the test harness, but it's such a weird special case that it doesn't seem terribly valuable.

This revision was automatically updated to reflect the committed changes.