Skip to content

Conversation

@feniljain
Copy link
Contributor

@feniljain feniljain commented Jun 2, 2022

@feniljain feniljain marked this pull request as draft June 2, 2022 14:56
@feniljain feniljain marked this pull request as ready for review June 2, 2022 15:31
@feniljain feniljain changed the title fix(completion): recursive super:: completion in crate root fix(completion): recursive super:: completion in crate root Jun 2, 2022
Copy link
Member

@Veykril Veykril left a comment

Choose a reason for hiding this comment

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

This doesn't only apply use paths, but to all paths, so we should add the is_crate_root check for add_nameref_keywords_with_colon to all of them, or rather, we should make is_crate_root a propery of CompletionContext and then just check for that inside add_nameref_keywords_with_colon without passing an extra bool

@feniljain feniljain force-pushed the fix_completions branch 2 times, most recently from 817240a to b2c9ecf Compare June 3, 2022 15:21
@bors
Copy link
Contributor

bors commented Jun 3, 2022

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

@feniljain feniljain requested a review from Veykril June 5, 2022 15:34
@bors
Copy link
Contributor

bors commented Jun 17, 2022

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

@Veykril
Copy link
Member

Veykril commented Jun 17, 2022

I've done some major refactorings to the ide-completion crate, so you might wanna manually port your changes over again instead of rebasing, sorry 😅

@Veykril Veykril self-assigned this Jun 21, 2022
@feniljain
Copy link
Contributor Author

feniljain commented Jul 10, 2022

Hey @Veykril , sorry for the delay. I have been stuck with something for a while.

Thanks for all the reviews. I have applied all of them and as mentioned I have manually ported all the changes and opened a new PR here: #12735

We should be able to close this PR in favour of the latest one

@Veykril Veykril closed this Jul 13, 2022
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.

use erroneously suggests super:: as a completion option even when it's not valid.

3 participants