Fix LSP not giving completion when an identifier is prefixed by a delimiting character #1043
Conversation
lsp/nls/src/requests/completion.rs
Outdated
| let path = path | ||
| .split('.') | ||
| .map(|str| { | ||
| let (str, is_unicode) = remove_quotes(str); |
There was a problem hiding this comment.
the is_unicode name is a bit unsettling here. The documentation of remove_quotes says that the return value indicate if one of the fields was quoted, isn't it? What's the relation to unicode?
There was a problem hiding this comment.
I guess most static fields are quoted because they probably have a character that isn't allowed in a Nickel identifier (e.g unicode characters). But you're right, a it might not be a unicode string. I've made changes, thank you!
lsp/nls/src/requests/completion.rs
Outdated
| static ref RE: regex::Regex = regex::Regex::new(r"[_a-zA-Z0-9-']*[a-zA-Z]_?").unwrap(); | ||
| // unwraps are safe here because we know these are correct regexes. | ||
| // This regexp must be the same as the regex for identifiers in the lexer (nickel_lang::parser::lexer) | ||
| static ref RE: regex::Regex = regex::Regex::new(r"_?[a-zA-Z][_a-zA-Z0-9-']*").unwrap(); |
There was a problem hiding this comment.
In retrospective, RE is a pretty bad name. I guess it's the regular expression for an identifier? RE_IDENTIFIER might be more explicit.
| @@ -287,16 +298,21 @@ fn get_identifier_path(text: &str) -> Option<Vec<String>> { | |||
|
|
|||
| let result: Vec<_> = RE_SPACE.split(text.as_ref()).map(String::from).collect(); | |||
There was a problem hiding this comment.
It doesn't have to be addressed in this PR, but I guess this won't work well with spaces within a quoted field, like foo."bar baz".blarg ?
There was a problem hiding this comment.
On possibility for the future, which might be more resilient, could be to run the lexer on the text, and pop the tokens. Doing so, we are sure to handle it in the same way the Nickel parser would do.
There was a problem hiding this comment.
Ah yes, this will require us to use the lexer/parser (I was trying to avoid this, but we will have to use it now). I'll create another issue to track this.
There was a problem hiding this comment.
@ebresafegaga I think the lexer is enough for that, and it's both simpler and lighter than the parser I guess. There might just a bit of concatenation to do when emitting string chunks.
Closes #1040