lualoader: Added a menu option for unmuting logs.#387
Open
b-aaz wants to merge 1 commit into
Open
Conversation
Added a menu option for toggling the `boot_menu` kenv variable so that the user can easily toggle `boot_menu` off to see the kernel messages. Useful for debugging, etc. Also moved the `core.recordDefaults()` out of the end of core.lua and put it on top of menu.lua . When the function ran at the end of core.lua, it did not capture the variables defined in loader configuration files. This is a small bug in base. Signed-off-by: b-aaz <b-aazbsd@proton.me>
Reviewer's GuideAdds support for tracking and toggling the loader’s boot_mute setting alongside other boot options, exposes a new "Mute logs" boot menu entry, and fixes when defaults are recorded so configuration-file variables are captured correctly. Sequence diagram for toggling boot_mute via the new Mute logs menu entrysequenceDiagram
actor User
participant Menu as menu.boot_options
participant Core as core
participant Loader as loader
User->>Menu: select Mute_logs_entry
Menu->>Core: setMute(mute)
alt mute is nil
Core->>Core: [toggle core.mute]
end
alt core.mute is true
Core->>Loader: setenv(boot_mute, YES)
else core.mute is false
Core->>Loader: unsetenv(boot_mute)
end
Core->>Core: core.mute = mute
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Thank you for taking the time to contribute to FreeBSD!
Please review CONTRIBUTING.md, then update and push your branch again. |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Moving
core.recordDefaults()out ofcore.luaand only invoking it frommenu.luameans any other consumer ofcore.setDefaults()that doesn’t loadmenu.luawill now seedefault_*values (especiallydefault_mute) coming from the hardcoded initializers rather than the environment; consider either keeping an initial call insidecore.lua(after config is loaded) or havingsetDefaults()lazily callrecordDefaults()if defaults haven’t been initialized. - The new
default_mute = trueinitializer will be used whenevercore.setDefaults()is called beforerecordDefaults()has run, which may unexpectedly enableboot_mute; consider defaulting this tofalseor explicitly guardingsetDefaults()to only use environment-derived values.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Moving `core.recordDefaults()` out of `core.lua` and only invoking it from `menu.lua` means any other consumer of `core.setDefaults()` that doesn’t load `menu.lua` will now see `default_*` values (especially `default_mute`) coming from the hardcoded initializers rather than the environment; consider either keeping an initial call inside `core.lua` (after config is loaded) or having `setDefaults()` lazily call `recordDefaults()` if defaults haven’t been initialized.
- The new `default_mute = true` initializer will be used whenever `core.setDefaults()` is called before `recordDefaults()` has run, which may unexpectedly enable `boot_mute`; consider defaulting this to `false` or explicitly guarding `setDefaults()` to only use environment-derived values.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
|
Verbose should set boot_mute. I did add that a month our 2 ago. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added a menu option for toggling the
boot_menukenv variable so that the user can easily toggleboot_menuoff to see the kernel messages. Useful for debugging, etc.Also moved the
core.recordDefaults()out of the end of core.lua and put it on top of menu.lua . When the function ran at the end of core.lua, it did not capture the variables defined in loader configuration files. This is a small bug in base.This feature came up on discussions and I thought it would be useful to have. The patch is also included in the unionfs ISO I have shared for testing.
Summary by Sourcery
Add support for toggling kernel boot log muting from the loader menu and ensure default boot options are captured correctly from configuration.
New Features:
Bug Fixes:
Enhancements: