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

Formatting Smithy document #89

Merged
merged 4 commits into from Mar 10, 2023

Conversation

daddykotex
Copy link
Contributor

Recently, we made great progress on our Smithy parser. We were able to build a formatter to go along with it. This PR adds formatting capability to the language server implementation. Formatting happens when the client requests it. For example in VS Code:

  1. explicitly ask to format a file
  2. configure format on save - editor calls format before calling save

Here is a small demo:

smithy-formatter-vscode.mov

@daddykotex daddykotex requested a review from a team as a code owner March 1, 2023 18:37
@srchase
Copy link
Contributor

srchase commented Mar 9, 2023

I got a chance to wire this into the Smithy for VS Code extension, and it's great!

Is it possible to tweak to the underlying formatter?

I noticed that trailing commas are added when 3 or more mixins are split into separate lines:

This:

structure UsesMixin with [MyMixin, MixinTwo, MixinThree] {
    foo: String
}

Gets reformatted to this:

structure UsesMixin with [
    MyMixin,
    MixinTwo,
    MixinThree
] {
    foo: String
}

Could those trailing commas get dropped? For comparison, when errors on an operation are broken into separate lines, trailing commas are not added there.

@daddykotex
Copy link
Contributor Author

Is it possible to tweak to the underlying formatter?
Definitely, right now this means, changing the formatter implementation, then updating the version here.
I'll do it.

If this gets merged, I'd like to take some time to work on a configuration that could be used to tweak the formatter version, and, maybe, specify the version so we don't have to update the language server everytime the formatter gets an update.

@srchase
Copy link
Contributor

srchase commented Mar 10, 2023

If this gets merged, I'd like to take some time to work on a configuration that could be used to tweak the formatter version, and, maybe, specify the version so we don't have to update the language server everytime the formatter gets an update.

Making this configurable makes sense. I'll merge this now, and will bump the formatter after disneystreaming/smithy-translate#102 is merged before we cut a release of the language server.

@srchase srchase merged commit 24c1442 into smithy-lang:main Mar 10, 2023
@daddykotex
Copy link
Contributor Author

Thanks, @srchase this is amazing

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