Release: v2.50.0 #1897
Release: v2.50.0 #1897
Conversation
Also make keys case-sensitive
…descriptive explanations for allowed values
Moves module option structs out of global config and removes dynamic allocation, using static buffers and size assertions for safer, simpler management. Refactors module initialization and destruction to work with isolated option structs, eliminating the need for the previous modules container and related code. Unifies module option header usage and updates detection logic to leverage statically-sized option buffers, improving encapsulation and maintainability. Simplifies module function signatures and reduces coupling between modules and config. Enhances future extensibility and reliability by enforcing max size constraints and removing unnecessary indirections.
There was a problem hiding this comment.
Pull Request Overview
This pull request refactors the module system architecture by removing the FFModuleBaseInfo moduleInfo field from module option structures and making module info externally accessible. This architectural change reduces memory usage and improves modularity.
Key changes include:
- Removed
moduleInfofield from all module option structures - Made module info globally accessible via extern declarations
- Added size assertions to ensure option structures fit within maximum allowed size
- Updated module initialization to use external module info references
Reviewed Changes
Copilot reviewed 300 out of 363 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/modules/swap/option.h | Removed moduleInfo field and added size assertion |
| src/modules/sound/sound.h | Added extern declaration for module info |
| src/modules/sound/sound.c | Refactored module info to be external and updated parsing |
| src/modules/sound/option.h | Removed moduleInfo field and added size assertion |
| src/modules/shell/shell.h | Added extern declaration for module info |
| src/modules/shell/shell.c | Refactored module info to be external and updated parsing |
| src/modules/shell/option.h | Removed moduleInfo field and added size assertion |
| src/modules/separator/separator.h | Added extern declaration for module info |
| src/modules/separator/separator.c | Refactored module info and fixed hardcoded TODO comment |
| src/modules/separator/option.h | Removed moduleInfo field and added size assertion |
| src/modules/publicip/publicip.h | Added extern declaration and fixed naming inconsistency |
| src/modules/publicip/publicip.c | Refactored module info to be external and updated parsing |
| src/modules/publicip/option.h | Fixed struct naming and removed moduleInfo field |
Comments suppressed due to low confidence (3)
src/modules/physicaldisk/physicaldisk.c:95
- The condition
dev->temperature == dev->temperatureis always true and doesn't check for FF_PHYSICALDISK_TEMP_UNSET. This should bedev->temperature != FF_PHYSICALDISK_TEMP_UNSET.
if (dev->temperature != FF_PHYSICALDISK_TEMP_UNSET)
src/modules/gpu/gpu.c:49
- The condition
gpu->temperature == gpu->temperatureis always true and doesn't check for FF_GPU_TEMP_UNSET. This should begpu->temperature != FF_GPU_TEMP_UNSET.
if(gpu->temperature != FF_GPU_TEMP_UNSET)
src/modules/gpu/gpu.c:129
- The condition
gpu->coreUsage == gpu->coreUsageis always true and doesn't check for FF_GPU_CORE_USAGE_UNSET. This should begpu->coreUsage != FF_GPU_CORE_USAGE_UNSET.
if (gpu->coreUsage != FF_GPU_CORE_USAGE_UNSET)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
|
|
||
| ffStrbufInit(&options->namePrefix); | ||
| options->defaultRouteOnly = | ||
| #if __ANDROID__ |
There was a problem hiding this comment.
The conditional compilation directive was changed from #if __ANDROID__ || __OpenBSD__ to just #if __ANDROID__, removing the OpenBSD condition. This change in behavior should be verified as intentional.
| #if __ANDROID__ | |
| #if defined(__ANDROID__) || defined(__OpenBSD__) |
| "" | ||
| #elif __OpenBSD__ | ||
| "" | ||
| #elif __Haiku__ |
There was a problem hiding this comment.
[nitpick] Adding a new platform condition for Haiku without a corresponding icon assignment (empty string). Consider adding an appropriate icon for consistency.
src/modules/separator/separator.c
Outdated
| setlocale(LC_CTYPE, ""); | ||
| mbstate_t state = {}; | ||
| bool fqdn = instance.config.modules.title.fqdn; | ||
| bool fqdn = true; // TODO: Make this configurable |
There was a problem hiding this comment.
[nitpick] The hardcoded value true with a TODO comment suggests this should be made configurable. Consider addressing this technical debt or creating a tracking issue.
| bool fqdn = true; // TODO: Make this configurable | |
| bool fqdn = options->fqdn; // Now configurable via FFSeparatorOptions; ensure struct is updated accordingly |
No description provided.