-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor/clippy #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor/clippy #21
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request refactors the main application code to improve maintainability by introducing the AppFlags struct to group related boolean UI flags, extracting logic into dedicated methods, and applying various clippy-suggested improvements including better string formatting, const fn annotations, and more idiomatic Rust patterns.
Changes:
- Introduced
AppFlagsstruct to consolidate UI state flags (show_options_window, zen_mode, show_zen_info_modal, export_request) - Refactored monolithic
updatemethod into smaller, focused methods for better code organization - Enhanced CSS parsing with dedicated type-safe functions for radius and spacing values
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/app.rs | Introduced AppFlags struct, extracted update logic into helper methods, improved state management |
| src/ui/options.rs | Created OptionsState struct, extracted section rendering into separate functions |
| src/ui/table.rs | Removed unused variable, added helper functions for table column building, improved let-chain syntax |
| src/ui/header.rs | Refactored gradient text color calculations, updated parameter types, improved string formatting |
| src/parser.rs | Simplified error handling with io::Error::other, updated string formatting |
| src/models.rs | Applied const fn to constructors, replaced empty string literals with String::new() |
| src/main.rs | Updated string formatting to use inline format syntax |
| src/icons.rs | Simplified iterator usage in tests |
| src/css.rs | Added parse_radius and parse_spacing functions with proper type safety, improved error handling |
| src/config.rs | Simplified error handling with io::Error::other |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This change forcus to make it easy to maintain
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 39 out of 40 changed files in this pull request and generated 10 comments.
Comments suppressed due to low confidence (4)
src/config/user.rs:40
- The use of
io::Error::otheris not available in stable Rust. Useio::Error::new(io::ErrorKind::Other, e.to_string())instead for stable compatibility.
src/ui/styling/css.rs:37 - The
parse_radiusfunction usesfloor()which truncates toward negative infinity, but this is converting from f32 to u8 which should be non-negative. Usinground()would provide more accurate conversion. For example, 6.8 would become 6 with floor but should become 7 for better accuracy.
src/hyprland/models.rs:36 - The
newmethod cannot beconstbecause it acceptsStringparameters.Stringis not a const-constructible type in Rust, and this will cause a compilation error. Theconstkeyword should be removed.
src/ui/styling/css.rs:45 - The
parse_spacingfunction usesfloor()which truncates values. For negative values close to zero (e.g., -0.4), this would result in -1 instead of 0. Usinground()would provide more intuitive behavior for CSS spacing values.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 39 out of 40 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/hyprland/models.rs:36
- The
const fn newdeclaration cannot acceptStringarguments in Rust.Stringis not a const-constructible type because it involves heap allocation. This function should be changed to a regularpub fn newinstead ofpub const fn new.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ry2x
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bump up version to 0.1.4
This pull request refactors the main application logic to improve maintainability and extensibility, especially around UI state management, keyboard navigation, and theme/CSS handling. The most significant change is the introduction of the
AppFlagsstruct to group related boolean flags, which simplifies state handling and reduces code duplication. Additionally, several UI and logic functions are modularized for clarity, and custom theme/CSS support is enhanced with more robust parsing and default handling.Refactoring and State Management:
AppFlagsstruct to encapsulate related boolean UI state flags (such asshow_options_window,zen_mode,show_zen_info_modal, andexport_request), replacing scattered individual fields inKeybindsAppand updating all relevant logic to access these flags through the new struct. [1] [2] [3] [4] [5] [6]handle_zen_mode_shortcuts,handle_search_bar_focus,apply_theme_or_css,handle_keyboard_navigation, etc.), leading to a cleaner and more modularupdatefunction. [1] [2] [3] [4] [5] [6]UI and Interaction Improvements:
handle_keyboard_navigation), simplifying row selection and making behavior more consistent and easier to maintain. [1] [2]AppFlagsstructure, and refactored options state passing to use a struct for better extensibility.Theme and CSS Handling:
parse_radius,parse_spacing), using clamping and type safety to avoid overflows and errors. Also added default constants for these values. [1] [2]Code Quality and Consistency:
These changes collectively make the codebase easier to extend and maintain, especially as new UI flags or configuration options are added.