refactor: apply §5 high-ROI cleanups from CODE_REVIEW.md#65
Merged
Conversation
Move `settings.rs` into `settings/mod.rs` and extract the three private `validate_*` helpers into a dedicated `validate.rs` so that `mod.rs` stays focused on type definitions and file I/O. Refs #64
…dule The 950-line `settings_handler.rs` bundled 20+ `save_*` methods for hotkey, layout, storage, theme, and update concerns in one file. Group them into themed submodules so each concern lives next to its peers: - `hotkey.rs` — activation-key recording and `save_hotkey` - `layout.rs` — `save_selected_layout` - `storage.rs` — `save_max_history` / `save_max_storage` and the `has_pending_max_*` helpers - `theme.rs` — theme, language, and window-opacity persistence - `update.rs` — `save_auto_check_enabled` / `save_include_prerelease_enabled` `mod.rs` keeps shared helpers (notifications, `persist_settings_update`, selection sync, validation helpers) plus autostart / confirm-mode / hover-preview handlers that don't fit the five themes. Refs #64
…tions)
`records_list.rs` previously defined five near-duplicate helpers
(`truncate_content`, `truncate_content_with_lines`,
`truncate_content_for_list`, `truncate_content_for_grid`,
`truncate_content_for_preview`) that all routed to the same core
routine with different hard-coded line limits.
Consolidate them into a single `truncate(content, char_limit, options)`
paired with a `TruncateOptions { max_lines: Option<usize> }` struct:
`None` means character-limit only, `Some(n)` applies a line cap.
Promote the previously inline `3` (list) and `10` (tooltip preview)
magic numbers into named `LIST_CONTENT_PREVIEW_MAX_LINES` /
`TOOLTIP_CONTENT_PREVIEW_MAX_LINES` constants so each call site
documents its intent.
Refs #64
Add a dedicated `cargo-machete` step to the Precheck job in `.github/workflows/ci.yml` so unused crate dependencies fail CI before they reach main. `taiki-e/install-action` provides a cached binary install, keeping the extra CI time minimal. Keep `scripts/precheck.sh` focused on the fmt/check/clippy/test gate — contributors can opt into local machete checks via `scripts/init.sh`, which now includes `cargo-machete` in its `cargo binstall` list. Refs #64
StudentWeis
force-pushed
the
refactor/64-code-review-section-5-cleanups
branch
from
d622674 to
d82211d
Compare
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
1 participant
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.
Summary
Apply the four small, high-ROI cleanups called out in
doc/CODE_REVIEW.md§5. All changes are mechanical refactors with no user-visible behavior change — the existing 411-test suite is the safety net.Linked Issue
Closes #64
Changes
refactor(config): splitsrc/config/settings.rs(726 lines) intosettings/mod.rs+settings/validate.rs.mod.rsnow owns type definitions and file I/O;validate.rsowns the three privatevalidate_*helpers invoked bySettings::load.refactor(gui): splitsrc/gui/board/settings_handler.rs(950 lines) intosettings_handler/{mod.rs, hotkey.rs, layout.rs, storage.rs, theme.rs, update.rs}. Each themed submodule owns itssave_*methods;mod.rskeeps the shared notification/persistence helpers plus the autostart / confirm-mode / hover-preview handlers that don't fit the five themes.refactor(gui): collapse the fivetruncate_*wrappers inrecords_list.rsinto a singletruncate(content, char_limit, options)paired with aTruncateOptions { max_lines: Option<usize> }struct. Promote the previously inline3and10magic numbers into namedLIST_CONTENT_PREVIEW_MAX_LINES/TOOLTIP_CONTENT_PREVIEW_MAX_LINESconstants.ci: add a dedicatedcargo-machetestep to the Precheck job in.github/workflows/ci.yml(viataiki-e/install-actionfor a cached binary install) so unused crate dependencies fail CI before they reach main.scripts/precheck.shstays focused on the fmt/check/clippy/test gate; contributors can installcargo-machetelocally throughscripts/init.sh, which now includes it in itscargo binstalllist.Testing
scripts/precheck.shpasses locally (411 tests, 0 clippy warnings)cargo machetepasses locally (0 unused dependencies)truncate_*unit tests were updated in-place to call the new API