From ba4c0abf5606605e8f71912634713eee53819475 Mon Sep 17 00:00:00 2001 From: Oghenevwogaga Ebresafe Date: Mon, 16 Jan 2023 12:11:58 +0100 Subject: [PATCH 1/6] Add tests --- lsp/nls/src/requests/completion.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lsp/nls/src/requests/completion.rs b/lsp/nls/src/requests/completion.rs index d3e149a84c..ec296ae934 100644 --- a/lsp/nls/src/requests/completion.rs +++ b/lsp/nls/src/requests/completion.rs @@ -572,6 +572,12 @@ mod tests { vec!["me", "ca", "cb"], ), ("Single quote", "\"", vec!["\""]), + ("Parenthesis prefix", "(alpha.beta", vec!["alpha", "beta"]), + ( + "Curly brace prefix", + "{first.second", + vec!["first", "second"], + ), ]; for (case_name, input, expected) in tests { let actual = get_identifier_path(input); From 2ebf411ca377da287dd89e4c351ae1a88041235c Mon Sep 17 00:00:00 2001 From: Oghenevwogaga Ebresafe Date: Mon, 16 Jan 2023 13:10:36 +0100 Subject: [PATCH 2/6] Rewrite `get_identifier_path` using regex matching --- lsp/nls/src/requests/completion.rs | 44 ++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/lsp/nls/src/requests/completion.rs b/lsp/nls/src/requests/completion.rs index ec296ae934..b3ef004c7a 100644 --- a/lsp/nls/src/requests/completion.rs +++ b/lsp/nls/src/requests/completion.rs @@ -267,15 +267,26 @@ fn find_fields_from_term(term: &RichTerm, path: &mut Vec) -> Vec Option> { + /// Remove quotes from a record field's name (if any) and return a + /// tuple of the field's name with the quotes removed and a bool + /// indicating if the field's name actually contained quotes. + fn remove_quotes(name: &str) -> (String, bool) { + let mut chars = name.chars(); + if let (Some('"'), Some('"')) = (chars.next(), chars.last()) { + (String::from(&name[1..name.len() - 1]), true) + } else { + (String::from(name), false) + } + } + // skip any `.` at the end of the text let text = { let mut text = String::from(text); @@ -287,16 +298,21 @@ fn get_identifier_path(text: &str) -> Option> { let result: Vec<_> = RE_SPACE.split(text.as_ref()).map(String::from).collect(); let path = result.iter().rev().next().cloned()?; - // Remove quotes from a record name - fn remove_quotes(name: &str) -> String { - let mut chars = name.chars(); - if let (Some('"'), Some('"')) = (chars.next(), chars.last()) { - String::from(&name[1..name.len() - 1]) - } else { - String::from(name) - } - } - Some(path.split('.').map(remove_quotes).collect()) + + let path = path + .split('.') + .map(|str| { + let (str, is_unicode) = remove_quotes(str); + if is_unicode { + // Nickel identifiers cannot contain unicode characters + Some(str) + } else { + RE.find(&str).map(|m| String::from(m.as_str())) + } + }) + .collect::>>()?; + + Some(path) } /// Get the identifiers before `.` for record completion. From 3a028ebd04ee86512121a8484b9a47dadcd50732 Mon Sep 17 00:00:00 2001 From: Oghenevwogaga Ebresafe Date: Mon, 16 Jan 2023 13:11:30 +0100 Subject: [PATCH 3/6] Improve tests --- lsp/nls/src/requests/completion.rs | 36 ++++++++++++++++++------------ 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/lsp/nls/src/requests/completion.rs b/lsp/nls/src/requests/completion.rs index b3ef004c7a..9ee9c8022b 100644 --- a/lsp/nls/src/requests/completion.rs +++ b/lsp/nls/src/requests/completion.rs @@ -547,58 +547,66 @@ mod tests { ( "Simple let binding with nested record index", "let person = {} in \n a.b.c.d", - vec!["a", "b", "c", "d"], + Some(vec!["a", "b", "c", "d"]), ), - ("Simple record index", " ab.dec", vec!["ab", "dec"]), + ("Simple record index", " ab.dec", Some(vec!["ab", "dec"])), ( "Multiple let bindings with nested record index ", r##"let name = { sdf.clue.add.bar = 10 } in let other = name in let another = other in name.sdf.clue.add.bar"##, - vec!["name", "sdf", "clue", "add", "bar"], + Some(vec!["name", "sdf", "clue", "add", "bar"]), ), ( "Incomplete record with nested record index", r##"{ foo = let bar = {a.b.c.d = 10} in bar.a.b.c.d"##, - vec!["bar", "a", "b", "c", "d"], + Some(vec!["bar", "a", "b", "c", "d"]), + ), + ( + "Simple record Index", + "name.class", + Some(vec!["name", "class"]), ), - ("Simple record Index", "name.class", vec!["name", "class"]), ( "Simple record Index ending with a dot", "name.class.", - vec!["name", "class"], + Some(vec!["name", "class"]), ), - ("Single record variable", "number", vec!["number"]), + ("Single record variable", "number", Some(vec!["number"])), ( "Single record variable ending with a dot", "number.", - vec!["number"], + Some(vec!["number"]), ), ( "Record binding with unicode string names for fields", r##"let x = {"fo京o" = {bar = 42}} in x."fo京o".foo"##, - vec!["x", "fo京o", "foo"], + Some(vec!["x", "fo京o", "foo"]), ), ( "Typed record binding with nested record indexing", r##"let me : _ = { name = "foo", time = 1800, a.b.c.d = 10 } in me.ca.cb"##, - vec!["me", "ca", "cb"], + Some(vec!["me", "ca", "cb"]), + ), + ("Single quote", "\"", None), + ( + "Parenthesis prefix", + "(alpha.beta", + Some(vec!["alpha", "beta"]), ), - ("Single quote", "\"", vec!["\""]), - ("Parenthesis prefix", "(alpha.beta", vec!["alpha", "beta"]), ( "Curly brace prefix", "{first.second", - vec!["first", "second"], + Some(vec!["first", "second"]), ), ]; for (case_name, input, expected) in tests { let actual = get_identifier_path(input); let expected: Option> = - Some(expected.iter().cloned().map(String::from).collect()); + expected.map(|path| path.iter().map(|s| String::from(*s)).collect()); assert_eq!(actual, expected, "test failed: {}", case_name) } } From 9fdb21003cc1683c103cfcd124470a59b93db61b Mon Sep 17 00:00:00 2001 From: Oghenevwogaga Ebresafe Date: Mon, 16 Jan 2023 13:14:54 +0100 Subject: [PATCH 4/6] Add more tests --- lsp/nls/src/requests/completion.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lsp/nls/src/requests/completion.rs b/lsp/nls/src/requests/completion.rs index 9ee9c8022b..9d2958f198 100644 --- a/lsp/nls/src/requests/completion.rs +++ b/lsp/nls/src/requests/completion.rs @@ -593,15 +593,17 @@ mod tests { ), ("Single quote", "\"", None), ( - "Parenthesis prefix", - "(alpha.beta", + "Nested Parenthesis prefix", + "(alpha.beta.", Some(vec!["alpha", "beta"]), ), + ("Parenthesis prefix", "(single", Some(vec!["single"])), ( - "Curly brace prefix", + "Nested curly brace prefix", "{first.second", Some(vec!["first", "second"]), ), + ("Curly brace prefix", "{double.", Some(vec!["double"])), ]; for (case_name, input, expected) in tests { let actual = get_identifier_path(input); From a519f3706b44bf68ec380c90482d4ed523e0da94 Mon Sep 17 00:00:00 2001 From: Oghenevwogaga Ebresafe Date: Mon, 16 Jan 2023 13:18:29 +0100 Subject: [PATCH 5/6] Improve doc to `remove_quotes` --- lsp/nls/src/requests/completion.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lsp/nls/src/requests/completion.rs b/lsp/nls/src/requests/completion.rs index 9d2958f198..e218b2a7f2 100644 --- a/lsp/nls/src/requests/completion.rs +++ b/lsp/nls/src/requests/completion.rs @@ -275,9 +275,9 @@ lazy_static! { /// Get the string chunks that make up an identifier path. fn get_identifier_path(text: &str) -> Option> { - /// Remove quotes from a record field's name (if any) and return a - /// tuple of the field's name with the quotes removed and a bool - /// indicating if the field's name actually contained quotes. + /// Remove quotes from a record fields name (if any) and return a tuple + /// of a String (the fields name) with the quotes removed and a bool + /// indicating if the fields name actually contained quotes. fn remove_quotes(name: &str) -> (String, bool) { let mut chars = name.chars(); if let (Some('"'), Some('"')) = (chars.next(), chars.last()) { From 6d16a79ab875ced03456ed07813c6562d93c1cc6 Mon Sep 17 00:00:00 2001 From: Oghenevwogaga Ebresafe Date: Mon, 16 Jan 2023 16:08:49 +0100 Subject: [PATCH 6/6] Apply suggestions from review --- lsp/nls/src/requests/completion.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lsp/nls/src/requests/completion.rs b/lsp/nls/src/requests/completion.rs index e218b2a7f2..09b9b8f25e 100644 --- a/lsp/nls/src/requests/completion.rs +++ b/lsp/nls/src/requests/completion.rs @@ -269,7 +269,7 @@ fn find_fields_from_term(term: &RichTerm, path: &mut Vec) -> Vec Option> { let path = path .split('.') .map(|str| { - let (str, is_unicode) = remove_quotes(str); - if is_unicode { + let (str, had_quotes) = remove_quotes(str); + if had_quotes { // Nickel identifiers cannot contain unicode characters Some(str) } else { - RE.find(&str).map(|m| String::from(m.as_str())) + RE_IDENTIFIER.find(&str).map(|m| String::from(m.as_str())) } }) .collect::>>()?; @@ -321,7 +321,7 @@ fn get_identifiers_before_field(text: &str) -> Option> { let text: String = text .chars() .rev() - .skip_while(|c| RE.is_match(c.to_string().as_ref())) + .skip_while(|c| RE_IDENTIFIER.is_match(c.to_string().as_ref())) .collect::() .chars() .rev()