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

Only append data on completion item selected #2639

Merged
merged 1 commit into from May 8, 2023

Conversation

jdneo
Copy link
Contributor

@jdneo jdneo commented May 6, 2023

  • Instead of adding data to the completion items directly, now we only append those data when 'onDidSelect' happens. Because those data are only used by the registered ranking providers. Both server and client do not care about it actually.

- Instead of adding data to the completion items directly, now we only
  append those data when 'onDidSelect' happens. Because those data are
  only used by the registered ranking providers. Both server and client
  do not care about it actually.

Signed-off-by: Sheng Chen <sheche@microsoft.com>
@jdneo
Copy link
Contributor Author

jdneo commented May 6, 2023

Make sure the IntelliCode is installed and then triggering completion at | in the following snippet:

public static void main(String[] args) {
    args.|
}

The response size reduced from 22.2KB to 15.8KB.

If completion after a string: args[0].|

Then response size reduced from 146KB to 83.7KB

@jdneo jdneo marked this pull request as ready for review May 8, 2023 01:41
@rgrunber rgrunber added this to the End May 2023 milestone May 8, 2023
@rgrunber
Copy link
Contributor

rgrunber commented May 8, 2023

Ok, so based on https://github.com/eclipse/eclipse.jdt.ls/pull/2639/files#diff-335831335331faa697d0e108634d599cf7cddd65328987dd8047572fd233b55dR96-R100 the data field was never meant for use by the client. If that's the case, then this seems fine to me.

@rgrunber rgrunber merged commit 2b32352 into eclipse-jdtls:master May 8, 2023
6 checks passed
@jdneo jdneo deleted the cs/ranking-data branch May 9, 2023 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants