Page MenuHomePhabricator

Remove all whitespace options/configuration everywhere
ClosedPublic

Authored by epriestley on Feb 16 2019, 2:16 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 12, 10:58 PM
Unknown Object (File)
Sun, Dec 1, 2:59 AM
Unknown Object (File)
Sat, Nov 30, 5:11 AM
Unknown Object (File)
Wed, Nov 27, 1:11 PM
Unknown Object (File)
Nov 19 2024, 4:02 AM
Unknown Object (File)
Nov 15 2024, 7:24 PM
Unknown Object (File)
Oct 21 2024, 10:13 PM
Unknown Object (File)
Oct 18 2024, 8:00 PM
Subscribers
None

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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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!
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 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.Feb 19 2019, 8:21 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.