-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Discoverabiltiy in kuksa.val.v1 API to enable e.g. Autocompletion #605
Changes from 28 commits
306dd27
67551e1
966ab61
f346c78
02082a7
c85df9e
c8a102d
f786b8e
cc4bb94
4d2c731
173c6b3
9b7be38
8608557
713d242
df7435e
40e4c1b
b8c7417
5d40304
3db4b7e
27870f9
f124d51
3bbd8c9
873d413
79a41ee
9d9cb65
aa56d51
913a3fc
effe593
28c7ae3
80aaf5a
808bd9b
aa3eace
38e7683
e4d260f
518c347
8971f7a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -13,6 +13,7 @@ | |||||
|
||||||
use regex::Regex; | ||||||
|
||||||
#[derive(Debug)] | ||||||
pub enum Error { | ||||||
RegexError, | ||||||
} | ||||||
|
@@ -51,3 +52,70 @@ pub fn to_regex(glob: &str) -> Result<Regex, Error> { | |||||
let re = to_regex_string(glob); | ||||||
Regex::new(&re).map_err(|_err| Error::RegexError) | ||||||
} | ||||||
|
||||||
pub fn to_regex_string_new(glob: &str) -> String { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why add a new function There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as before |
||||||
// Construct regular expression | ||||||
|
||||||
// Make sure we match the whole thing | ||||||
// Start from the beginning | ||||||
let mut re = String::from("^"); | ||||||
|
||||||
// Replace all "standalone" wildcards with ".*" | ||||||
re.push_str( | ||||||
&glob | ||||||
.split('.') | ||||||
.map(|part| match part { | ||||||
"*" => "[^.]+", | ||||||
"**" => ".*", | ||||||
other => other, | ||||||
}) | ||||||
.collect::<Vec<&str>>() | ||||||
.join(r"\."), | ||||||
); | ||||||
// And finally, make sure we match until EOL | ||||||
re.push('$'); | ||||||
|
||||||
re | ||||||
} | ||||||
|
||||||
pub fn to_regex_new(glob: &str) -> Result<Regex, Error> { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why add a new function There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, the intention was to adapt the existing function, I didn't expect for now that this PR would be also focused on the implementation, but just on the user-output so I didn't want to concern myself with any compatibility in the code for now. My idea was to clean up and refine the code once the output is accurate. |
||||||
let re = to_regex_string_new(glob); | ||||||
Regex::new(&re).map_err(|_err| Error::RegexError) | ||||||
} | ||||||
|
||||||
pub fn matches_path_pattern(input: &str) -> bool { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
let pattern = r"^(?:\w+(?:\.\w+)*)(?:\.\*+)?$"; | ||||||
let regex = Regex::new(pattern).unwrap(); | ||||||
|
||||||
let pattern2 = r"^(\*{2}|\*\.)?[^.]+(\.[^.]+)*\.(\*{2}|\*|[A-Za-z]+)(\.[^.]+)*\.[^.]+$"; | ||||||
let regex2 = Regex::new(pattern2).unwrap(); | ||||||
|
||||||
regex.is_match(input) || regex2.is_match(input) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These regex:es do not seem to correctly identify valid patterns. You can have a look at the regex in |
||||||
} | ||||||
|
||||||
#[tokio::test] | ||||||
async fn test_valid_request_path() { | ||||||
assert_eq!(matches_path_pattern("String.*"), true); | ||||||
assert_eq!(matches_path_pattern("String.**"), true); | ||||||
assert_eq!(matches_path_pattern("String.String.String.String.*"), true); | ||||||
assert_eq!(matches_path_pattern("String.String.String.String.**"), true); | ||||||
assert_eq!(matches_path_pattern("String.String.String.String"), true); | ||||||
assert_eq!(matches_path_pattern("String.String.String.String.String.**"), true); | ||||||
assert_eq!(matches_path_pattern("String.String.String.*.String"), true); | ||||||
assert_eq!(matches_path_pattern("String.String.String.**.String"), true); | ||||||
assert_eq!(matches_path_pattern("String.String.String.String.String.**.String"), true); | ||||||
assert_eq!(matches_path_pattern("String.String.String.String.*.String.String"), true); | ||||||
assert_eq!(matches_path_pattern("String.*.String.String"), true); | ||||||
assert_eq!(matches_path_pattern("String.String.**.String.String"), true); | ||||||
assert_eq!(matches_path_pattern("String.**.String.String"), true); | ||||||
assert_eq!(matches_path_pattern("String.String.String."), false); | ||||||
assert_eq!(matches_path_pattern("String.String.String.String.."), false); | ||||||
assert_eq!(matches_path_pattern("String.*.String.String.."), false); | ||||||
assert_eq!(matches_path_pattern("*.String.String.String.."), false); | ||||||
assert_eq!(matches_path_pattern("*.String.String.String"), true); | ||||||
assert_eq!(matches_path_pattern("**.String.String.String.**"), true); | ||||||
assert_eq!(matches_path_pattern("**.String.String.String.*"), true); | ||||||
assert_eq!(matches_path_pattern("**.String"), true); | ||||||
//assert_eq!(matches_path_pattern("*.String"), true); | ||||||
|
||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this function even "getting entries by wildcard"?
Looks more like it's doing a substring match.
Doesn't look like it's used anyway, so I would remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is not needed anymore, will be removed