[PATCH] D28165: Change clang-format's Chromium JavaScript defaults

Dan Beam via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 3 13:59:00 PST 2017


danbeam marked an inline comment as done.
danbeam added inline comments.


================
Comment at: lib/Format/Format.cpp:643
+    ChromiumStyle.AllowShortIfStatementsOnASingleLine = false;
+    ChromiumStyle.AllowShortLoopsOnASingleLine = false;
+  }
----------------
thakis wrote:
> Thanks for the patch! Do we want these as false in Chromium's JS? I would've thought the diff would just be 
> 
> ```
> - } else {
> + } else if (Language != FormatStyle::LK_JavaScript)
> ```
> 
> so that we just use google style for JS.
> 
> If we do want to deviate from google style here for some reason then
> a) say why somewhere
> b) change the check for cpp to also include `|| Language == FormatStyle::LK_ObjC`
> 
> (If you include more diff context as described on http://llvm.org/docs/Phabricator.html, reviewing on phab is a bit easier.)
> so that we just use google style for JS

we want to tweak Google style a little bit.  I mentioned this on the [[ https://groups.google.com/a/chromium.org/d/msg/chromium-dev/OTEfsPvp0qc/UsaOTR9IEQAJ | chromium-dev@ thread ]].

I added a specific branch for JS with explicit specializations so it's clearer to the reader how Google and Chromium differ.

> (If you include more diff context as described on http://llvm.org/docs/Phabricator.html, reviewing on phab is a bit easier.)

Done.


https://reviews.llvm.org/D28165





More information about the cfe-commits mailing list