Merge pull request CesiumGS#8744 from CesiumGS/lets-argue-over-minutiae

kring · web-flow · commit 05380a6d7f78 · 2020-04-17T10:11:33.000+10:00
Add configuration for prettier code formatting
diff --git a/.editorconfig b/.editorconfig
@@ -3,13 +3,7 @@ root = true
 
 [*]
 indent_style = space
-indent_size = 4
+indent_size = 2
 charset = utf-8
 trim_trailing_whitespace = true
 insert_final_newline = true
-
-[*.md]
-trim_trailing_whitespace = false
-
-[package.json]
-indent_size = 2
diff --git a/.eslintrc.json b/.eslintrc.json
@@ -1,5 +1,5 @@
 {
-    "extends": "./Tools/eslint-config-cesium/browser.js",
+    "extends": ["./Tools/eslint-config-cesium/browser.js", "prettier"],
     "plugins": [
         "html"
     ],
diff --git a/.prettierignore b/.prettierignore
@@ -0,0 +1,11 @@
+/node_modules
+/Build
+/ThirdParty
+/Apps/Sandcastle/ThirdParty
+/Source/Cesium.js
+/Source/Shaders/**/*.js
+/Source/ThirdParty/**
+/Source/Workers/**/*
+!/Source/Workers/cesiumWorkerBootstrapper.js
+!/Source/Workers/transferTypedArrayTest.js
+*.min.js
diff --git a/.travis.yml b/.travis.yml
@@ -9,6 +9,7 @@ script:
   - npm --silent run deploy-status -- --status pending --message 'Waiting for build'
 
   - npm --silent run eslint
+#  - npm --silent run prettier-check
 
   - npm --silent run build
   - npm --silent run coverage -- --browsers FirefoxHeadless --webgl-stub --failTaskOnError --suppressPassed
diff --git a/Apps/CesiumViewer/CesiumViewer.js b/Apps/CesiumViewer/CesiumViewer.js
@@ -71,7 +71,8 @@ function main() {
         var message = formatError(exception);
         console.error(message);
         if (!document.querySelector('.cesium-widget-errorPanel')) {
-            window.alert(message); //eslint-disable-line no-alert
+            //eslint-disable-next-line no-alert
+            window.alert(message);
         }
         return;
     }
diff --git a/Apps/Sandcastle/CesiumSandcastle.js b/Apps/Sandcastle/CesiumSandcastle.js
@@ -758,6 +758,7 @@ require({
                 }
 
                 var scriptCode = scriptMatch[1];
+                scriptCode = scriptCode.replace(/^ {8}/gm, ""); //Account for Prettier spacing
 
                 var htmlText = '';
                 var childIndex = 0;
diff --git a/Apps/Sandcastle/gallery/CZML Custom Properties.html b/Apps/Sandcastle/gallery/CZML Custom Properties.html
@@ -20,7 +20,6 @@
 <div id="toolbar">
     <div id="propertiesMenu"></div>
 </div>
-</div>
 
 <script id="cesium_sandcastle_script">
 function startup(Cesium) {
diff --git a/Documentation/Contributors/CodingGuide/README.md b/Documentation/Contributors/CodingGuide/README.md
@@ -90,53 +90,9 @@ A few more naming conventions are introduced below along with their design patte
 
 ## Formatting
 
-In general, format new code in the same way as the existing code.
-
-* Use four spaces for indentation.  Do not use tab characters.
-* Do not include trailing whitespace.
-* Put `{` on the same line as the previous statement:
-```javascript
-function defaultValue(a, b) {
-   // ...
-}
-
-if (!defined(result)) {
-   // ...
-}
-```
-* Use curly braces even for single line `if`, `for`, and `while` blocks, e.g.,
-```javascript
-if (!defined(result))
-    result = new Cartesian3();
-```
-is better written as
-```javascript
-if (!defined(result)) {
-    result = new Cartesian3();
-}
-```
-* Use parenthesis judiciously, e.g.,
-```javascript
-var foo = x > 0.0 && y !== 0.0;
-```
-is better written as
-```javascript
-var foo = (x > 0.0) && (y !== 0.0);
-```
-* Use vertical whitespace to separate functions and to group related statements within a function, e.g.,
-```javascript
-function Model(options) {
-    // ...
-    this._allowPicking = defaultValue(options.allowPicking, true);
-
-    this._ready = false;
-    this._readyPromise = when.defer();
-    // ...
-};
-```
-* In JavaScript code, use single quotes, `'`, instead of double quotes, `"`.  In HTML, use double quotes.
-
-* Text files, including JavaScript files, end with a newline to minimize the noise in diffs.
+* We use [prettier](https://prettier.io/) to automatically re-format all JS code at commit time, so all of the work is done for you. Code is automatically reformatted when you commit.
+* For HTML code, keep the existing style. Use double quotes.
+* Text files, end with a newline to minimize the noise in diffs.
 
 ## Linting
 
@@ -165,10 +121,11 @@ For syntax and style guidelines, we use the ESLint recommended settings (the lis
 - [no-new-require](http://eslint.org/docs/rules/no-new-require)
 
 **[Disabling Rules with Inline Comments](http://eslint.org/docs/user-guide/configuring#disabling-rules-with-inline-comments)**
- * When disabling linting for one line, use `//eslint-disable-line`:
+ * When disabling linting for one line, use `//eslint-disable-next-line`:
 ```js
 function exit(warningMessage) {
-    window.alert('Cannot exit: ' + warningMessage); //eslint-disable-line no-alert
+    //eslint-disable-next-line no-alert
+    window.alert('Cannot exit: ' + warningMessage);
 }
 ```
 
diff --git a/Source/Core/Resource.js b/Source/Core/Resource.js
@@ -1898,9 +1898,12 @@ import TrustedServers from './TrustedServers.js';
 
     function loadWithHttpRequest(url, responseType, method, data, headers, deferred, overrideMimeType) {
         // Note: only the 'json' and 'text' responseTypes transforms the loaded buffer
-        var URL = require('url').parse(url); // eslint-disable-line
-        var http = URL.protocol === 'https:' ? require('https') : require('http'); // eslint-disable-line
-        var zlib = require('zlib'); // eslint-disable-line
+        /* eslint-disable no-undef */
+        var URL = require('url').parse(url);
+        var http = URL.protocol === 'https:' ? require('https') : require('http');
+        var zlib = require('zlib');
+        /* eslint-enable no-undef */
+
         var options = {
             protocol : URL.protocol,
             hostname : URL.hostname,
@@ -1924,7 +1927,8 @@ import TrustedServers from './TrustedServers.js';
                 });
 
                 res.on('end', function() {
-                    var result = Buffer.concat(chunkArray); // eslint-disable-line
+                    // eslint-disable-next-line no-undef
+                    var result = Buffer.concat(chunkArray);
                     if (res.headers['content-encoding'] === 'gzip') {
                         zlib.gunzip(result, function(error, resultUnzipped) {
                             if (error) {
diff --git a/Source/Core/buildModuleUrl.js b/Source/Core/buildModuleUrl.js
@@ -31,7 +31,8 @@ import Resource from './Resource.js';
         a.href = url;
 
         // IE only absolutizes href on get, not set
-        a.href = a.href; // eslint-disable-line no-self-assign
+        // eslint-disable-next-line no-self-assign
+        a.href = a.href;
         return a.href;
     }
 
diff --git a/Source/Core/isCrossOriginUrl.js b/Source/Core/isCrossOriginUrl.js
@@ -22,7 +22,8 @@ import defined from './defined.js';
 
         a.href = url;
         // IE only absolutizes href on get, not set
-        a.href = a.href; // eslint-disable-line no-self-assign
+        // eslint-disable-next-line no-self-assign
+        a.href = a.href;
 
         return protocol !== a.protocol || host !== a.host;
     }
diff --git a/Source/Scene/Cesium3DTileContent.js b/Source/Scene/Cesium3DTileContent.js
@@ -39,7 +39,8 @@ import DeveloperError from '../Core/DeveloperError.js';
          * @readonly
          */
         featuresLength : {
-            get : function() {  // eslint-disable-line getter-return
+            // eslint-disable-next-line getter-return
+            get : function() {
                 DeveloperError.throwInstantiationError();
             }
         },
@@ -59,7 +60,8 @@ import DeveloperError from '../Core/DeveloperError.js';
          * @readonly
          */
         pointsLength : {
-            get : function() {  // eslint-disable-line getter-return
+            // eslint-disable-next-line getter-return
+            get : function() {
                 DeveloperError.throwInstantiationError();
             }
         },
@@ -73,7 +75,8 @@ import DeveloperError from '../Core/DeveloperError.js';
          * @readonly
          */
         trianglesLength : {
-            get : function() {  // eslint-disable-line getter-return
+            // eslint-disable-next-line getter-return
+            get : function() {
                 DeveloperError.throwInstantiationError();
             }
         },
@@ -87,7 +90,8 @@ import DeveloperError from '../Core/DeveloperError.js';
          * @readonly
          */
         geometryByteLength : {
-            get : function() {  // eslint-disable-line getter-return
+            // eslint-disable-next-line getter-return
+            get : function() {
                 DeveloperError.throwInstantiationError();
             }
         },
@@ -101,7 +105,8 @@ import DeveloperError from '../Core/DeveloperError.js';
          * @readonly
          */
         texturesByteLength : {
-            get : function() {  // eslint-disable-line getter-return
+            // eslint-disable-next-line getter-return
+            get : function() {
                 DeveloperError.throwInstantiationError();
             }
         },
@@ -115,7 +120,8 @@ import DeveloperError from '../Core/DeveloperError.js';
          * @readonly
          */
         batchTableByteLength : {
-            get : function() {  // eslint-disable-line getter-return
+            // eslint-disable-next-line getter-return
+            get : function() {
                 DeveloperError.throwInstantiationError();
             }
         },
@@ -132,7 +138,8 @@ import DeveloperError from '../Core/DeveloperError.js';
          * @readonly
          */
         innerContents : {
-            get : function() {  // eslint-disable-line getter-return
+            // eslint-disable-next-line getter-return
+            get : function() {
                 DeveloperError.throwInstantiationError();
             }
         },
@@ -146,7 +153,8 @@ import DeveloperError from '../Core/DeveloperError.js';
          * @readonly
          */
         readyPromise : {
-            get : function() {  // eslint-disable-line getter-return
+            // eslint-disable-next-line getter-return
+            get : function() {
                 DeveloperError.throwInstantiationError();
             }
         },
@@ -160,7 +168,8 @@ import DeveloperError from '../Core/DeveloperError.js';
          * @readonly
          */
         tileset : {
-            get : function() {  // eslint-disable-line getter-return
+            // eslint-disable-next-line getter-return
+            get : function() {
                 DeveloperError.throwInstantiationError();
             }
         },
@@ -174,7 +183,8 @@ import DeveloperError from '../Core/DeveloperError.js';
          * @readonly
          */
         tile : {
-            get : function() {  // eslint-disable-line getter-return
+            // eslint-disable-next-line getter-return
+            get : function() {
                 DeveloperError.throwInstantiationError();
             }
         },
@@ -187,7 +197,8 @@ import DeveloperError from '../Core/DeveloperError.js';
          * @readonly
          */
         url : {
-            get : function() {  // eslint-disable-line getter-return
+            // eslint-disable-next-line getter-return
+            get : function() {
                 DeveloperError.throwInstantiationError();
             }
         },
@@ -205,7 +216,8 @@ import DeveloperError from '../Core/DeveloperError.js';
          * @private
          */
         batchTable : {
-            get : function() {  // eslint-disable-line getter-return
+            // eslint-disable-next-line getter-return
+            get : function() {
                 DeveloperError.throwInstantiationError();
             }
         }
diff --git a/Source/Scene/Polyline.js b/Source/Scene/Polyline.js
@@ -73,7 +73,8 @@ import Material from './Material.js';
 
         this._actualLength = undefined;
 
-        this._propertiesChanged = new Uint32Array(NUMBER_OF_PROPERTIES); //eslint-disable-line no-use-before-define
+        // eslint-disable-next-line no-use-before-define
+        this._propertiesChanged = new Uint32Array(NUMBER_OF_PROPERTIES);
         this._polylineCollection = polylineCollection;
         this._dirty = false;
         this._pickId = undefined;
diff --git a/Source/Widgets/Cesium3DTilesInspector/Cesium3DTilesInspectorViewModel.js b/Source/Widgets/Cesium3DTilesInspector/Cesium3DTilesInspectorViewModel.js
@@ -34,7 +34,8 @@ import knockout from '../../ThirdParty/knockout.js';
             viewModel._eventHandler.removeInputAction(ScreenSpaceEventType.MOUSE_MOVE);
 
             // Restore hover-over selection to its current value
-            viewModel.picking = viewModel.picking; // eslint-disable-line no-self-assign
+            // eslint-disable-next-line no-self-assign
+            viewModel.picking = viewModel.picking;
         }
     }
 
@@ -321,6 +322,7 @@ import knockout from '../../ThirdParty/knockout.js';
          */
         this.colorBlendMode = Cesium3DTileColorBlendMode.HIGHLIGHT;
 
+        var showOnlyPickedTileDebugLabel = knockout.observable();
         var picking = knockout.observable();
         knockout.defineProperty(this, 'picking', {
             get : function() {
@@ -347,7 +349,7 @@ import knockout from '../../ThirdParty/knockout.js';
                         if (!defined(that._tileset)) {
                             return;
                         }
-                        if (showOnlyPickedTileDebugLabel && defined(picked) && defined(picked.content)) { //eslint-disable-line no-use-before-define
+                        if (showOnlyPickedTileDebugLabel && defined(picked) && defined(picked.content)) {
                             var position;
                             if (scene.pickPositionSupported) {
                                 position = scene.pickPosition(e.endPosition);
@@ -503,7 +505,6 @@ import knockout from '../../ThirdParty/knockout.js';
          */
         this.freezeFrame = false;
 
-        var showOnlyPickedTileDebugLabel = knockout.observable();
         knockout.defineProperty(this, 'showOnlyPickedTileDebugLabel', {
             get : function() {
                 return showOnlyPickedTileDebugLabel();
@@ -1126,7 +1127,8 @@ import knockout from '../../ThirdParty/knockout.js';
                     var length = settings.length;
                     for (var i = 0; i < length; ++i) {
                         var setting = settings[i];
-                        this[setting] = this[setting]; // eslint-disable-line no-self-assign
+                        //eslint-disable-next-line no-self-assign
+                        this[setting] = this[setting];
                     }
 
                     // update view model with existing tileset settings
diff --git a/Source/Workers/transferTypedArrayTest.js b/Source/Workers/transferTypedArrayTest.js
@@ -1,6 +1,7 @@
 // make sure self is defined so that the Dojo build can evaluate this file without crashing.
 if (typeof self === 'undefined') {
-    self = {}; //eslint-disable-line no-implicit-globals, no-global-assign
+    //eslint-disable-next-line no-implicit-globals, no-global-assign
+    self = {};
 }
 
 self.onmessage = function(event) {
diff --git a/Specs/Scene/ModelSpec.js b/Specs/Scene/ModelSpec.js
@@ -2494,7 +2494,8 @@ describe('Scene/Model', function() {
             }
             Matrix4.multiplyByMatrix3(m.modelMatrix, rotate, m.modelMatrix);
 
-            expect(scene).toRenderAndCall(function(rgba) { //eslint-disable-line no-loop-func
+            //eslint-disable-next-line no-loop-func
+            expect(scene).toRenderAndCall(function(rgba) {
                 expect(rgba).not.toEqual([0, 0, 0, 255]);
                 expect(rgba).not.toEqual(oldPixelColor);
                 oldPixelColor = rgba;
diff --git a/Specs/Scene/PointCloud3DTileContentSpec.js b/Specs/Scene/PointCloud3DTileContentSpec.js
@@ -448,7 +448,8 @@ describe('Scene/PointCloud3DTileContent', function() {
                 /* jshint loopfunc: true */
                 while (defined(picked)) {
                     picked.show = false;
-                    expect(scene).toPickAndCall(function(result) { //eslint-disable-line no-loop-func
+                    //eslint-disable-next-line no-loop-func
+                    expect(scene).toPickAndCall(function(result) {
                         picked = result;
                     });
                     ++pickedCountCulling;
@@ -471,7 +472,8 @@ describe('Scene/PointCloud3DTileContent', function() {
                 /* jshint loopfunc: true */
                 while (defined(picked)) {
                     picked.show = false;
-                    expect(scene).toPickAndCall(function(result) { //eslint-disable-line no-loop-func
+                    //eslint-disable-next-line no-loop-func
+                    expect(scene).toPickAndCall(function(result) {
                         picked = result;
                     });
                     ++pickedCount;
diff --git a/Specs/Scene/ShadowMapSpec.js b/Specs/Scene/ShadowMapSpec.js
diff --git a/package.json b/package.json

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`{`
`2`		`- "extends": "./Tools/eslint-config-cesium/browser.js",`
	`2`	`+ "extends": ["./Tools/eslint-config-cesium/browser.js", "prettier"],`
`3`	`3`	`"plugins": [`
`4`	`4`	`"html"`
`5`	`5`	`],`
Original file line number	Diff line number	Diff line change
`@@ -71,7 +71,8 @@ function main() {`
`71`	`71`	`var message = formatError(exception);`
`72`	`72`	`console.error(message);`
`73`	`73`	`if (!document.querySelector('.cesium-widget-errorPanel')) {`
`74`		`- window.alert(message); //eslint-disable-line no-alert`
	`74`	`+ //eslint-disable-next-line no-alert`
	`75`	`+ window.alert(message);`
`75`	`76`	`}`
`76`	`77`	`return;`
`77`	`78`	`}`
Original file line number	Diff line number	Diff line change
`@@ -758,6 +758,7 @@ require({`
`758`	`758`	`}`
`759`	`759`
`760`	`760`	`var scriptCode = scriptMatch[1];`
	`761`	`+ scriptCode = scriptCode.replace(/^ {8}/gm, ""); //Account for Prettier spacing`
`761`	`762`
`762`	`763`	`var htmlText = '';`
`763`	`764`	`var childIndex = 0;`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,8 @@ import Resource from './Resource.js';`
`31`	`31`	`a.href = url;`
`32`	`32`
`33`	`33`	`// IE only absolutizes href on get, not set`
`34`		`- a.href = a.href; // eslint-disable-line no-self-assign`
	`34`	`+ // eslint-disable-next-line no-self-assign`
	`35`	`+ a.href = a.href;`
`35`	`36`	`return a.href;`
`36`	`37`	`}`
`37`	`38`
Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,8 @@ import defined from './defined.js';`
`22`	`22`
`23`	`23`	`a.href = url;`
`24`	`24`	`// IE only absolutizes href on get, not set`
`25`		`- a.href = a.href; // eslint-disable-line no-self-assign`
	`25`	`+ // eslint-disable-next-line no-self-assign`
	`26`	`+ a.href = a.href;`
`26`	`27`
`27`	`28`	`return protocol !== a.protocol \|\| host !== a.host;`
`28`	`29`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,7 @@`
`1`	`1`	`// make sure self is defined so that the Dojo build can evaluate this file without crashing.`
`2`	`2`	`if (typeof self === 'undefined') {`
`3`		`- self = {}; //eslint-disable-line no-implicit-globals, no-global-assign`
	`3`	`+ //eslint-disable-next-line no-implicit-globals, no-global-assign`
	`4`	`+ self = {};`
`4`	`5`	`}`
`5`	`6`
`6`	`7`	`self.onmessage = function(event) {`
Original file line number	Diff line number	Diff line change
`@@ -2494,7 +2494,8 @@ describe('Scene/Model', function() {`
`2494`	`2494`	`}`
`2495`	`2495`	`Matrix4.multiplyByMatrix3(m.modelMatrix, rotate, m.modelMatrix);`
`2496`	`2496`
`2497`		`- expect(scene).toRenderAndCall(function(rgba) { //eslint-disable-line no-loop-func`
	`2497`	`+ //eslint-disable-next-line no-loop-func`
	`2498`	`+ expect(scene).toRenderAndCall(function(rgba) {`
`2498`	`2499`	`expect(rgba).not.toEqual([0, 0, 0, 255]);`
`2499`	`2500`	`expect(rgba).not.toEqual(oldPixelColor);`
`2500`	`2501`	`oldPixelColor = rgba;`