-
Notifications
You must be signed in to change notification settings - Fork 146
Write nom parser for Grub menuentries #1480
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
Conversation
|
|
||
| [features] | ||
| default = ["install-to-disk"] | ||
| default = ["install-to-disk", "grub"] |
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.
@Johan-Liebert1 note I made this an opt-outable feature
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.
Code Review
This pull request introduces a nom-based parser for GRUB menuentry files, adding nom as a dependency and gating the new functionality behind a grub feature flag. The implementation is a good start, but there are several areas for improvement. My review includes suggestions to enhance error reporting, fix a bug in parsing quoted strings, improve code clarity and style by removing unused code and adhering to Rust idioms, and adjust visibility for better encapsulation. These changes will make the parser more robust and maintainable.
324c322 to
e8d9f9a
Compare
This will be needed in some scenarios for composefs. Assisted-by: Claude Code Signed-off-by: Johan-Liebert1 <[email protected]> Signed-off-by: Colin Walters <[email protected]>
e8d9f9a to
3548a1d
Compare
Johan-Liebert1
left a comment
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.
LGTM
No description provided.