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

feat: Allow excluding specific traits from completion #18179

Merged
merged 6 commits into from
Jan 1, 2025

Conversation

ChayimFriedman2
Copy link
Contributor

@ChayimFriedman2 ChayimFriedman2 commented Sep 24, 2024

It should be pretty easy to extend support to excluding only specific methods, but I didn't do that currently.

Traits configured to be excluded are resolved in each completion request from scratch. If this proves too expensive, it is easy enough to cache them in the DB.

Leave #17477 open, as per @dtolnay request.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 24, 2024
@ChayimFriedman2 ChayimFriedman2 force-pushed the omit-trait-completion branch 2 times, most recently from 61667ba to 1878d26 Compare September 24, 2024 18:13
crates/hir/src/lib.rs Outdated Show resolved Hide resolved
crates/rust-analyzer/src/config.rs Outdated Show resolved Hide resolved
crates/rust-analyzer/src/config.rs Outdated Show resolved Hide resolved
crates/rust-analyzer/src/config.rs Outdated Show resolved Hide resolved
crates/ide-completion/src/context.rs Outdated Show resolved Hide resolved
crates/ide-completion/src/completions/expr.rs Show resolved Hide resolved
@Veykril Veykril added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 25, 2024
crates/hir/src/lib.rs Outdated Show resolved Hide resolved
crates/ide-completion/src/context.rs Outdated Show resolved Hide resolved
crates/rust-analyzer/src/config.rs Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented Sep 30, 2024

☔ The latest upstream changes (presumably #18167) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Oct 14, 2024

☔ The latest upstream changes (presumably #18291) made this pull request unmergeable. Please resolve the merge conflicts.

@Veykril
Copy link
Member

Veykril commented Dec 24, 2024

So, I am finally coming back to this (apologies, I forgot about it!) and have since seen the PR to IJ that added similar things intellij-rust/intellij-rust#9123.

They have apparently two types of exclusion, they allow excluding completions of only the traits methods, or the methods as well as the trait itself. I think we wanna stick with what we have here for now, but make the config more general. That is instead of completion_autoimport_excludeTraits just completion_autoimport_exclude and then have that take a map instead of a list where the value is the kind of exclusion (which only allows one kind for now). That allows us to re-use this for more things. Similar treatment for completion_excludeTraits -> completion_exclude. Thoughts on that?

@Veykril
Copy link
Member

Veykril commented Dec 31, 2024

Relevant issue #13786 (comment)

To be accurate, only their methods are excluded, the trait themselves are still available.

I also excluded a bunch of std traits by default. Some less opinionated, like `AsRef`, which should never be used directly except in generic scenarios (and won't be excluded there), some more opinionated, like the ops traits, which I know some users sometimes want to use directly. Either way it's configurable.

It should be pretty easy to extend support to excluding only specific methods, but I didn't do that currently.

Traits configured to be excluded are resolved in each completion request from scratch. If this proves too expensive, it is easy enough to cache them in the DB.
… the name

I.e. with `fn foo()`, don't complete at `x.fo|`, but complete (with imports) for `x.foo|`, since this is less likely to have false positives.

I opted to only do that for flyimport, even though for basic imports there can also be snippet completion (completing the params list for a method), since this is less universally applicable and seems not so useful.
@Veykril Veykril force-pushed the omit-trait-completion branch from 1d9543f to a02a1af Compare January 1, 2025 12:57
@Veykril Veykril force-pushed the omit-trait-completion branch from f804cd0 to fc4940f Compare January 1, 2025 14:06
@Veykril Veykril force-pushed the omit-trait-completion branch from fc4940f to 1adc805 Compare January 1, 2025 14:22
@Veykril Veykril enabled auto-merge January 1, 2025 14:22
@Veykril Veykril added this pull request to the merge queue Jan 1, 2025
Merged via the queue into rust-lang:master with commit 7e639ee Jan 1, 2025
9 checks passed
@ChayimFriedman2 ChayimFriedman2 deleted the omit-trait-completion branch January 1, 2025 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants