Skip to content

Write nom parser for Grub menuentries#1480

Merged
cgwalters merged 1 commit into
bootc-dev:mainfrom
cgwalters:composefs-prep-grub
Aug 5, 2025
Merged

Write nom parser for Grub menuentries#1480
cgwalters merged 1 commit into
bootc-dev:mainfrom
cgwalters:composefs-prep-grub

Conversation

@cgwalters

Copy link
Copy Markdown
Collaborator

No description provided.

@cgwalters cgwalters mentioned this pull request Aug 1, 2025
22 tasks
Comment thread crates/lib/Cargo.toml

[features]
default = ["install-to-disk"]
default = ["install-to-disk", "grub"]

Copy link
Copy Markdown
Collaborator Author

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

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread crates/lib/src/parsers/grub_menuconfig.rs Outdated
Comment thread crates/lib/src/parsers/grub_menuconfig.rs Outdated
Comment thread crates/lib/src/parsers/grub_menuconfig.rs
Comment thread crates/lib/src/parsers/grub_menuconfig.rs Outdated
Comment thread crates/lib/src/parsers/grub_menuconfig.rs Outdated
Comment thread crates/lib/src/parsers/grub_menuconfig.rs Outdated
Comment thread crates/lib/src/parsers/grub_menuconfig.rs
Comment thread crates/lib/src/parsers/grub_menuconfig.rs Outdated
Comment thread crates/lib/src/parsers/grub_menuconfig.rs
@cgwalters cgwalters force-pushed the composefs-prep-grub branch from 324c322 to e8d9f9a Compare August 4, 2025 20:39
This will be needed in some scenarios for composefs.

Assisted-by: Claude Code
Signed-off-by: Johan-Liebert1 <pragyanpoudyal41999@gmail.com>
Signed-off-by: Colin Walters <walters@verbum.org>
@cgwalters cgwalters force-pushed the composefs-prep-grub branch from e8d9f9a to 3548a1d Compare August 4, 2025 20:40
@cgwalters cgwalters enabled auto-merge August 4, 2025 21:36

@Johan-Liebert1 Johan-Liebert1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@cgwalters cgwalters merged commit a279555 into bootc-dev:main Aug 5, 2025
27 checks passed
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