Repository: jlongster/prettier
Pull request #689 (2017-02-14 18:26:36)

[RFC] Remove parenthesis object special case
I've started the discussion around it but I think that it was a mistake. It feels weird to add parenthesis on object values sometimes but not others. I think that it's best to let prettier do its work on staying under 80 columns based on the rules we added.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 26a06a9de..971bc8415 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,4 +1,6 @@
-# 0.17.0
+# 0.17.1
+
+* Use `readline` api to manipulate `process.stdout` output. (#687)
 
 [link](https://github.com/jlongster/prettier/compare/0.16.0...0.17.0)
 
diff --git a/bin/prettier.js b/bin/prettier.js
index e108e2942..608b03af0 100755
--- a/bin/prettier.js
+++ b/bin/prettier.js
@@ -7,6 +7,7 @@ const getStdin = require("get-stdin");
 const glob = require("glob");
 const chalk = require("chalk");
 const minimist = require("minimist");
+const readline = require("readline");
 const prettier = require("../index");
 
 const argv = minimist(process.argv.slice(2), {
@@ -218,8 +219,8 @@ if (stdin) {
 
       if (write) {
         // Remove previously printed filename to log it with duration.
-        process.stdout.clearLine();
-        process.stdout.cursorTo(0);
+        readline.clearLine(process.stdout, 0);
+        readline.cursorTo(process.stdout, 0, null);
 
         // Don't write the file if it won't change in order not to invalidate
         // mtime based caches.
diff --git a/docs/index.html b/docs/index.html
index 1bbb9d15f..967e7c1b2 100644
--- a/docs/index.html
+++ b/docs/index.html
@@ -131,6 +131,20 @@ function getOptions() {
   return options;
 }
 
+function replaceHash(hash) {
+  if (
+    typeof URL === "function" &&
+    typeof history === "object" &&
+    typeof history.replaceState === "function"
+  ) {
+    var url = new URL(location);
+    url.hash = hash;
+    history.replaceState(null, null, url);
+  } else {
+    location.hash = hash;
+  }
+}
+
 function format() {
   var options = getOptions();
   [docEditor, outputEditor].forEach(function(editor) {
@@ -146,7 +160,7 @@ function format() {
       Object.assign({content: inputEditor.getValue(), options: options})
     )
   );
-  location.hash = value;
+  replaceHash(value);
   var res;
   try {
     res = prettier.format(inputEditor.getValue(), options);
diff --git a/package.json b/package.json
index 26b0f2535..69d152810 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "prettier",
-  "version": "0.17.0",
+  "version": "0.17.1",
   "description": "Prettier is an opinionated JavaScript formatter",
   "bin": {
     "prettier": "./bin/prettier.js"
diff --git a/src/printer.js b/src/printer.js
index bca622e86..0acad6736 100644
--- a/src/printer.js
+++ b/src/printer.js
@@ -185,7 +185,8 @@ function genericPrintNoParens(path, options, print) {
       const parent = path.getParentNode();
 
       // Avoid indenting sub-expressions in if/etc statements.
-      if(
+      if (
+        shouldInlineLogicalExpression(n) ||
         (parent.type === "IfStatement" ||
          parent.type === "WhileStatement" ||
          parent.type === "DoStatement" ||
@@ -703,21 +704,7 @@ function genericPrintNoParens(path, options, print) {
         } else {
           parts.push(printPropertyKey(path, options, print));
         }
-
-        let printedValue = path.call(print, "value");
-        if (shouldPrintSameLine(n.value)) {
-          parts.push(concat([": ", printedValue]));
-        } else {
-          parts.push(
-            group(concat([
-              ":",
-              ifBreak(" (", " "),
-              indent(options.tabWidth, concat([softline, printedValue])),
-              softline,
-              ifBreak(")")
-            ]))
-          );
-        }
+        parts.push(concat([": ", path.call(print, "value")]));
       }
 
       return concat(parts); // Babel 6
@@ -1828,8 +1815,7 @@ function printArgumentsList(path, options, print) {
           lastArg.body.type === "ObjectExpression" ||
           lastArg.body.type === "ArrayExpression" ||
           lastArg.body.type === "CallExpression" ||
-          lastArg.body.type === "JSXElement") ||
-      lastArg.type === "NewExpression");
+          lastArg.body.type === "JSXElement"));
 
   if (groupLastArg) {
     const shouldBreak = printed.slice(0, -1).some(willBreak);
@@ -2381,7 +2367,9 @@ function printJSXChildren(path, options, print, jsxWhitespace) {
             }
           });
 
-          children.push(softline);
+          if (!isLineNext(util.getLast(children))) {
+            children.push(softline);
+          }
         } else if (/\n/.test(value)) {
           children.push(hardline);
 
@@ -2572,6 +2560,13 @@ function isBinaryish(node) {
   return node.type === "BinaryExpression" || node.type === "LogicalExpression";
 }
 
+function shouldInlineLogicalExpression(node) {
+  return node.type === "LogicalExpression" && (
+    node.right.type === "ObjectExpression" ||
+    node.right.type === "ArrayExpression"
+  );
+}
+
 // For binary expressions to be consistent, we need to group
 // subsequent operators with the same precedence level under a single
 // group. Otherwise they will be nested such that some of them break
@@ -2614,7 +2609,11 @@ function printBinaryishExpressions(path, parts, print, options, isNested) {
       parts.push(path.call(print, "left"));
     }
 
-    const right = concat([node.operator, line, path.call(print, "right")]);
+    const right = concat([
+      node.operator,
+      shouldInlineLogicalExpression(node) ? " " : line,
+      path.call(print, "right")
+    ]);
 
     // If there's only a single binary expression: everything except && and ||,
     // we want to create a group in order to avoid having a small right part
@@ -2782,28 +2781,6 @@ function isObjectTypePropertyAFunction(node) {
     util.locStart(node.key) !== util.locStart(node.value);
 }
 
-function shouldPrintSameLine(node) {
-  const type = node.type;
-  return namedTypes.Literal.check(node) ||
-    type === "ArrayExpression" ||
-    type === "ArrayPattern" ||
-    type === "ArrowFunctionExpression" ||
-    type === "AssignmentPattern" ||
-    type === "CallExpression" ||
-    type === "FunctionExpression" ||
-    type === "Identifier" ||
-    type === "JSXElement" ||
-    type === "Literal" ||
-    type === "MemberExpression" ||
-    type === "NewExpression" ||
-    type === "ObjectExpression" ||
-    type === "ObjectPattern" ||
-    type === "StringLiteral" ||
-    type === "ThisExpression" ||
-    type === "TypeCastExpression" ||
-    type === "UnaryExpression";
-}
-
 function isFlowNodeStartingWithDeclare(node, options) {
   if (options.parser !== "flow") {
     return false;


Test patch:
diff --git a/tests/binary-expressions/__snapshots__/jsfmt.spec.js.snap b/tests/binary-expressions/__snapshots__/jsfmt.spec.js.snap
index 31d4ce314..5ce0e6c95 100644
--- a/tests/binary-expressions/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/binary-expressions/__snapshots__/jsfmt.spec.js.snap
@@ -1,3 +1,36 @@
+exports[`test inline-object-array.js 1`] = `
+"prevState = prevState || {
+  catalogs: [],
+  loadState: LOADED,
+  opened: false,
+  searchQuery: \'\',
+  selectedCatalog: null,
+};
+
+this.steps = steps || [
+  {
+    name: \'mock-module\',
+    path: \'/nux/mock-module\',
+  },
+];
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+prevState = prevState || {
+  catalogs: [],
+  loadState: LOADED,
+  opened: false,
+  searchQuery: \"\",
+  selectedCatalog: null
+};
+
+this.steps = steps || [
+  {
+    name: \"mock-module\",
+    path: \"/nux/mock-module\"
+  }
+];
+"
+`;
+
 exports[`test short-right.js 1`] = `
 "this._cumulativeHeights &&
  Math.abs(
diff --git a/tests/binary-expressions/inline-object-array.js b/tests/binary-expressions/inline-object-array.js
new file mode 100644
index 000000000..76d47eabc
--- /dev/null
+++ b/tests/binary-expressions/inline-object-array.js
@@ -0,0 +1,14 @@
+prevState = prevState || {
+  catalogs: [],
+  loadState: LOADED,
+  opened: false,
+  searchQuery: '',
+  selectedCatalog: null,
+};
+
+this.steps = steps || [
+  {
+    name: 'mock-module',
+    path: '/nux/mock-module',
+  },
+];
diff --git a/tests/flow/promises/__snapshots__/jsfmt.spec.js.snap b/tests/flow/promises/__snapshots__/jsfmt.spec.js.snap
index 62b8c4e86..abd7104a9 100644
--- a/tests/flow/promises/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/flow/promises/__snapshots__/jsfmt.spec.js.snap
@@ -376,9 +376,11 @@ new Promise((resolve, reject) => resolve(0)).then(function(num) {
 
 // Promise constructor resolve(Promise<T>) -> then(T)
 new Promise(function(resolve, reject) {
-  resolve(new Promise(function(resolve, reject) {
-    resolve(0);
-  }));
+  resolve(
+    new Promise(function(resolve, reject) {
+      resolve(0);
+    })
+  );
 }).then(function(num) {
   var a: number = num;
   var b: string = num; // Error: number ~> string
@@ -386,11 +388,15 @@ new Promise(function(resolve, reject) {
 
 // Promise constructor resolve(Promise<Promise<T>>) -> then(T)
 new Promise(function(resolve, reject) {
-  resolve(new Promise(function(resolve, reject) {
-    resolve(new Promise(function(resolve, reject) {
-      resolve(0);
-    }));
-  }));
+  resolve(
+    new Promise(function(resolve, reject) {
+      resolve(
+        new Promise(function(resolve, reject) {
+          resolve(0);
+        })
+      );
+    })
+  );
 }).then(function(num) {
   var a: number = num;
   var b: string = num; // Error: number ~> string
@@ -428,9 +434,11 @@ new Promise(function(resolve, reject) {
 
 // TODO: Promise constructor reject(Promise<T>) ~> catch(Promise<T>)
 new Promise(function(resolve, reject) {
-  reject(new Promise(function(resolve, reject) {
-    reject(0);
-  }));
+  reject(
+    new Promise(function(resolve, reject) {
+      reject(0);
+    })
+  );
 }).catch(function(num) {
   var a: Promise<number> = num;
 
diff --git a/tests/jsx-newlines/__snapshots__/jsfmt.spec.js.snap b/tests/jsx-newlines/__snapshots__/jsfmt.spec.js.snap
index e6550d323..f89e70fd4 100644
--- a/tests/jsx-newlines/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/jsx-newlines/__snapshots__/jsfmt.spec.js.snap
@@ -81,6 +81,18 @@ regression_extra_newline = (
     New Messages
   </div>
 );
+
+
+regression_extra_newline_2 = (
+  <div>
+    (
+    <FormattedMessage
+      id=\"some-id\"
+      defaultMessage=\"some loooooooooooooooooooooooooooong default\"
+    />
+    )
+  </div>
+);
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 keep = (
   <p>
@@ -160,5 +172,16 @@ regression_extra_newline = (
     New Messages
   </div>
 );
+
+regression_extra_newline_2 = (
+  <div>
+    (
+    <FormattedMessage
+      id=\"some-id\"
+      defaultMessage=\"some loooooooooooooooooooooooooooong default\"
+    />
+    )
+  </div>
+);
 "
 `;
diff --git a/tests/jsx-newlines/test.js b/tests/jsx-newlines/test.js
index 55b1ae44f..d5fedf9e3 100644
--- a/tests/jsx-newlines/test.js
+++ b/tests/jsx-newlines/test.js
@@ -80,3 +80,15 @@ regression_extra_newline = (
     New Messages
   </div>
 );
+
+
+regression_extra_newline_2 = (
+  <div>
+    (
+    <FormattedMessage
+      id="some-id"
+      defaultMessage="some loooooooooooooooooooooooooooong default"
+    />
+    )
+  </div>
+);
diff --git a/tests/object-prop-break-in/__snapshots__/jsfmt.spec.js.snap b/tests/object-prop-break-in/__snapshots__/jsfmt.spec.js.snap
index cb925d852..7bb3a515e 100644
--- a/tests/object-prop-break-in/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/object-prop-break-in/__snapshots__/jsfmt.spec.js.snap
@@ -49,9 +49,8 @@ const a = classnames({
 });
 
 const b = classnames({
-  \"some-prop\": (
-    this.state.longLongLongLongLongLongLongLongLongTooLongProp === true
-  )
+  \"some-prop\": this.state.longLongLongLongLongLongLongLongLongTooLongProp ===
+    true
 });
 
 const c = classnames({
@@ -71,9 +70,8 @@ const f = classnames({
 });
 
 const g = classnames({
-  \"some-prop\": (
-    longLongLongLongLongLongLongLongLongLongLongLongLongTooLongVar || 1337
-  )
+  \"some-prop\": longLongLongLongLongLongLongLongLongLongLongLongLongTooLongVar ||
+    1337
 });
 "
 `;
diff --git a/tests/object_colon_bug/__snapshots__/jsfmt.spec.js.snap b/tests/object_colon_bug/__snapshots__/jsfmt.spec.js.snap
index a2cc5d93c..6e02fe691 100644
--- a/tests/object_colon_bug/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/object_colon_bug/__snapshots__/jsfmt.spec.js.snap
@@ -5,9 +5,9 @@ exports[`test bug.js 1`] = `
 }
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 const foo = {
-  bar: (
-    props.bar ? props.bar : noop
-  ),
+  bar: props.bar
+    ? props.bar
+    : noop,
   baz: props.baz
 };
 "