New issue
Advanced search Search tips

Issue 682988 link

Starred by 2 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Touching LayoutTests/resources/dump-as-markup.js requests git cl format and then destroys the format of the file

Project Member Reported by esprehn@chromium.org, Jan 20 2017

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



 

Comment 1 by thakis@chromium.org, Jan 20 2017

Cc: dbeam@chromium.org
+js clang-format Mastermind

Comment 2 by dbeam@chromium.org, Jan 24 2017

we're moving to `git cl format --js` until we figure out what to do with LayoutTests

Comment 3 by dbeam@chromium.org, 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

Comment 4 by thakis@chromium.org, 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?

Comment 5 by dbeam@chromium.org, Jan 25 2017

Labels: Restrict-View-Google
https://chromium-review.googlesource.com/#/c/430526/
Why Restrict-View-Google?

Comment 7 by dbeam@chromium.org, Jan 25 2017

Labels: -Restrict-View-Google
oh, thought that was a google link, my bad.  it's a bunch of @chromium reviewers.  lifting

Comment 8 by thakis@chromium.org, 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