Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a --paren-spacing option to add whitespace according to WordPress style rules #2372

Closed
wants to merge 1 commit into from

Conversation

jsnajdr
Copy link
Contributor

@jsnajdr jsnajdr commented Jul 1, 2017

This PR adds a --paren-spacing option to Prettier that adds whitespace inside () and [] parens and inside {} in JSX attributes and string template arguments. For example:

function Comp( props ) {
  while ( ! a( 1, 2, [ 3, 4 ] ) ) {
    perform( p, e, { r, f } );
  }

  const label = `label: ${ props.name }`;
  return (
    <Button onClick={ this.handleClick }>
      { label }
    </Button>
  );
}

This style is widely used in the WordPress community, e.g., in projects like Calypso, Gutenberg or in Javascript code in WordPress core.

I don't really expect that this PR will ever be merged, as it's directly opposed to the Prettier design principle of adding as few options as possible. It's more likely that we'll be maintaining and updating this patch in a fork.

Its purpose is rather to provide the maintainers with info on how people use and abuse Prettier, and to ask for feedback:

  • is this the right way to implement the paren spacing option? Are there any potential improvements?
  • can anything be done in the official Prettier codebase to make this patch simpler and more maintainable? Like adding some abstractions etc.

How it works?
Very similar to the existing bracket-spacing option: when enabled, add a space (" ") at the right places, or turn a softline into line.

The patch doesn't support paren spacing in Flow and Typescript constructs, as it's not (yet) used in any our code and I'm not very familiar with their syntax. It should be easy to add at any time though.

There's only one part that's tricky, and that's expanding last function argument:

map( as, a =>
  b(a) // there's a `softline/line` at the end of last arg
); // no space before `)`

map( as, a => [
  a
] ); // no `line` at the end of last arg, space before `)`

The last arg's doc sometimes has a softline/line at the end, sometimes not. There's a space before the closing ), sometimes not. To handle all cases correctly, I had to add an addedLine option to the group with the formatted last arg, and then check for that option in printArgumentsList.

@lydell
Copy link
Member

lydell commented Jul 1, 2017

@jsnajdr https://github.com/arijs/prettier-miscellaneous might be interested in this PR :)

@azz
Copy link
Member

azz commented Jul 1, 2017

Agreed. This is not something that is particularly controversial and we don't want to be adding options to comply with particular style guides. We recommend that if you are using prettier you should disable most/all formatting related style rules.

You should definitely take this to prettier-miscellaneous though.

@azz azz closed this Jul 1, 2017
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jan 19, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants