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

Adopt Prettier for JavaScript code formatting #1166

Open
simonw opened this issue Dec 31, 2020 · 10 comments
Open

Adopt Prettier for JavaScript code formatting #1166

simonw opened this issue Dec 31, 2020 · 10 comments

Comments

@simonw
Copy link
Owner

simonw commented Dec 31, 2020

https://prettier.io/ - I'm going to go with 2 spaces.

simonw added a commit that referenced this issue Dec 31, 2020
simonw added a commit that referenced this issue Dec 31, 2020
@simonw
Copy link
Owner Author

simonw commented Dec 31, 2020

I want a CI check that confirms that files conform to prettier - but only datasette/static/*.js files that are not already minified.

This seems to do the job:

npx prettier --check 'datasette/static/*[!.min].js'

@simonw
Copy link
Owner Author

simonw commented Dec 31, 2020

This action looks good - tag 3.2 is equivalent to this commit hash: https://github.com/creyD/prettier_action/tree/bb361e2979cff283ca7684908deac8f95400e779

@simonw
Copy link
Owner Author

simonw commented Dec 31, 2020

Maybe not that action actually - I wanted to use a pre-built action to avoid installing Prettier every time, but that's what it seems to do: https://github.com/creyD/prettier_action/blob/bb361e2979cff283ca7684908deac8f95400e779/entrypoint.sh#L28-L37

@simonw
Copy link
Owner Author

simonw commented Dec 31, 2020

I think this should work:

- uses: actions/cache@v2
  with:
    path: ~/.npm
    key: ${{ runner.os }}-node-${{ hashFiles('**/prettier.yml' }}

I'll use the prettier.yml workflow that I'm about to create as the cache key for the NPM cache.

@simonw
Copy link
Owner Author

simonw commented Dec 31, 2020

Tests passed in https://github.com/simonw/datasette/runs/1631677726?check_suite_focus=true

I'm going to try submitting a pull request with badly formatted JavaScript to see if it gets caught.

simonw added a commit that referenced this issue Dec 31, 2020
@simonw
Copy link
Owner Author

simonw commented Dec 31, 2020

Oops, committed that bad formatting test to main instead of a branch!

@simonw
Copy link
Owner Author

simonw commented Dec 31, 2020

@simonw simonw closed this as completed in a93a65b Dec 31, 2020
@simonw
Copy link
Owner Author

simonw commented Dec 31, 2020

@simonw
Copy link
Owner Author

simonw commented Dec 31, 2020

I should configure the action to only run if changes have been made within the datasette/static directory.

@simonw simonw reopened this Dec 31, 2020
simonw added a commit that referenced this issue Jan 1, 2021
@simonw simonw closed this as completed Jan 1, 2021
simonw added a commit that referenced this issue Jan 19, 2021
@simonw simonw added this to the Datasette 0.54 milestone Jan 24, 2021
This was referenced Jan 25, 2021
simonw added a commit that referenced this issue Jan 25, 2021
@thorn0
Copy link

thorn0 commented Feb 22, 2021

Hi! I don't think Prettier supports this syntax for globs: datasette/static/*[!.min].js Are you sure that works?
Prettier uses https://github.com/mrmlnc/fast-glob, which in turn uses https://github.com/micromatch/micromatch, and the docs for these packages don't mention this syntax. As per the docs, square brackets should work as in regexes (foo-[1-5].js).

Tested it. Apparently, it works as a negated character class in regexes (like [^.min]). I wonder where this syntax comes from. Micromatch doesn't support that:

micromatch(['static/table.js', 'static/n.js'], ['static/*[!.min].js']);
// result: ["static/n.js"] -- brackets are treated like [!.min] in regexes, without negation

@simonw simonw reopened this Feb 22, 2021
@simonw simonw removed this from the Datasette 0.54 milestone Jan 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants