Add prettier beautifier and bracket_spacing option #2047

stevenzeck · 2018-02-19T17:18:52Z

What does this implement/fix? Explain your changes.

Adds a new option, bracket_spacing, to print spaces between brackets in object literals for JavaScript
Adds Prettier as a supported beautifier for JavaScript, CSS, LESS, SCSS, Vue, JSON, and Markdown.
...

Does this close any currently open issues?

Closes #1497
Closes #802
...

Any other comments?

...

Checklist

Check all those that are applicable and complete.

Merged with latest master branch
Regenerate documentation with npm run docs
Add change details to CHANGELOG.md under "Next" section
Added examples for testing to examples/ directory
Travis CI passes (Mac support)
AppVeyor passes (Windows support)

Glavin001 · 2018-02-19T17:29:54Z

src/beautifiers/prettier.coffee

+    _:
+      tabWidth: "indent_size"
+      useTabs: "indent_with_tabs"
+    JavaScript:


TypeScript is also a Prettier supported language.

Glavin001 · 2018-02-19T17:33:25Z

src/beautifiers/prettier.coffee

+  options: {
+    _:
+      tabWidth: "indent_size"
+      useTabs: "indent_with_tabs"


I recommend something like https://github.com/Glavin001/atom-beautify/blob/master/src/beautifiers/prettydiff.coffee#L10-L13

useTabs: ["indent_with_tabs", "indent_char", (indent_with_tabs, indent_char) -> return (indent_with_tabs is true) or (indent_char is "\t") ]

which is the following in JavaScript:

useTabs: [ "indent_with_tabs", "indent_char", function(indent_with_tabs, indent_char) { return (indent_with_tabs === true) || (indent_char === "\t"); } ]

Glavin001 · 2018-02-19T17:36:59Z

src/languages/javascript.coffee

@@ -108,5 +108,9 @@ module.exports = {
      default: "System Default"
      enum: ["CRLF","LF","System Default"]
      description: "Override EOL from line-ending-selector"
+    bracket_spacing:


How about object_curly_spacing to align with Unibeautify and ESLint? See https://unibeautify.com/docs/option-object-curly-spacing.html

Or object_bracket_spacing?

Glavin001 · 2018-02-19T17:37:15Z

src/options.json

+        "description": "Insert spaces between brackets in object literals (Supported by Coffee Formatter)",
+        "title": "Bracket spacing",
+        "beautifiers": [
+          "Coffee Formatter"


This is strange. Should be Prettier.

That's under the CoffeeScript language, which falls back to JavaScript.

stevenzeck · 2018-02-19T19:33:42Z

@Glavin001 made the changes you requested. Let me know what you want to do about the option being included in all of the languages that have js as a fallback.

The Travis build that fails will be fixed once #2048 is merged, so I'm x'ing that one.

Glavin001 · 2018-02-20T14:26:53Z

@szeck87 : Merged #2048. Please update this PR and we can see Travis CI passing. Thanks!

stevenzeck · 2018-02-20T16:32:08Z

The build passed, but I just noticed AB is not reading from the configuration file for prettier. Let me fix that.

stevenzeck · 2018-02-20T20:15:17Z

@Glavin001 good to go.

Glavin001

The documentation shows beautifiers supporting the new object_curly_spacing option which I know do not.

Glavin001 · 2018-02-20T20:25:54Z

docs/options.md

+
+**Type**: `boolean`
+
+**Supported Beautifiers**:  [`TypeScript Formatter`](#typescript-formatter) 


I do not think this beautifier supports this option.

Glavin001 · 2018-02-20T20:26:04Z

docs/options.md

+
+**Type**: `boolean`
+
+**Supported Beautifiers**:  [`Vue Beautifier`](#vue-beautifier) 


I do not think this beautifier supports this option.

Glavin001 · 2018-02-20T20:26:27Z

docs/options.md

+
+**Type**: `boolean`
+
+**Supported Beautifiers**:  [`JS Beautify`](#js-beautify) 


I do not think this beautifier supports this option.

Glavin001 · 2018-02-20T20:26:35Z

docs/options.md

+
+**Type**: `boolean`
+
+**Supported Beautifiers**:  [`JS Beautify`](#js-beautify) 


I do not think this beautifier supports this option.

Glavin001 · 2018-02-20T20:41:33Z

@szeck87 : For example: https://github.com/Glavin001/atom-beautify/blob/master/src/beautifiers/typescript-formatter.coffee#L8-L9

This is false. TypeScript Formatter does NOT support all options therefore should not be true.

Glavin001

Looks like it would be better to revert the last commit about fixing the beautifier support. We can hold this off until a future PR then since it is more complicated than anticipated.

Glavin001 · 2018-02-20T20:58:07Z

src/beautifiers/coffee-formatter.coffee

@@ -7,7 +7,7 @@ module.exports = class CoffeeFormatter extends Beautifier
  link: "https://github.com/Glavin001/Coffee-Formatter"

  options: {
-    CoffeeScript: true
+    CoffeeScript: false


Glavin001 · 2018-02-20T20:59:40Z

src/beautifiers/typescript-formatter.coffee

@@ -5,7 +5,7 @@ module.exports = class TypeScriptFormatter extends Beautifier
  name: "TypeScript Formatter"
  link: "https://github.com/vvakame/typescript-formatter"
  options: {
-    TypeScript: true
+    TypeScript: false


Nope.

See rest of file:

if options.indent_with_tabs opts.convertTabsToSpaces = false else opts.tabSize = options.tab_width or options.indent_size opts.indentSize = options.indent_size

Firstly, this beautifier supports indent_with_tabs, tab_with, and indent_size.

Secondly, I expect TypeScript and TSX languages to have the same support, unless otherwise specified.

Glavin001 · 2018-02-20T21:00:05Z

src/beautifiers/vue-beautifier.coffee

@@ -6,7 +6,7 @@ module.exports = class VueBeautifier extends Beautifier
  link: "https://github.com/Glavin001/atom-beautify/blob/master/src/beautifiers/vue-beautifier.coffee"

  options:
-    Vue: true
+    Vue: false


Since this uses JS Beautify may as well leave as true.

Glavin001 · 2018-02-20T21:43:18Z

src/beautifiers/prettier.coffee

+      else
+        reject(new Error("Unknown language for Prettier"))
+
+      filePath = atom.workspace.getActiveTextEditor().getPath()


Use context.filePath instead.
See

atom-beautify/src/beautifiers/rustfmt.coffee

Lines 20 to 21 in 788d002

beautify: (text, language, options, context) ->

cwd = context.filePath and path.dirname context.filePath

for example.

What does Prettier resolveConfig do if filePath = undefined?

It returns null if it can't find anything.

Glavin001 · 2018-02-20T21:45:13Z

src/languages/tsx.coffee

@@ -2,7 +2,7 @@ module.exports = {

  name: "TSX"
  namespace: "tsx"
-  fallback: ['ts']
+  fallback: ['js']


What is this for?

The options defined here https://github.com/szeck87/atom-beautify/blob/add-prettier/src/beautifiers/typescript-formatter.coffee#L12-L15 aren't recognized if the fallback is to ts.

Glavin001 · 2018-02-20T21:45:30Z

src/languages/typescript.coffee

@@ -18,4 +18,6 @@ module.exports = {
    "ts"
  ]

+  defaultBeautifier: "TypeScript Formatter"


👍 This was the previous default beautifier.

Yep, the build failed originally because it used prettier for the tests.

kachkaev · 2018-02-21T20:52:57Z

Despite being an author of #2004 and #2008, I'd vote for making prettier a default beautifier for markdown and ignoring a switch to remark-cli! I've been using Prettier from a command line recently and quite like its design! If I'm not wrong, Prettier uses remark to parse markdown, so we'll still keep remark under the hood 😉

If there is a plan to make prettier a default markdown beautifier, feel free to close my two PRs mentioned above!

I can also return to #1990 once Prettier lands atom-beautify. Although prettier already formats some code blocks recursively, it'd be cool to additionally beautify a user-defined set of langauges with atom-beautify. So until prettier supports language XYZ as a plugin, people can enjoy nested code block formatting in their markdowns 👍

kachkaev · 2018-02-21T21:09:38Z

To be completely fair about prettier, there is one inconvenience in it that could make markdown beautification a bit frustrating for some users. It is to do with how numbered lists are indented, see prettier/prettier#3459 (comment)

There is a PR to solve this prettier/prettier#3990, but until it’s discussed and merged it’d be perhaps wise to keep Prettier non-default (at least in markdown).

Markdown support in prettier is still pretty new, but I like the pace at which they are improving stuff!

stevenzeck · 2018-02-22T17:57:41Z

@kachkaev prettier won't be the default for anything yet. I want to get it out there for people to start using and make sure it doesn't have any serious problems. But certainly open to it once it's been out there for a while.

Glavin001 · 2018-03-01T22:58:47Z

src/beautifiers/prettier.coffee

+      ]
+    JavaScript:
+      bracketSpacing: "object_curly_spacing"
+    TypeScript: false


Should this be true?

kachkaev · 2018-03-06T16:18:23Z

May be related: #2070

harmtemolder · 2019-09-18T05:31:55Z

I can't find any documentation on the bracket_spacing option implemented here. What are its possible values in .jsbeautifyrc?

stevenzeck added 3 commits February 19, 2018 00:09

Add Prettier as a beautifier, first commit

35f8c92

Fix up options for Prettier, add new bracket_spacing option

cc56ba3

Update changelog

0c6ae87

stevenzeck requested a review from Glavin001 February 19, 2018 17:18

Fix lint error

c661bff

Glavin001 requested changes Feb 19, 2018

View reviewed changes

Glavin001 assigned stevenzeck Feb 19, 2018

Glavin001 added add-beautifier add-beautifier-option high priority labels Feb 19, 2018

stevenzeck added 2 commits February 19, 2018 11:53

Make suggested updates

152d696

Rerun docs, make typescript-formatter the default for typescript

1de54ef

Fix options not working from Atom UI and config file

db80ba6

Glavin001 requested changes Feb 20, 2018

View reviewed changes

Fix options supported by beautifiers

7c2c0d3

Glavin001 requested changes Feb 20, 2018

View reviewed changes

stevenzeck added 2 commits February 20, 2018 15:19

Fix options for typescript-formatter and vue-beautifier

e97cdea

Update TSX options

b4c75b4

Glavin001 requested changes Feb 20, 2018

View reviewed changes

Use path and context to retrieve the directory

5017215

Glavin001 reviewed Mar 1, 2018

View reviewed changes

src/beautifiers/prettier.coffee

]

JavaScript:

bracketSpacing: "object_curly_spacing"

TypeScript: false

Copy link

Owner

Glavin001 Mar 1, 2018

Choose a reason for hiding this comment

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

Should this be true?

Glavin001 merged commit 26d8cf0 into Glavin001:master Mar 1, 2018

kachkaev mentioned this pull request Mar 2, 2018

Upgrade prettier to 1.11 #2060

Merged

6 tasks

stevenzeck deleted the add-prettier branch March 2, 2018 13:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add prettier beautifier and bracket_spacing option #2047

Add prettier beautifier and bracket_spacing option #2047

stevenzeck commented Feb 19, 2018 •

edited

Loading

Glavin001 Feb 19, 2018

Glavin001 Feb 19, 2018

Glavin001 Feb 19, 2018

Glavin001 Feb 19, 2018

stevenzeck Feb 19, 2018

stevenzeck commented Feb 19, 2018 •

edited

Loading

Glavin001 commented Feb 20, 2018

stevenzeck commented Feb 20, 2018

stevenzeck commented Feb 20, 2018

Glavin001 left a comment

Glavin001 Feb 20, 2018

Glavin001 Feb 20, 2018

Glavin001 Feb 20, 2018

Glavin001 Feb 20, 2018

Glavin001 commented Feb 20, 2018

Glavin001 left a comment

Glavin001 Feb 20, 2018

Glavin001 Feb 20, 2018

Glavin001 Feb 20, 2018

Glavin001 Feb 20, 2018

Glavin001 Feb 20, 2018

stevenzeck Feb 20, 2018

Glavin001 Feb 20, 2018

stevenzeck Feb 20, 2018

Glavin001 Feb 20, 2018

stevenzeck Feb 20, 2018

kachkaev commented Feb 21, 2018 •

edited

Loading

kachkaev commented Feb 21, 2018

stevenzeck commented Feb 22, 2018

Glavin001 Mar 1, 2018

kachkaev commented Mar 6, 2018

harmtemolder commented Sep 18, 2019


		Type: `boolean`

		Supported Beautifiers: [`TypeScript Formatter`](#typescript-formatter)


		Type: `boolean`

		Supported Beautifiers: [`Vue Beautifier`](#vue-beautifier)


		Type: `boolean`

		Supported Beautifiers: [`JS Beautify`](#js-beautify)

	beautify: (text, language, options, context) ->
	cwd = context.filePath and path.dirname context.filePath

Add prettier beautifier and bracket_spacing option #2047

Add prettier beautifier and bracket_spacing option #2047

Conversation

stevenzeck commented Feb 19, 2018 • edited Loading

What does this implement/fix? Explain your changes.

Does this close any currently open issues?

Any other comments?

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevenzeck commented Feb 19, 2018 • edited Loading

Glavin001 commented Feb 20, 2018

stevenzeck commented Feb 20, 2018

stevenzeck commented Feb 20, 2018

Glavin001 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Glavin001 commented Feb 20, 2018

Glavin001 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kachkaev commented Feb 21, 2018 • edited Loading

kachkaev commented Feb 21, 2018

stevenzeck commented Feb 22, 2018

Choose a reason for hiding this comment

kachkaev commented Mar 6, 2018

harmtemolder commented Sep 18, 2019

stevenzeck commented Feb 19, 2018 •

edited

Loading

stevenzeck commented Feb 19, 2018 •

edited

Loading

kachkaev commented Feb 21, 2018 •

edited

Loading