Repository: jlongster/prettier (js)

Issue:
JSX ternary is strange
```diff
-            {__DEV__ ? this.renderDevApp() : (
-              <div>
-                {routes.map((route) =>
-                  <MatchAsync
-                    key={`${route.to}-async`}
-                    pattern={route.to}
-                    exactly={route.to === '/'}
-                    getComponent={routeES6Modules[route.value]}
-                  />,
-                )}
-              </div>
-            )}
+            {__DEV__ ? this.renderDevApp() : <div>
+                  {
+                    routes.map(route => (
+                      <MatchAsync
+                        key={`${route.to}-async`}
+                        pattern={route.to}
+                        exactly={route.to === '/'}
+                        getComponent={routeES6Modules[route.value]}
+                      />
+                    ))
+                  }
+                </div>}
```

Just ran prettier on the codebase at work after your 0.0.9 release and this is a really strange diff.

https://jlongster.github.io/prettier/#%7B%22content%22%3A%22class%20App%20extends%20PureComponent%20%7B%5Cn%20%20props%3A%20Props%3B%5Cn%5Cn%20%20render()%20%7B%5Cn%20%20%20%20return%20(%5Cn%20%20%20%20%20%20%3Cdiv%20className%3D%7Bstyles.container%7D%3E%5Cn%20%20%20%20%20%20%20%20%3Cdiv%20className%3D%7Bstyles.content%7D%3E%5Cn%20%20%20%20%20%20%20%20%20%20%3CHeader%20%2F%3E%5Cn%20%20%20%20%20%20%20%20%20%20%7B__DEV__%20%3F%20this.renderDevApp()%20%3A%20(%5Cn%20%20%20%20%20%20%20%20%20%20%20%20%3Cdiv%3E%5Cn%20%20%20%20%20%20%20%20%20%20%20%20%20%20%7Broutes.map((route)%20%3D%3E%5Cn%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%3CMatchAsync%5Cn%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20key%3D%7B%60%24%7Broute.to%7D-async%60%7D%5Cn%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20pattern%3D%7Broute.to%7D%5Cn%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20exactly%3D%7Broute.to%20%3D%3D%3D%20'%2F'%7D%5Cn%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20getComponent%3D%7BrouteES6Modules%5Broute.value%5D%7D%5Cn%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%2F%3E%2C%5Cn%20%20%20%20%20%20%20%20%20%20%20%20%20%20)%7D%5Cn%20%20%20%20%20%20%20%20%20%20%20%20%3C%2Fdiv%3E%5Cn%20%20%20%20%20%20%20%20%20%20)%7D%5Cn%20%20%20%20%20%20%20%20%3C%2Fdiv%3E%5Cn%20%20%20%20%20%20%3C%2Fdiv%3E%5Cn%20%20%20%20)%3B%5Cn%20%20%7D%5Cn%7D%5Cn%22%2C%22options%22%3A%7B%22printWidth%22%3A80%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Atrue%2C%22trailingComma%22%3Atrue%2C%22bracketSpacing%22%3Afalse%7D%7D

Pull request:
[RFC] Hug Conditionals in JSX
Conditionals are very common in JSX and it is unfortunate that they take up so much vertical space in the current prettier.

This pull request does a few tweaks:
 - It hugs ConditionalExpression (ternary) and LogicalExpression (&&) inside of `{}` in a jsx child, not an attribute
 - It doesn't output parenthesis if your parent is a LogicalExpression (&&)

Fixes #317

Patch:
diff --git a/bin/prettier.js b/bin/prettier.js
index 137b71d46..860830802 100755
--- a/bin/prettier.js
+++ b/bin/prettier.js
@@ -19,16 +19,19 @@ const argv = minimist(process.argv.slice(2), {
     // listed here is to avoid "Ignored unknown option: --no-color" warnings.
     // See https://github.com/chalk/supports-color/#info for more information.
     "color",
+    "help",
     "version",
     "debug-print-doc",
     // Deprecated in 0.0.10
     "flow-parser"
   ],
-  string: [ "print-width", "tab-width", "parser" ],
+  string: ["print-width", "tab-width", "parser"],
   default: { color: true, "bracket-spacing": true, parser: "babylon" },
+  alias: { help: "h", version: "v" },
   unknown: param => {
     if (param.startsWith("-")) {
       console.warn("Ignored unknown option: " + param + "\n");
+      return false;
     }
   }
 });
@@ -40,9 +43,9 @@ if (argv["version"]) {
 
 const filepatterns = argv["_"];
 const write = argv["write"];
-const stdin = argv["stdin"];
+const stdin = argv["stdin"] || !filepatterns.length && !process.stdin.isTTY;
 
-if (!filepatterns.length && !stdin) {
+if (argv["help"] || !filepatterns.length && !stdin) {
   console.log(
     "Usage: prettier [opts] [filename ...]\n\n" +
       "Available options:\n" +
@@ -61,28 +64,63 @@ if (!filepatterns.length && !stdin) {
       "  --no-bracket-spacing\n" +
       "  --bracket-spacing=false"
   );
-  process.exit(1);
+  process.exit(argv["help"] ? 0 : 1);
 }
 
-function getParser() {
+function getParserOption() {
+  const optionName = "parser";
+  const value = argv[optionName];
+
+  if (value === undefined) {
+    return value;
+  }
+
   // For backward compatibility. Deprecated in 0.0.10
   if (argv["flow-parser"]) {
     console.warn("`--flow-parser` is deprecated. Use `--parser flow` instead.");
     return "flow";
   }
 
-  if (argv["parser"] === "flow") {
-    return "flow";
+  if (value === "flow" || value === "babylon") {
+    return value;
   }
 
+  console.warn(
+    "Ignoring unknown --" +
+      optionName +
+      ' value, falling back to "babylon":\n' +
+      '  Expected "flow" or "babylon", but received: ' +
+      JSON.stringify(value)
+  );
+
   return "babylon";
 }
 
+function getIntOption(optionName) {
+  const value = argv[optionName];
+
+  if (value === undefined) {
+    return value;
+  }
+
+  if (/^\d+$/.test(value)) {
+    return Number(value);
+  }
+
+  console.error(
+    "Invalid --" +
+      optionName +
+      " value. Expected an integer, but received: " +
+      JSON.stringify(value)
+  );
+  process.exit(1);
+}
+
 const options = {
-  printWidth: argv["print-width"] && parseInt(argv["print-width"]),
-  tabWidth: argv["tab-width"] && parseInt(argv["tab-width"]),
+  printWidth: getIntOption("print-width"),
+  tabWidth: getIntOption("tab-width"),
   bracketSpacing: argv["bracket-spacing"],
-  parser: getParser(),
+  parser: getParserOption(),
   singleQuote: argv["single-quote"],
   trailingComma: argv["trailing-comma"]
 };
@@ -95,14 +133,37 @@ function format(input) {
   return prettier.format(input, options);
 }
 
+function handleError(filename, e) {
+  const isParseError = Boolean(e && e.loc);
+  const isValidationError = /Validation Error/.test(e && e.message);
+
+  // For parse errors and validation errors, we only want to show the error
+  // message formatted in a nice way. `String(e)` takes care of that. Other
+  // (unexpected) errors are passed as-is as a separate argument to
+  // `console.error`. That includes the stack trace (if any), and shows a nice
+  // `util.inspect` of throws things that aren't `Error` objects. (The Flow
+  // parser has mistakenly thrown arrays sometimes.)
+  if (isParseError) {
+    console.error(filename + ": " + String(e));
+  } else if (isValidationError) {
+    console.error(String(e));
+    // If validation fails for one file, it will fail for all of them.
+    process.exit(1);
+  } else {
+    console.error(filename + ":", e);
+  }
+
+  // Don't exit the process if one file failed
+  process.exitCode = 2;
+}
+
 if (stdin) {
   getStdin().then(input => {
     try {
       // Don't use `console.log` here since it adds an extra newline at the end.
       process.stdout.write(format(input));
     } catch (e) {
-      process.exitCode = 2;
-      console.error("stdin: " + e);
+      handleError("stdin", e);
       return;
     }
   });
@@ -124,13 +185,7 @@ if (stdin) {
       try {
         output = format(input);
       } catch (e) {
-        process.exitCode = 2;
-        if(e.loc) {
-          console.error(filename + ": " + e);
-        }
-        else {
-          console.error(filename + ":", e);
-        }
+        handleError(filename, e);
         return;
       }
 
diff --git a/src/printer.js b/src/printer.js
index 9681b45aa..8332acf48 100644
--- a/src/printer.js
+++ b/src/printer.js
@@ -1092,14 +1092,20 @@ function genericPrintNoParens(path, options, print) {
       ]);
     case "JSXSpreadAttribute":
       return concat([ "{...", path.call(print, "argument"), "}" ]);
-    case "JSXExpressionContainer":
-      const shouldIndent = n.expression.type !== "ArrayExpression" &&
-        n.expression.type !== "ObjectExpression" &&
-        n.expression.type !== "ArrowFunctionExpression" &&
-        n.expression.type !== "CallExpression" &&
-        n.expression.type !== "FunctionExpression";
-
-      if (!shouldIndent) {
+    case "JSXExpressionContainer": {
+      const parent = path.getParentNode(0);
+
+      const shouldInline = n.expression.type === "ArrayExpression" ||
+        n.expression.type === "ObjectExpression" ||
+        n.expression.type === "ArrowFunctionExpression" ||
+        n.expression.type === "CallExpression" ||
+        n.expression.type === "FunctionExpression" ||
+        parent.type === "JSXElement" && (
+          n.expression.type === "ConditionalExpression" ||
+          n.expression.type === "LogicalExpression"
+        );
+
+      if (shouldInline) {
         return concat([ "{", path.call(print, "expression"), "}" ]);
       }
 
@@ -1114,6 +1120,7 @@ function genericPrintNoParens(path, options, print) {
           "}"
         ])
       );
+    }
     case "JSXElement": {
       const elem = printJSXElement(path, options, print);
       return maybeWrapJSXElementInParens(path, elem, options);
@@ -2311,7 +2318,8 @@ function maybeWrapJSXElementInParens(path, elem, options) {
     JSXElement: true,
     ExpressionStatement: true,
     CallExpression: true,
-    ConditionalExpression: true
+    ConditionalExpression: true,
+    LogicalExpression: true,
   };
   if (NO_WRAP_PARENTS[parent.type]) {
     return elem;


Test patch:
diff --git a/tests/jsx-stateless-arrow-fn/__snapshots__/jsfmt.spec.js.snap b/tests/jsx-stateless-arrow-fn/__snapshots__/jsfmt.spec.js.snap
index 64b8e80ee..73b3b59d4 100644
--- a/tests/jsx-stateless-arrow-fn/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/jsx-stateless-arrow-fn/__snapshots__/jsfmt.spec.js.snap
@@ -190,45 +190,39 @@ const renderTernary = props => (
     size=\"large\"
     submitLabel=\"Sign in with Google\"
   >
-    {
-      props.showTheThing
-        ? <BaseForm
-          url=\"/auth/google\"
-          method=\"GET\"
-          colour=\"blue\"
-          size=\"large\"
-          submitLabel=\"Sign in with Google\"
-        >
-          Hello world
-        </BaseForm>
-        : \"hello \" + \"howdy! \"
-    }
-    {
-      props.showTheThing
-        ? <BaseForm
-          url=\"/auth/google\"
-          method=\"GET\"
-          colour=\"blue\"
-          size=\"large\"
-          submitLabel=\"Sign in with Google\"
-        >
-          Hello world
-        </BaseForm>
-        : null
-    }
-    {
-      props.showTheThing
-        ? null
-        : <BaseForm
-          url=\"/auth/google\"
-          method=\"GET\"
-          colour=\"blue\"
-          size=\"large\"
-          submitLabel=\"Sign in with Google\"
-        >
-          Hello world
-        </BaseForm>
-    }
+    {props.showTheThing
+      ? <BaseForm
+        url=\"/auth/google\"
+        method=\"GET\"
+        colour=\"blue\"
+        size=\"large\"
+        submitLabel=\"Sign in with Google\"
+      >
+        Hello world
+      </BaseForm>
+      : \"hello \" + \"howdy! \"}
+    {props.showTheThing
+      ? <BaseForm
+        url=\"/auth/google\"
+        method=\"GET\"
+        colour=\"blue\"
+        size=\"large\"
+        submitLabel=\"Sign in with Google\"
+      >
+        Hello world
+      </BaseForm>
+      : null}
+    {props.showTheThing
+      ? null
+      : <BaseForm
+        url=\"/auth/google\"
+        method=\"GET\"
+        colour=\"blue\"
+        size=\"large\"
+        submitLabel=\"Sign in with Google\"
+      >
+        Hello world
+      </BaseForm>}
     {props.showTheOtherThing ? <div>I am here</div> : <div attr=\"blah\" />}
     {props.showTheOtherThing ? <div>I am here!!</div> : null}
   </BaseForm>
diff --git a/tests/jsx/__snapshots__/jsfmt.spec.js.snap b/tests/jsx/__snapshots__/jsfmt.spec.js.snap
index 98fd7bb7d..0e16d3f46 100644
--- a/tests/jsx/__snapshots__/jsfmt.spec.js.snap
+++ b/tests/jsx/__snapshots__/jsfmt.spec.js.snap
@@ -123,6 +123,66 @@ exports[`test expression.js 1`] = `
 "
 `;
 
+exports[`test hug.js 1`] = `
+"<div>
+  {__DEV__
+    ? this.renderDevApp()
+    : <div>
+      {routes.map(route => (
+        <MatchAsync
+          key={\`\${route.to}-async\`}
+          pattern={route.to}
+          exactly={route.to === \"/\"}
+          getComponent={routeES6Modules[route.value]}
+        />
+      ))}
+    </div>}
+</div>;
+
+<div>
+  {__DEV__ && <div>
+    {routes.map(route => (
+      <MatchAsync
+        key={\`\${route.to}-async\`}
+        pattern={route.to}
+        exactly={route.to === \"/\"}
+        getComponent={routeES6Modules[route.value]}
+      />
+    ))}
+    </div>}
+</div>;
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+<div>
+  {__DEV__
+    ? this.renderDevApp()
+    : <div>
+      {routes.map(route => (
+        <MatchAsync
+          key={\`\${route.to}-async\`}
+          pattern={route.to}
+          exactly={route.to === \"/\"}
+          getComponent={routeES6Modules[route.value]}
+        />
+      ))}
+    </div>}
+</div>;
+
+<div>
+  {__DEV__ &&
+    <div>
+      {routes.map(route => (
+        <MatchAsync
+          key={\`\${route.to}-async\`}
+          pattern={route.to}
+          exactly={route.to === \"/\"}
+          getComponent={routeES6Modules[route.value]}
+        />
+      ))}
+    </div>}
+</div>;
+"
+`;
+
 exports[`test quotes.js 1`] = `
 "<div id=\"&quot;\'<>&amp;quot;\" />;
 <div id=\'\"&#39;<>&amp;quot;\' />;
diff --git a/tests/jsx/hug.js b/tests/jsx/hug.js
new file mode 100644
index 000000000..2652899e6
--- /dev/null
+++ b/tests/jsx/hug.js
@@ -0,0 +1,27 @@
+<div>
+  {__DEV__
+    ? this.renderDevApp()
+    : <div>
+      {routes.map(route => (
+        <MatchAsync
+          key={`${route.to}-async`}
+          pattern={route.to}
+          exactly={route.to === "/"}
+          getComponent={routeES6Modules[route.value]}
+        />
+      ))}
+    </div>}
+</div>;
+
+<div>
+  {__DEV__ && <div>
+    {routes.map(route => (
+      <MatchAsync
+        key={`${route.to}-async`}
+        pattern={route.to}
+        exactly={route.to === "/"}
+        getComponent={routeES6Modules[route.value]}
+      />
+    ))}
+    </div>}
+</div>;