Remove all unnecessary #[allow(...)] annotations and fix underlying issues#52
Conversation
…ssues Remove dead annotations where lint would not fire (too_many_lines on functions well under 100 lines, unwrap_used with zero unwrap calls). Replace #[allow(dead_code)] with #[cfg(test)] for test-only functions. Extract helpers to resolve too_many_lines. Introduce context structs for too_many_arguments. Take parameters by reference for needless_pass_by_value. Migrate deprecated Theme constants to new API. Replace panic! with assert!, unwrap with let-else pattern, and fix field_reassign_with_default in test code.
There was a problem hiding this comment.
Sorry @leszek3737, you have reached your weekly rate limit of 1500000 diff characters.
Please try again later or upgrade to continue using Sourcery
There was a problem hiding this comment.
Code Review
This pull request refactors the codebase to eliminate various Clippy lint suppressions, specifically targeting too_many_lines, too_many_arguments, and unwrap_used. Key changes include extracting logic into helper functions in directory_tree.rs and menu_actions.rs, introducing a MousePosition struct in mouse.rs, and utilizing context structs in search.rs. Additionally, several instances of #[allow(dead_code)] were replaced with #[cfg(test)]. Reviewer feedback suggests further streamlining job_runner.rs by removing an unnecessary intermediate variable, grouping the excessive arguments in process_batch_entry into a state struct to fully satisfy the PR's objectives, and restoring a maintainability comment regarding UTF-8 logic in viewer.rs.
…move cancel_opt intermediate
Summary
This PR removes suppression annotations across 19 files by either deleting dead
#[allow(...)]attributes or fixing the underlying lint issues they were masking.#[allow(dead_code)]on test-only items becomes#[cfg(test)](file_ops.rs,natsort.rs);#[allow(clippy::field_reassign_with_default)]in tests becomes struct-update syntax;#[allow(clippy::unwrap_used)]inwatcher.rsbecomes alet-else; deprecatedTheme::DIRECTORY-style constants intheme.rsare replaced withTheme::directory()accessors.too_many_lines: Large functions indirectory_tree.rs,menu_actions.rs,mouse.rs,search.rs,ui/dir_tree.rs, andviewer.rsare split into focused helpers; context structs (MousePosition,FileSearchContext,ContentSearchContext) replace long argument lists.cancel: Option<Arc<AtomicBool>>andProgressSnapshotinbatch.rsare now taken by reference, with all call-sites updated accordingly.Confidence Score: 4/5
Safe to merge — all changes are lint clean-up or equivalent refactors with no functional impact on production paths.
The two style observations (unreachable error branch in watcher.rs and partial MousePosition adoption in mouse.rs) are cosmetic and cannot affect runtime behavior. The apply_metadata and natsort cfg(test) gates are consistent with their callers, and the extracted helpers preserve original logic exactly.
src/fs/watcher.rs (unreachable let-else error branch) and src/input/mouse.rs (two functions not yet migrated to &MousePosition) are worth a quick glance, but neither poses a correctness risk.
Important Files Changed
unwrap()increate_fallbackwith alet-elsethat returnsErr. The error branch is logically unreachable.MousePositioncontext struct.handle_mouse_menu_barandhandle_mouse_menu_dropdownwere not updated, leaving a minor inconsistency.execute_menu_actioninto four helpers. All original MenuAction variants are accounted for.cancelandProgressSnapshotto be passed by reference. All call-sites updated consistently.#[allow(dead_code)]on five test-only functions to#[cfg(test)]. No production path affected.natsortand helpers with#[cfg(test)]. Productionnatsort_keyis unaffected.render_tree_entriesbut recomputesvisible_heightandhas_scrollbarredundantly inside the helper.push_match_segmentsto deduplicate the hex-search match-segment loop. Logic is identical to both originals.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[execute_menu_action] --> B{MenuAction type} B -->|ToggleListingMode CycleSortOrder OpenFilter ResetPanelFilter TogglePermissions| C[execute_panel_config_action] B -->|DirectoryTree FindFile CompareDirs History Hotlist CommandLine| D[execute_nav_action] B -->|Rename Chmod| E[execute_dialog_action] B -->|ViewFile EditFile Copy Move MakeDir Delete RefreshPanel ToggleHidden| F[Return KeyCode] B -->|SwapPanels SwitchPanels SaveSetup Quit OpenUserMenu| G[Inline handler] C --> H[Option result] D --> H E --> H F --> H G --> H