-
Notifications
You must be signed in to change notification settings - Fork 410
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
Integrate the client's file formatting options with JDT code action handlers #1657
Integrate the client's file formatting options with JDT code action handlers #1657
Conversation
For e2e test, this PR requires redhat-developer/vscode-java#1081 to return the current file's formatting options. |
672164a
to
468a462
Compare
Signed-off-by: Sheng Chen <[email protected]>
…andlers Signed-off-by: Jinbo Wang <[email protected]>
468a462
to
72fca51
Compare
<repository location="https://download.eclipse.org/eclipse/updates/4.19-I-builds/I20210217-1800/"/> | ||
</location> | ||
<location includeAllPlatforms="false" includeConfigurePhase="false" includeMode="planner" includeSource="true" type="InstallableUnit"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4.19M3-I-build contains the dependent formatting changes we made in the upstream jdt.core. But the I-build doesn't contain xtend and xtext sdks, so still need fetch xtend and xtext from the old release. Once 2021-03 release is ready, we can merge these two locations back into one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@testforstephen There will be another PR to remove https://github.com/redhat-developer/vscode-java/blob/master/src/extension.ts#L676 right? |
yes, @snjeza has two PRs to handle that parts, see redhat-developer/vscode-java#1631 and #1551. Once this PR is merged, then we can merge the @snjeza's. |
If using a eclipse.jdt.ls version that includes eclipse-jdtls/eclipse.jdt.ls#1657 this will finally make code-actions respect the clients shiftwidth/expandtab options and no longer include tabs when the user uses spaces. The alternative to overriding the handler would have been to document that users should include these options in their `config.settings`, but given that there is no good reason that I can think of where you don't want to respond with your local editor settings it seems justified to include this out of the box instead of requiring every user to add this.
If using a eclipse.jdt.ls version that includes eclipse-jdtls/eclipse.jdt.ls#1657 this will finally make code-actions respect the clients shiftwidth/expandtab options and no longer include tabs when the user uses spaces. The alternative to overriding the handler would have been to document that users should include these options in their `config.settings`, but given that there is no good reason that I can think of where you don't want to respond with your local editor settings it seems justified to include this out of the box instead of requiring every user to add this.
If using a eclipse.jdt.ls version that includes eclipse-jdtls/eclipse.jdt.ls#1657 this will finally make code-actions respect the clients shiftwidth/expandtab options and no longer include tabs when the user uses spaces. The alternative to overriding the handler would have been to document that users should include these options in their `config.settings`, but given that there is no good reason that I can think of where you don't want to respond with your local editor settings it seems justified to include this out of the box instead of requiring every user to add this.
Signed-off-by: Sheng Chen [email protected]
[Jinbo Wang]: Adopt the new compilation unit API to consume the client's formatting options.
Fixes #1157
Signed-off-by: Jinbo Wang [email protected]
Based on #1194