pub(crate) completion leaves closing ) behind

[jonas-schievink]

impl () {
    pub(crate) fn f() {}

    pu$0
}

Triggering completion here suggests pub(crate). Then, after typing b(c, accepting the pub(crate) completion produces this:

impl () {
    pub(crate) fn f() {}

    pub(crate)$0)
}

1. That's wrong :)
2. The pub(crate) completion does not appear in pub($0), which kinda makes sense (since crate by itself appears there), but I also feel like we should be consistent there, since you can "carry over" a completion when you request it after pu$0.

[Veykril]

To me it feels like we shouldn't carry this completion over at all tbh, though I don't think we are in charge of that as the server.

[yue4u]

I did some debugging but didn't understand why it renders correctly with pub() but not pub(c), was thinking the cause of this issue is a extra ) been produced by auto closing after typing (,

I think It's possible to prevent carrying by removing parentheses from lookup and set it to something like pubcrate. As a drawback, this approach won't show any completions after typing ( after pub$0 until typing out next c or s.

[Veykril]

As a drawback, this approach won't show any completions after typing ( after pub$0 until typing out next c or s.

This is most likely because ( isn't set as a trigger character by the server capabilities, so the client doesn't request completions for it.

[Veykril]

This list here

rust-analyzer/crates/rust-analyzer/src/caps.rs

Line 31 in 58234c6

trigger_characters: Some(vec![":".to_string(), ".".to_string(), "'".to_string()]),

, though adding ( to the list might create a lot of completion noise

[yue4u]

Adding ( seems good to me in this situation but it will sure change the overall completion behavior a lot. (It doesn't make any test fail though 😆 ).

if-2022-05-29_18.53.32.mp4

[yue4u]

We don't need to change the lookup at all if add ( to trigger_characters since it prevents carrying at the first place.

[yue4u]

Adding ( is really annoying when you only want to return () or use any tuple types.

[Veykril]

Ye that's what I worry about, in most cases you don't want that behaviour, but in the pub$0 case this makes a lot of sense, since you can't have pub(). I think adding this is fine if we make it only work in this occurence.