Skip to content

Commit

Permalink
Merge pull request #8744 from CesiumGS/lets-argue-over-minutiae
Browse files Browse the repository at this point in the history
Add configuration for prettier code formatting
  • Loading branch information
kring authored Apr 17, 2020
2 parents 0b48ba8 + d77b348 commit 05380a6
Show file tree
Hide file tree
Showing 19 changed files with 92 additions and 89 deletions.
8 changes: 1 addition & 7 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"extends": "./Tools/eslint-config-cesium/browser.js",
"extends": ["./Tools/eslint-config-cesium/browser.js", "prettier"],
"plugins": [
"html"
],
Expand Down
11 changes: 11 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion Apps/CesiumViewer/CesiumViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions Apps/Sandcastle/CesiumSandcastle.js
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,7 @@ require({
}

var scriptCode = scriptMatch[1];
scriptCode = scriptCode.replace(/^ {8}/gm, ""); //Account for Prettier spacing

var htmlText = '';
var childIndex = 0;
Expand Down
1 change: 0 additions & 1 deletion Apps/Sandcastle/gallery/CZML Custom Properties.html
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
<div id="toolbar">
<div id="propertiesMenu"></div>
</div>
</div>

<script id="cesium_sandcastle_script">
function startup(Cesium) {
Expand Down
55 changes: 6 additions & 49 deletions Documentation/Contributors/CodingGuide/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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);
}
```

Expand Down
12 changes: 8 additions & 4 deletions Source/Core/Resource.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion Source/Core/buildModuleUrl.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
3 changes: 2 additions & 1 deletion Source/Core/isCrossOriginUrl.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
36 changes: 24 additions & 12 deletions Source/Scene/Cesium3DTileContent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
},
Expand All @@ -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();
}
},
Expand All @@ -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();
}
},
Expand All @@ -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();
}
},
Expand All @@ -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();
}
},
Expand All @@ -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();
}
},
Expand All @@ -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();
}
},
Expand All @@ -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();
}
},
Expand All @@ -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();
}
},
Expand All @@ -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();
}
},
Expand All @@ -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();
}
},
Expand All @@ -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();
}
}
Expand Down
3 changes: 2 additions & 1 deletion Source/Scene/Polyline.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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() {
Expand All @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion Source/Workers/transferTypedArrayTest.js
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion Specs/Scene/ModelSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 4 additions & 2 deletions Specs/Scene/PointCloud3DTileContentSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
Loading

0 comments on commit 05380a6

Please sign in to comment.