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

Added suppressCompletionReranking configuration setting. #63

Merged
merged 11 commits into from
Nov 20, 2020
Merged

Added suppressCompletionReranking configuration setting. #63

merged 11 commits into from
Nov 20, 2020

Conversation

OlegOAndreev
Copy link
Contributor

@OlegOAndreev OlegOAndreev commented Aug 27, 2020

Added suppressCompletionReranking configuration setting to disable the forced clangd-based completion ranking added in https://github.com/OlegOAndreev/vscode-clangd/commit/5616b189db01094940a70db2e8314d9135ab9a10#diff-45327f86d4438556066de133327f4ca2

Motivation: for shorter completion lists (e.g. object methods after the dot/arrow) the forced round-trips to clangd add some perceptible UI lag (fixes #62, but may as well fix #37).

@sam-mccall
Copy link
Member

I'm really sorry we let this fly by. I think it makes sense and we should merge it, just want to tweak the wording a little to ensure it's clear.

package.json Outdated
@@ -119,6 +119,11 @@
},
"description": "Extra clang flags used to parse files when no compilation database is found."
},
"clangd.suppressCompletionReranking": {
Copy link
Member

Choose a reason for hiding this comment

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

I think the doulbe-negative feel (suppress + "reranking" implying undoing the old ranking) makes this a bit hard to understand. And the description only mentions one half of the tradeoff, so it's hard to make a choice.

What about clangd.serverCompletionRanking / "Always rank completion items on the server as you type. This produces more accurate results at the cost of higher latency than client-side filtering."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I did not like the original name as well =) Fixed.

Copy link
Member

@sam-mccall sam-mccall left a comment

Choose a reason for hiding this comment

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

Thank you!

@sam-mccall sam-mccall merged commit 45abce8 into clangd:master Nov 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants