Skip to content

fix: Improve parameter completion #12040

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

Merged
merged 2 commits into from
Apr 21, 2022

Conversation

yue4u
Copy link
Contributor

@yue4u yue4u commented Apr 20, 2022

fix #12016 and handles some extra cases.

Copy link
Contributor Author

@yue4u yue4u left a comment

Choose a reason for hiding this comment

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

Another possible approach is to calculate and remove common prefix (attrs) and suffix (type params) existing in current param from completion's newText, but seems more difficult to do?

Comment on lines +588 to +589
fn g(foo: (), mut ba$0: u32)
fn f(foo: (), mut bar: u32) {}
Copy link
Contributor Author

@yue4u yue4u Apr 20, 2022

Choose a reason for hiding this comment

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

Only when editor's insert mode set to replace? Is there a way to test edit results with lsp cap and editor settings? Not seems possible with CompletionConfig yet

Comment on lines +619 to +620
fn f(foo: (), #[baz = "qux"] mut bar: u32) {}
fn g(foo: (), mut ba$0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tests pass for both completion listing and edit, but doesn't work on my machine, not sure what's wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

This might be the same bug that I fixed in #11967 - basically, the completion's lookup_by must start with the characters in its range, otherwise the editor will filter out the completion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I understand correctly, 8f8f20f should fix this? At least it became working in my environment.

@jonas-schievink
Copy link
Contributor

Works great for me, thanks! @bors r+

@bors
Copy link
Contributor

bors commented Apr 21, 2022

📌 Commit 8f8f20f has been approved by jonas-schievink

@bors
Copy link
Contributor

bors commented Apr 21, 2022

⌛ Testing commit 8f8f20f with merge 24f9209...

@bors
Copy link
Contributor

bors commented Apr 21, 2022

☀️ Test successful - checks-actions
Approved by: jonas-schievink
Pushing 24f9209 to master...

@bors bors merged commit 24f9209 into rust-lang:master Apr 21, 2022
bors added a commit that referenced this pull request Apr 27, 2022
fix: fn_param completion lookup

close #12073 caused by #12040
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parameter completion can include incorrect ,
3 participants