-
Notifications
You must be signed in to change notification settings - Fork 2
fix: remove getting skipped files popup #68
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
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 PR removes the "Getting Skipped Files" popup by replacing the synchronous, UI-blocking Task implementation with a cache-based approach that loads skipped worktree files asynchronously in the background.
Key changes:
- Introduced
SkippedWorktreeFilesCacheto manage async loading and caching of skipped worktree files - Converted
GetSkippedWorktreeFilesTaskfrom a UI-blocking Task to a standalone suspend function - Modified the changes view modifier to use cached data instead of triggering a blocking popup
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| SkippedWorktreeFilesCache.kt | New service that handles async loading and caching of skipped worktree files |
| SkippedWorktreeChangesViewModifier.kt | Updated to use cache instead of ProgressManager task |
| GetSkippedWorktreeFilesTask.kt | Converted from Task class to standalone suspend function |
| ExtendedUpdateIndexTask.kt | Added cache clearing and view refresh after index updates |
| plugin.xml | Registered the new cache service |
| SkippedWorktreeFilesCacheTest.kt | New test file for cache functionality |
| SkippedWorktreeChangesViewModifierTest.kt | Updated tests to work with cache-based approach |
| GetSkippedWorktreeFilesTaskTest.kt | Updated tests for new suspend function |
| ExtendedUpdateIndexTaskTest.kt | Added verification of cache clearing behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!isLoading) { | ||
| isLoading = true | ||
| scope.launch { |
Copilot
AI
Nov 5, 2025
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.
Race condition: Multiple concurrent calls to getOrLoad() could pass the !isLoading check before isLoading is set to true, potentially launching multiple coroutines. Use an atomic boolean or synchronization mechanism to ensure thread-safe state transitions.
...otlin/com/github/monosoul/git/updateindex/extended/changes/view/SkippedWorktreeFilesCache.kt
Outdated
Show resolved
Hide resolved
...otlin/com/github/monosoul/git/updateindex/extended/changes/view/SkippedWorktreeFilesCache.kt
Show resolved
Hide resolved
.../kotlin/com/github/monosoul/git/updateindex/extended/changes/view/GetSkippedWorktreeFiles.kt
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #68 +/- ##
==========================================
- Coverage 92.85% 90.90% -1.95%
==========================================
Files 10 11 +1
Lines 112 132 +20
Branches 15 17 +2
==========================================
+ Hits 104 120 +16
- Misses 3 6 +3
- Partials 5 6 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 10 out of 10 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This change removes getting skipped files popup.
Note on implementation: given that I don't use that plugin myself atm and don't want to spend too much time working on it, this change is 99% AI generated. I did check the implementation tho and it seems to be sane.
Fixes #67