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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dprint fixer #4024

Merged
merged 3 commits into from
Jan 4, 2022
Merged

Add dprint fixer #4024

merged 3 commits into from
Jan 4, 2022

Conversation

myobie
Copy link
Contributor

@myobie myobie commented Dec 26, 2021

This is attempt number 3. See https://github.com/dense-analysis/ale/pulls?q=is%3Apr+is%3Aclosed+dprint

I have changed to use stdin. This is working well for me so far 馃挭

@myobie myobie force-pushed the dprint-fixer branch 7 times, most recently from c208aa3 to 728bc6c Compare December 28, 2021 13:04
@myobie
Copy link
Contributor Author

myobie commented Dec 28, 2021

@hsanson sorry for noise in this PR (I had to get docker installed locally so I could run the tests). It's now ready to go. The problem I couldn't figure out was that, of course, dprint is not installed inside the test docker container. I added some extra tests and I feel good about how they look. Thanks for your patience.

Copy link
Contributor

@hsanson hsanson left a comment

Choose a reason for hiding this comment

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

Looking good, thanks for the work. Would be great if you can add also help documentation to the PR:

  • Add ale-dprint-options section into ale.txt explaining the possible configuration variables.
  • Add ale-javascript-dprint section to ale-javascript.txt which points to ale-dprint-options. See ale-javascript-deno and ale-javascript-cspell for example.
  • Repeat for every language supported by dprint (e.g. typescript, json, and markdown).

@myobie myobie force-pushed the dprint-fixer branch 2 times, most recently from 7cdaef3 to 7f99416 Compare January 4, 2022 10:45
@myobie myobie requested a review from hsanson January 4, 2022 10:52
@myobie
Copy link
Contributor Author

myobie commented Jan 4, 2022

Thank you for your help getting this PR into shape. I've added all the languages which dprint supports with their wasm plugins. There are some process plugins, but they have a warning on their website and I think they are new. I'll revisit the ale docs in the future once I see how those non-wasm plugins turn out.

Also related: dprint/plugins#6

doc/ale.txt Outdated Show resolved Hide resolved
Copy link
Contributor

@hsanson hsanson left a comment

Choose a reason for hiding this comment

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

Thanks for the extensive documentation. Just one small detail and this should be ready to merge.

Copy link
Contributor

@hsanson hsanson left a comment

Choose a reason for hiding this comment

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

Thanks, good contribution and well documented.

@hsanson hsanson merged commit ac0495d into dense-analysis:master Jan 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants