Skip to content

fix: Windows shortcut ico lookup and case-insensitive system directory guard - #304

Merged
fraa2a merged 2 commits into
mainfrom
fix/windows-path-guards
Nov 6, 2026
Merged

fraa2a merged 2 commits into
mainfrom
fix/windows-path-guards

Conversation

@fraa2a

@fraa2a fraa2a commented Nov 6, 2026

Copy link
Copy Markdown
Owner

What changed?

Two small Windows fixes from the review, one commit each:

  • Shortcut .ico icons never found: existing_paths scanned ImageFormat::ALL, which has no Ico, so path(ShortcutIcon) always returned None and .ico files were orphaned. New ArtworkKind::formats() in game_artwork.rs: ShortcutIcon also scans ICO; Icon and Banner keep ALL (the constant is unchanged because modding/mod_icons.rs uses it too).
  • %SystemRoot% guard was case-sensitive: is_system_directory now uses is_within_windows_directory, which normalizes case and separators, trims trailing separators and requires a separator boundary (C:\WindowsApps does not match C:\Windows). Also covers the caller in game_library.rs. The helper is compiled for tests so it runs on Linux CI.

Validation


Generated by Claude Code

Shortcut icon lookups scanned ImageFormat::ALL, which excludes Ico, so
prepared Windows shortcut icons were never found, listed or removed.
Scan ICO for shortcut icons only; other artwork kinds are unchanged.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DGbCKVdcUfJfjZ93bYY9p3
The SystemRoot guard compared paths component by component with the raw
environment value, so C:\WINDOWS and C:\Windows did not match. Compare
normalized Windows path strings instead.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DGbCKVdcUfJfjZ93bYY9p3
@fraa2a
fraa2a merged commit 59196a2 into main Nov 6, 2026
7 checks passed
@fraa2a
fraa2a deleted the fix/windows-path-guards branch November 6, 2026 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants