alexanderweiss / nova-prettier

Prettier extension for Nova
MIT License
35 stars 6 forks source link

Add support for forks of prettier #68

Closed reykjalin closed 3 years ago

reykjalin commented 3 years ago

I like using Automattic/wp-prettier for my projects because of the additional formatting rules it provides, in particular adding spaces inside parentheses and around function arguments. It's a fork of prettier with some additional options available for use in the .prettierrc.

I also have to use wp-prettier at work, so the extension unfortunately doesn't work for my work projects.

The way the extension currently looks for a prettier installation doesn't work however, even though wp-prettier is installed in place of prettier via npm i -D 'prettier@npm:wp-prettier@latest'.

The output of npm ls prettier --parseable --long --depth 0 with the installation like that results in a path like this: <path_to_project>/node_modules/prettier:wp-prettier@2.2.1-beta-1. The wp-prettier part here fails the !name.startsWith('prettier') check here:

https://github.com/alexanderweiss/nova-prettier/blob/91588368b760ae301f028a1f3a8b588a41d313dc/src/Scripts/prettier-installation.js#L19

and thus the extension can't find the prettier installed in the project despite wp-prettier being installed in place:

$ ls node_modules/prettier/
bin-prettier.js  index.js      parser-angular.js  parser-flow.js     parser-html.js      parser-postcss.js     README.md
doc.js           LICENSE       parser-babel.js    parser-glimmer.js  parser-markdown.js  parser-typescript.js  standalone.js
esm              package.json  parser-espree.js   parser-graphql.js  parser-meriyah.js   parser-yaml.js        third-party.js
$ npx prettier -v
2.2.1-beta-1

Proposed solutions

Hard-coding an additional check for wp-prettier doesn't seem very extendable, but maybe making the check more flexible would be appropriate? Instead of checking for name.startsWith('prettier'), maybe instead check for if the path variable ends with prettier? Or even just a name.contains('prettier') although that check may be a bit too lax.

An alternative solution that sort of bypasses this problem entirely would be to add an option that allows you to specify a direct path to the prettier directory or even a script that will run prettier for you. That way you could configure the extension to use a global installation of prettier for example.

alexanderweiss commented 3 years ago

I prefer adding a configuration option at both the Nova and project levels. The reason for the strict name check is because e.g. prettier plugins would otherwise also trigger the check.

reykjalin commented 3 years ago

The reason for the strict name check is because e.g. prettier plugins would otherwise also trigger the check.

Ah yeah, that makes sense. In that case being able to specify a custom path is perfectly fine by me 🙂

alexanderweiss commented 3 years ago

The latest version (v2.3.0) now allows you to set the path to the module in the project settings (as well as a default in the extension settings). Let me know if it works well for you!

reykjalin commented 3 years ago

Works flawlessly, thank you! 🥇