Touching LayoutTests/resources/dump-as-markup.js requests git cl format and then destroys the format of the file |
||
Issue description
It wants me to format this JS, but when I do it "git cl format" actually formats the entire file instead of the few lines I actually touched. It also does terrible things like inserting massive amounts of whitespace and squishing all of the text to the far right.
I don't think we should require git cl format of the JS inside LayoutTests in general, but it also shouldn't incorrectly format the file or touch every line outside the ones the patch did.
ex.
-Markup._namespace = function(node)
-{
- if (Markup._NAMESPACE_URI_MAP[node.namespaceURI])
- return Markup._NAMESPACE_URI_MAP[node.namespaceURI] + ' ';
- return '';
+ Markup._namespace =
+ function(node) {
+ if (Markup._NAMESPACE_URI_MAP[node.namespaceURI])
+ return Markup._NAMESPACE_URI_MAP[node.namespaceURI] + ' ';
+ return '';
}
-Markup._dumpCalls = 0
+ Markup._dumpCalls = 0
,
Jan 24 2017
we're moving to `git cl format --js` until we figure out what to do with LayoutTests
,
Jan 24 2017
by the way, the main craziness involved here is your use of automatic semicolon insertion (ASI), which clang-format (semi-intentionally) doesn't handle well https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/resources/dump-as-markup.js?q=Markup._namespace&sq=package:chromium&dr=C&l=256
,
Jan 25 2017
> we're moving to `git cl format --js` until we figure out what to do with LayoutTests Can you expand on what that means? If you touch .js files, you need to pass --js to `git cl format`? What if you forget? What if your Cl contains both .js and .cc files?
,
Jan 25 2017
,
Jan 25 2017
Why Restrict-View-Google?
,
Jan 25 2017
oh, thought that was a google link, my bad. it's a bunch of @chromium reviewers. lifting
,
Jan 26 2017
To answer comment 4: https://chromium-review.googlesource.com/#/c/430526/ is the actual change; --js to `git cl format` just makes formatting js code opt-in. That looks like a good workaround to me for now. |
||
►
Sign in to add a comment |
||
Comment 1 by thakis@chromium.org
, Jan 20 2017