Skip to content

🧹 Gate legacy migration functions with #[cfg(windows)] #31

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.

Sign up for GitHub

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

Merged
bearice merged 1 commit into from
Feb 20, 2026
Merged

🧹 Gate legacy migration functions with #[cfg(windows)] #31

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/icon_manager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ impl IconManager {
}

// Migration support - convert old numeric IDs to string IDs
#[allow(dead_code)]
#[cfg(windows)]
Copy link

@gemini-code-assist gemini-code-assist bot Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While using #[cfg(windows)] is a definite improvement over #[allow(dead_code)], this change highlights a potential code organization improvement. Since both migrate_from_numeric_id and get_theme_from_numeric_id are legacy migration helpers used only on Windows, consider moving them into src/platform/windows/settings.rs.

This would offer several advantages:

  • Improved Separation of Concerns: IconManager would be cleaner, containing only current, cross-platform logic.
  • Co-location: The migration logic would live directly alongside its only point of use in migrate_legacy_settings.
  • Better Encapsulation: The functions could be made private to the platform::windows::settings module, as they are implementation details of the Windows settings migration.

This refactoring would also make the #[cfg(windows)] attribute on these functions unnecessary, as the entire file is already Windows-specific.

pub fn migrate_from_numeric_id(old_id: usize) -> String {
let is_cat = (old_id & 2) == 0;

Expand All @@ -235,7 +235,7 @@ impl IconManager {
}
}

#[allow(dead_code)]
#[cfg(windows)]
pub fn get_theme_from_numeric_id(old_id: usize) -> Theme {
let is_dark = (old_id & 1) == 0;
if is_dark {
Expand Down
Loading