fix: remove getting skipped files popup#68
Conversation
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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.
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