refactor: deduplicate preset handler and add stage_registry! macro#200
refactor: deduplicate preset handler and add stage_registry! macro#200
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the GUI stage plumbing to reduce repetitive dispatch code and streamlines preset loading logic by extracting small helpers, improving maintainability when adding or modifying stages/presets.
Changes:
- Introduces a
stage_registry!macro to generate stage module declarations, re-exports, andStageMessage/StageConfigdispatch implementations. - Updates
LevelConfig::to_stageto accept a (currently unused)sample_ratefor uniform stage construction. - Deduplicates preset listing and preset-load task construction via
preset_names()andbuild_preset_load_tasks()helpers.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/gui/stages/mod.rs |
Replaces hand-written stage dispatch with stage_registry! macro-generated items. |
src/gui/stages/level.rs |
Aligns LevelConfig::to_stage signature with other stages by adding a _sample_rate parameter. |
src/gui/handlers/preset.rs |
Extracts helpers to avoid repeated preset-name mapping and repeated task batching when loading presets. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| stage_registry! { | ||
| Filter => filter, FilterConfig, FilterMessage; | ||
| Preamp => preamp, PreampConfig, PreampMessage; | ||
| Compressor => compressor, CompressorConfig, CompressorMessage; | ||
| ToneStack => tonestack, ToneStackConfig, ToneStackMessage; | ||
| PowerAmp => poweramp, PowerAmpConfig, PowerAmpMessage; | ||
| Level => level, LevelConfig, LevelMessage; | ||
| NoiseGate => noise_gate, NoiseGateConfig, NoiseGateMessage; | ||
| MultibandSaturator => multiband_saturator, MultibandSaturatorConfig, MultibandSaturatorMessage; | ||
| } | ||
|
|
||
| // --- StageType --- | ||
| // Kept manual: needs #[default] on one variant and tr!() i18n keys in Display. | ||
|
|
There was a problem hiding this comment.
The PR description claims adding a new stage becomes a single-line change, but StageType (and its Display impl / i18n keys) still has to be updated manually alongside the stage_registry! invocation. Consider adjusting the PR description to reflect the remaining manual steps, or extend the macro inputs to also generate StageType/Display so the claim is accurate.
| fn build_preset_load_tasks(preset: &Preset) -> Task<Message> { | ||
| let set_stage_task = Task::done(Message::SetStages(preset.stages.clone())); | ||
| let set_ir_task = match &preset.ir_name { | ||
| Some(ir_name) => Task::done(Message::IrSelected(ir_name.clone())), | ||
| None => Task::none(), | ||
| }; | ||
| let set_ir_gain_task = Task::done(Message::IrGainChanged(preset.ir_gain)); | ||
| let set_pitch_shift_task = Task::done(Message::PitchShiftChanged(preset.pitch_shift_semitones)); | ||
|
|
There was a problem hiding this comment.
build_preset_load_tasks takes &Preset and clones ir_name (and stages). In the current callers, preset is already an owned value (Option<Preset>), so this helper could take Preset by value and move stages/ir_name into the messages to avoid extra allocations/copies on every preset load.
Extract preset_names() and build_preset_load_tasks() helpers in the preset handler to eliminate repeated code. Replace all hand-written stage dispatch impls with a stage_registry! macro that generates StageType, StageMessage, StageConfig, and all their impls from a single registration list. Adding a new stage is now a single line in the macro invocation plus the stage module file.
OpenSauce
force-pushed
the
refactor/preset-cleanup-stage-macro
branch
from
76751a4 to
7364f3c
Compare
Extract preset_names() and build_preset_load_tasks() helpers in the preset handler to eliminate repeated code. Replace 4 hand-written stage dispatch impls (32 match arms) with a stage_registry! macro, making new stage addition a single-line change.