Skip to content
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

Parser #2353

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Parser #2353

wants to merge 7 commits into from

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
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

interp/src/debugger/commands/core.rs Show resolved Hide resolved
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants