Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Parser #2353

Merged
merged 7 commits into from
Nov 29, 2024
Merged

Parser #2353

merged 7 commits into from
Nov 29, 2024

Conversation

eliascxstro
Copy link
Contributor

Created a parser to process the path we've described such as: ".-0-1-b"

@eliascxstro eliascxstro self-assigned this Nov 18, 2024
@EclecticGriffin EclecticGriffin self-requested a review November 21, 2024 16:56
Copy link
Collaborator

@EclecticGriffin EclecticGriffin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff! I left some notes to address but once those are handled we should be good to go. Just remember to tag me for a re-review when you submit those

self.nodes.clone()
}

pub fn from_iter<I>(iter: I) -> ParsePath
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As clippy points out, there is a trait called FromIterator which you should move this implementation into.

See: https://doc.rust-lang.org/std/iter/trait.FromIterator.html

Comment on lines +317 to +319
pub fn get_path(&self) -> Vec<ParseNodes> {
self.nodes.clone()
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommend making this a standard accessor which returns a reference to the vec
or a slice and cloning externally when necessary

@@ -0,0 +1,13 @@
root = { "." }

seperator = { "-" }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: typo

Comment on lines 61 to 69

// Parse the path
pub fn parse_path(input_str: &str) -> Result<ParsePath, Error<Rule>> {
let entries = PathParser::parse(Rule::path, input_str)?;
let entry = entries.single()?;

PathParser::path(entry)
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add #[allow(dead_code)] here for the moment since this PR doesn't use the
function. This will quiet clippy down.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To fix the size issue change the return type to Box<Error<Rule>> and add
.map_err(Box::new) to the final line

@eliascxstro eliascxstro merged commit 154da35 into main Nov 29, 2024
18 checks passed
@eliascxstro eliascxstro deleted the parser branch November 29, 2024 22:58
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants