Use yaml package for formatting#983
Use yaml package for formatting#983remcohaszing wants to merge 2 commits intoredhat-developer:mainfrom
Conversation
This replaces Prettier as the LSP formatting solution with the `yaml` package. The `yaml` package can format YAML just fine, and we already have it as a dependency. Prettier is a big dependency. If people want to use Prettier, other Prettier integrations are probably better suited for them. For example, the YAML language server doesn’t respect Prettier configuration files. This is a proof of concept. I tried to map existing options to the new implementation. A more ideal solution might be to change the formatting options. Refs redhat-developer#933
| const formatted = prettier.format(text, prettierOptions); | ||
| const formatted = doc.toString(toStringOptions); | ||
|
|
||
| if (formatted === text) { |
There was a problem hiding this comment.
Can we get this broken out as a separate PR? My LSP client gets confused when the original text gets sent back in response to a formatting request.
|
I took another look at this; this seems like a feasible alternative to prettier. The main drawback of this approach is that we still don't have direct control of the formatting logic. There's quite a few bugs about weird formatting behaviour and feature requests for new formatting settings. I guess we could always submit PRs upstream to eemeli/yaml, and I'm guessing the maintainer of the parser is more receptive to adding new formatter options than prettier. There's one test failure after the rebase: "Formatting handles trailing commas (enabled)". Prettier added this option so I added it to the extension a while back, but eemeli/yaml doesn't support this yet. In order to preserve this functionality, we'd need to file a bug upstream, and probably submit the patch to implement it. |
| if (shouldFormat) { | ||
| this.formatterEnabled = shouldFormat.format; | ||
| this.yamlVersion = shouldFormat.yamlVersion; | ||
| this.customTags = shouldFormat.customTags; |
There was a problem hiding this comment.
these customTags aren't used anywhere. There's a helper method getCustomTags that converts them from the string format to the format that parseDocument is expecting.
4 participants
What does this PR do?
This replaces Prettier as the LSP formatting solution with the
yamlpackage. Theyamlpackage can format YAML just fine, and we already have it as a dependency. Prettier is a big dependency.If people want to use Prettier, other Prettier integrations are probably better suited for them. For example, the YAML language server doesn’t respect Prettier configuration files.
This is a proof of concept. I tried to map existing options to the new implementation. A more ideal solution might be to change the formatting options.
What issues does this PR fix or reference?
Refs #933
Is it tested? How?
npm testI discovered eemeli/yaml#562 while working on this.