Add parens around return for binaryish expressions #870

Merged
merged 1 commit into from Mar 19, 2017

Conversation

Projects
None yet
4 participants
@vjeux
Collaborator

vjeux commented Mar 3, 2017

It looks better when the first element is aligned with the rest and this is consistent with the way we render if test.

Fixes #866

+ 'Press `a` to run all tests, or run Jest with `--watchAll`.' :
+ 'Run Jest without `-o` to run all tests.',
+ )
+ );

This comment has been minimized.

@vjeux

vjeux Mar 3, 2017

Collaborator

This used to output

return chalk.bold(
  'No tests found related to files changed since last commit.\n',
) +
chalk.dim(
  patternInfo.watch ?
    'Press `a` to run all tests, or run Jest with `--watchAll`.' :
    'Run Jest without `-o` to run all tests.',
);

which looks broken.

@vjeux

vjeux Mar 3, 2017

Collaborator

This used to output

return chalk.bold(
  'No tests found related to files changed since last commit.\n',
) +
chalk.dim(
  patternInfo.watch ?
    'Press `a` to run all tests, or run Jest with `--watchAll`.' :
    'Run Jest without `-o` to run all tests.',
);

which looks broken.

This comment has been minimized.

@cpojer

This comment has been minimized.

Show comment
Hide comment
@cpojer

cpojer Mar 3, 2017

lovely. You are awesome.

cpojer commented Mar 3, 2017

lovely. You are awesome.

@vjeux vjeux referenced this pull request Mar 3, 2017

Closed

`return` printing #866

@jlongster

This comment has been minimized.

Show comment
Hide comment
@jlongster

jlongster Mar 3, 2017

Member

We need to talk more generally. There are other expressions which will break across lines like

return foo
  ? bar
  : baz

And I think we need to be consistent for all of them. I don't see why binary expressions should be special, but I also don't think we want to always add parens for anything that breaks. The reason we move it down in if is because there is indented code following that expression and it looks a lot clearer, but that's not the case for return.

Member

jlongster commented Mar 3, 2017

We need to talk more generally. There are other expressions which will break across lines like

return foo
  ? bar
  : baz

And I think we need to be consistent for all of them. I don't see why binary expressions should be special, but I also don't think we want to always add parens for anything that breaks. The reason we move it down in if is because there is indented code following that expression and it looks a lot clearer, but that's not the case for return.

@vjeux vjeux changed the title from Add parens around return for binaryish expressions to [RFC] Add parens around return for binaryish expressions Mar 3, 2017

@cpojer

This comment has been minimized.

Show comment
Hide comment
@cpojer

cpojer Mar 3, 2017

@jlongster I would probably vote for being consistent around lining things up.

return a &&
  b;
``

looks off, but

```return a
        ? b
        : c

lines up. Does that make sense?

cpojer commented Mar 3, 2017

@jlongster I would probably vote for being consistent around lining things up.

return a &&
  b;
``

looks off, but

```return a
        ? b
        : c

lines up. Does that make sense?

@jlongster jlongster changed the title from [RFC] Add parens around return for binaryish expressions to Add parens around return for binaryish expressions Mar 15, 2017

@jlongster

This comment has been minimized.

Show comment
Hide comment
@jlongster

jlongster Mar 17, 2017

Member

@cpojer I think that's a little aggressive to indent it so much and I'm never seen the style before. We do a similar thing with defining variables and it would be a lot of indentation with a longer name like const longVariableName = a .... It's nice to have a more consistent way to output this stuff on the next line regardless of where the initial expression is.

I talked to @vjeux about this PR and we'll start with this. If there are other expressions that don't work well on the same line as return or const we'll do a similar behavior of bumping the whole thing down.

If this is rebased it can be merged!

Member

jlongster commented Mar 17, 2017

@cpojer I think that's a little aggressive to indent it so much and I'm never seen the style before. We do a similar thing with defining variables and it would be a lot of indentation with a longer name like const longVariableName = a .... It's nice to have a more consistent way to output this stuff on the next line regardless of where the initial expression is.

I talked to @vjeux about this PR and we'll start with this. If there are other expressions that don't work well on the same line as return or const we'll do a similar behavior of bumping the whole thing down.

If this is rebased it can be merged!

Add parens around return for binaryish expressions
It looks better when the first element is aligned with the rest and this is consistent with the way we render `if` test.

Fixes #866
@vjeux

This comment has been minimized.

Show comment
Hide comment
@vjeux

vjeux Mar 19, 2017

Collaborator

Just rebased. I'm merging it.

Collaborator

vjeux commented Mar 19, 2017

Just rebased. I'm merging it.

@vjeux vjeux merged commit c749ddd into prettier:master Mar 19, 2017

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment