Skip to content

chore(playground): add new options#3350

Merged
azz merged 1 commit intoprettier:masterfrom
ikatyang:chore/playground-new-options
Dec 5, 2017
Merged

chore(playground): add new options#3350
azz merged 1 commit intoprettier:masterfrom
ikatyang:chore/playground-new-options

Conversation

@ikatyang
Copy link
Copy Markdown
Member

@ikatyang ikatyang commented Nov 29, 2017 •
edited by azz
Loading

suchipi
suchipi approved these changes Nov 29, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@suchipi suchipi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be cool if someday the HTML was generated from cli-constant

Comment thread src/cli-constant.js
category: CATEGORY_FORMAT,
forwardToApi: true,
description: "Include parentheses around a sole arrow function parameter.",
default: "avoid",
Copy link
Copy Markdown
Member

@suchipi suchipi Nov 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you going to fill this back in once we've decided on the new default?

Copy link
Copy Markdown
Member Author

@ikatyang ikatyang Nov 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, forgot to mention why I removed this line: we use the default value from options.js for all format options.

prettier/src/cli-util.js

Lines 574 to 576 in fbbfa52

if (optionCamelName in apiDefaultOptions) {
return apiDefaultOptions[optionCamelName];
}

Copy link
Copy Markdown
Member

@suchipi suchipi Nov 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that's feels a bit unintuitive, but changing it is out of the scope of this PR

@azz
Copy link
Copy Markdown
Member

azz commented Dec 1, 2017

Currently the playground uses the released version of Prettier (from package.json) so this won't actually work if we merge it.

@duailibe duailibe added this to the 1.9 milestone Dec 1, 2017
@duailibe
Copy link
Copy Markdown
Collaborator

duailibe commented Dec 1, 2017 •
edited
Loading

I added the 1.9 milestone just to help us remember it's related (and merge after release) but feel free to remove if you think someone might think this is a blocker.

azz
azz requested changes Dec 1, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@azz azz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(so we don't accidentally merge)

@suchipi
Copy link
Copy Markdown
Member

suchipi commented Dec 2, 2017

So to clarify, this should be merged post release?

@azz
Copy link
Copy Markdown
Member

azz commented Dec 2, 2017

Yes. After prettier's own dependency on prettier is updated to 1.9.0

@azz azz merged commit f3e7ca2 into prettier:master Dec 5, 2017
@ikatyang ikatyang deleted the chore/playground-new-options branch December 5, 2017 10:28
@ikatyang ikatyang mentioned this pull request Dec 7, 2017
Merged
@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.

Reviewers

@suchipi suchipi suchipi approved these changes

@azz azz azz approved these changes

Assignees

No one assigned

Labels

locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.

Projects

None yet

Milestone

1.9

Development

Successfully merging this pull request may close these issues.

4 participants

@ikatyang @azz @duailibe @suchipi