feat: Add doc-alias based completion #14433
Conversation
rustbot
added
the
S-waiting-on-review
|
btw. does CI not abort after a newer commit got pushed in? |
|
CI runs with |
|
It looks like the other |
crates/hir-def/src/attr.rs
Outdated
| pub fn doc_exprs(&self) -> Vec<DocExpr> { | ||
| self.by_key("doc").tt_values().map(DocExpr::parse).collect() | ||
| } | ||
|
|
||
| pub fn doc_aliases(&self) -> Vec<SmolStr> { | ||
| self.doc_exprs().into_iter().flat_map(|doc_expr| doc_expr.aliases()).collect() | ||
| } |
There was a problem hiding this comment.
| pub fn doc_exprs(&self) -> Vec<DocExpr> { | |
| self.by_key("doc").tt_values().map(DocExpr::parse).collect() | |
| } | |
| pub fn doc_aliases(&self) -> Vec<SmolStr> { | |
| self.doc_exprs().into_iter().flat_map(|doc_expr| doc_expr.aliases()).collect() | |
| } | |
| pub fn doc_exprs(&self) -> impl Iterator<Item = DocExpr> { | |
| self.by_key("doc").tt_values().map(DocExpr::parse) | |
| } | |
| pub fn doc_aliases(&self) -> impl Iterator<Item = SmolStr>{ | |
| self.doc_exprs().flat_map(|doc_expr| doc_expr.aliases()) | |
| } |
There was a problem hiding this comment.
hmm, since aliases now returns &[SmolStr], how do I make this compile?
There was a problem hiding this comment.
impl Iterator<Item = &SmolStr> should work
There was a problem hiding this comment.
nope, aliases().to_vec() works but copies
crates/ide-completion/src/item.rs
Outdated
| } else if let Some(doc_aliases) = self.doc_aliases { | ||
| label = SmolStr::from(format!("{label} (alias {doc_aliases})")); |
There was a problem hiding this comment.
Here is something we should do differently. I think we should split the completions according to their aliases up, that is for each alias we copy the item but change the label and lookup to thing (alias <the alias>) + the final one without its alias. This should be done in to_proto.rs similar to how we handle the CompletionItem::ref_match field.
There was a problem hiding this comment.
I'll comprehend this later today, but I played around some more with the code and setting lookup here works better.
If you mean there should be one CompletionItem per alias, that sounds worse than just having e.g. Label (alias A, B, C).
There was a problem hiding this comment.
Well if setting lookup with multiple aliases somehow works out then sure, but I feel like it won't. Do try it out thought since it would simplify things 👍
|
We can further discuss about the new changes. I can get the iterators further outside, right? tbh I should study iterators some more |
|
@bors r+ I fear this might not work in at least VSCode yet because of one quirk it has in regards to the text edit having to start with what source_range points to, but we'll see whether that is the case or not. If we do run into this we'll have to make use of additional text edits somehow. |
Veykril
changed the title
|
☀️ Test successful - checks-actions |
doc(alias)-based completion round 2 Follow-up on #14433 We can now complete fields, functions and some use/mods. Flyimports don't behave, I don't really have the time to understand the structure there either. While reading the flyimport code, I removed one method only used there, the closure-tree was a bit confusing, I can revert that if you want.
Closes #14406.
I adapted the parsing code from the CfgExpr parsing code, maybe there's a better abstraction for both, or attribute parsing in general. It also includes
doc(hidden)-parsing, which means it could replace the other function.There are a few tests for parsing.
process_all_nameschanged the most, I added some docs there to explain what happens.Many call sites just pass an empy vec to
add_path_resolution'sdoc_aliases, since either it doesn't make sense to pass anything (e.g. visibility completion) or I don't know where to get them from. Shouldn't really matter, as it will just not show aliases if the vec is empty and we can extend alias completion in these cases later.I added two tests in
special.rsfor struct name completion (which was the main thing I wanted). I also tried function and field names, but these don't work yet. I want to add those in a follow-up PR.