FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add support for LSP formatting `options` parameter by masad-frost · Pull Request #134 · python-lsp/python-lsp-server · GitHub

/ python-lsp-server Public

Add support for LSP formatting options parameter - #134

Merged
ccordoba12 merged 11 commits into
python-lsp:developfrom
masad-frost:fm-yapf-formatting-options
Apr 5, 2022
ConversationCommits (11)ChecksFiles changed
Merged

Add support for LSP formatting options parameter#134
ccordoba12 merged 11 commits into
python-lsp:developfrom
masad-frost:fm-yapf-formatting-options

Conversation

masad-frost commented Dec 9, 2021

Copy link
Copy Markdown
Contributor

This PR adds support for LSP formatting options https://microsoft.github.io/language-server-protocol/specifications/specification-3-17/#formattingOptions for the yapf

It adds explicit support for tabSize and insertSpaces, the rest of the options are not configurable in yapf. I did however add support for arbitrary configurations as specified in [key: string]: boolean | integer | string;.

I think this PR might break plugins as the API now passes options to formatter hooks, but not sure.

Nits are welcome as I don't normally read/write python.

masad-frost mentioned this pull request Dec 9, 2021
Closed
masad-frost closed this Dec 9, 2021
masad-frost reopened this Dec 9, 2021
masad-frost mentioned this pull request Dec 9, 2021
Closed
ccordoba12 reviewed Dec 9, 2021
View reviewed changes

ccordoba12 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hey @masad-frost, thanks for your contribution! Please read my comment below to understand how to reorient your work to fit the design of the other plugins in this project.

Comment thread pylsp/hookspecs.py

krassowski commented Dec 11, 2021

Copy link
Copy Markdown
Member

The tests are failing on CI:

>           style_config = style.CreateStyleFromConfig(style_config)
E           NameError: name 'style' is not defined

masad-frost force-pushed the fm-yapf-formatting-options branch from 8083a16 to e0872b5 Compare March 10, 2022 03:21
masad-frost force-pushed the fm-yapf-formatting-options branch from 5c7fd79 to 8aa9ce8 Compare March 17, 2022 20:47

masad-frost commented Mar 17, 2022

Copy link
Copy Markdown
Contributor Author

Lint caught an issue where I wasn't passing options for range formatting!

ccordoba12 commented Mar 30, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Hey @masad-frost, I fixed the linting errors reported here in another PR of mine.

Please let me know if you need help to fix the merge conflicts so we can merge this one.

masad-frost commented Mar 30, 2022

Copy link
Copy Markdown
Contributor Author

Done

masad-frost mentioned this pull request Mar 30, 2022
Merged

masad-frost commented Apr 3, 2022

Copy link
Copy Markdown
Contributor Author

Hmm, not sure why lint passed locally. Hopefully this is the last of it

ccordoba12 reviewed Apr 3, 2022
View reviewed changes

ccordoba12 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hey @masad-frost, last review then this should be ready.

I've also seen that you have struggled a lot with our linting tests, so I'll give you a hand with them if you're unable to fix them after this review.

Comment thread pylsp/plugins/autopep8_format.py Outdated
Comment thread pylsp/plugins/autopep8_format.py Outdated
Comment thread test/plugins/test_yapf_format.py Outdated
ccordoba12 added this to the v1.5.0 milestone Apr 4, 2022
ccordoba12 approved these changes Apr 5, 2022
View reviewed changes

ccordoba12 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks for your contribution @masad-frost!

ccordoba12 changed the title Add support for LSP formatting options parameter Add support for LSP formatting options parameter Apr 5, 2022
ccordoba12 merged commit 4bffc39 into python-lsp:develop Apr 5, 2022
masad-frost deleted the fm-yapf-formatting-options branch May 11, 2022 17:06

masad-frost commented May 11, 2022

Copy link
Copy Markdown
Contributor Author

Hey @ccordoba12, when do you plan on tagging a new release? Wondering because we wanna upgrade, also maybe i can slip #136 before then.

ccordoba12 commented May 11, 2022

Copy link
Copy Markdown
Member

Perhaps in a month or so.

masad-frost commented Jul 1, 2022

Copy link
Copy Markdown
Contributor Author

Good time to bump to 1.5?

ccordoba12 commented Jul 1, 2022

Copy link
Copy Markdown
Member

Yep, I'll release it next week.

This was referenced Sep 1, 2022
Closed
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

krassowski krassowski left review comments
ccordoba12 ccordoba12 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v1.5.0

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL