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

Respect formatting options #119

Merged
merged 7 commits into from
Jul 28, 2023

Conversation

6cdh
Copy link
Contributor

@6cdh 6cdh commented Jul 25, 2023

I also rewrote some formatting helper functions. It should fix #112

Comment on lines +95 to +100
(tabSize
insertSpaces
trimTrailingWhitespace
insertFinalNewline
trimFinalNewlines
key))
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure, but do we always use this style for structure fields? Or JSON requires that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's intended to be same as the names in LSP specification.

Maybe we can have a macro to automatically converts these names to racket name style.
Then we only use the LSP style names in jsexpr->FormattingOptions, and racket style names everywhere else.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be great if that is possible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about FormattingOptions and other top level structs? Their name are also inconsistent with Racket style.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same, but I think maybe let's have another PR for these changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok. I'm going to rename these fields.

@dannypsnl
Copy link
Contributor

Oh, and as a personal opinion, maybe all fo can be just replaced by opts in the program. Since they are obviously in context.

@6cdh
Copy link
Contributor Author

6cdh commented Jul 26, 2023

Oh, and as a personal opinion, maybe all fo can be just replaced by opts in the program. Since they are obviously in context.

ok

@jeapostrophe jeapostrophe merged commit 1325a43 into jeapostrophe:master Jul 28, 2023
@6cdh
Copy link
Contributor Author

6cdh commented Jul 29, 2023

The automatic renaming is in progress though. There are some difficulties. It could be in another PR.

@6cdh 6cdh deleted the respect-formatting-options branch September 6, 2023 02:44
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.

support formatting options
3 participants