windows: sort installed apps by last-used and extract UWP Application Ids - #1546
windows: sort installed apps by last-used and extract UWP Application Ids#1546f-trycua wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThis PR refines Windows installed-app enumeration by introducing app-sorting logic that prioritizes last-used timestamps and extracting actual Application IDs from UWP package manifests rather than hardcoding launch tokens. Supporting clarity improvements throughout make error handling and path construction more explicit. ChangesWindows App Enumeration Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libs/cua-driver-rs/crates/platform-windows/src/win32/installed_apps.rs (1)
306-316: 💤 Low valueEdge case:
Id="pattern may match attribute suffixes.The substring search for
Id="can incorrectly match attributes ending inId, such asEntryPointId="...", extracting the wrong value. Given the "best-effort" nature and "App" fallback, this is low-risk but could cause launch failures for affected packages.♻️ Optional: Use word-boundary matching
- let id_key = "Id=\""; - let id_pos = start_tag.find(id_key)? + id_key.len(); + // Match ` Id="` or start-of-tag `Id="` to avoid suffix matches like `EntryPointId=""` + let id_key = " Id=\""; + let id_pos = start_tag + .find(id_key) + .map(|p| p + id_key.len()) + .or_else(|| { + // Handle case where Id is the first attribute (no leading space after tag name) + start_tag.strip_prefix("Application ")?.find("Id=\"").map(|p| p + "Application ".len() + "Id=\"".len()) + })?;Alternatively, a simple check that the character before
Idis whitespace would suffice:let id_key = " Id=\""; // Note leading space🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/cua-driver-rs/crates/platform-windows/src/win32/installed_apps.rs` around lines 306 - 316, The Id extraction can match suffixes like EntryPointId; update the search to require a leading whitespace so only the Id attribute is matched: replace id_key = "Id=\"" with id_key = " Id=\"" when computing id_pos (and keep the existing id_pos/id_end/app_id logic), and add a fallback: if " Id=\"" is not found in start_tag, then try the original "Id=\"" only as a last resort to preserve current behavior for edge tag layouts; reference variables/apply changes around app_pos, start_tag, id_key, id_pos, and app_id in installed_apps.rs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@libs/cua-driver-rs/crates/platform-windows/src/win32/installed_apps.rs`:
- Around line 306-316: The Id extraction can match suffixes like EntryPointId;
update the search to require a leading whitespace so only the Id attribute is
matched: replace id_key = "Id=\"" with id_key = " Id=\"" when computing id_pos
(and keep the existing id_pos/id_end/app_id logic), and add a fallback: if "
Id=\"" is not found in start_tag, then try the original "Id=\"" only as a last
resort to preserve current behavior for edge tag layouts; reference
variables/apply changes around app_pos, start_tag, id_key, id_pos, and app_id in
installed_apps.rs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 031799bc-6e22-4d37-a76d-05a552820ee9
📒 Files selected for processing (1)
libs/cua-driver-rs/crates/platform-windows/src/win32/installed_apps.rs
Motivation
Application.Idso multi-entry packages launch correctly instead of always falling back toApp.Description
app_sort_keythat orders entries bylast_used(most-recent first) and then by lowercasenameas a deterministic fallback.read_uwp_application_idwhich parsesAppxManifest.xmlfor the first<Application ... Id="...">start tag and use thatAppIdwhen constructing the UWPlaunch_pathtoken instead of hardcodingApp..lnkreading and string formatting to improve readability.Testing
cargo buildandcargo testfor thelibs/cua-driver-rs/crates/platform-windowscrate and the test suite completed successfully.Codex Task
Summary by CodeRabbit