From 9c8aa3ac1d8a92792b84d7261efd98179307ce63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20W=C3=A5llberg?= Date: Sat, 15 Aug 2026 11:18:40 +0200 Subject: [PATCH 01/14] fix: use the merge base for common-ancestor scopes "Only Changes Since Common Ancestor" resolved its range by walking `git log` and unioning every commit's changes, so files reverted inside the range stayed listed and each path's base came from the oldest commit that touched it rather than the merge base. Resolve `git merge-base HEAD` instead and feed it through the same single-base diff the other scopes use, making a range scope exactly `git diff ...HEAD` -- the diff a pull request shows. Range parsing lives in ScopeRefRange, free of platform types so it can be unit tested. It accepts both dot forms but requires HEAD on the right, reporting anything else as an invalid scope rather than silently reading it as "since the common ancestor with HEAD". Also corrects the checkbox label, stops appending "...HEAD" to refs that already carry a range, drops the now-unused GitCommit.getChanges() reflection bridge, and runs :backend:test in CI. Fixes #104 Co-authored-by: Ondrej Smola --- .github/workflows/build.yml | 3 + CHANGELOG.md | 3 + backend/build.gradle.kts | 1 + .../compare/ChangesService.java | 45 ++++------ .../listener/MyTreeSelectionListener.java | 9 +- backend/src/main/java/model/MyModel.java | 9 +- .../java/toolwindow/BranchSelectView.java | 10 ++- backend/src/main/java/utils/GitUtil.java | 35 ++++++++ .../java/utils/PlatformApiReflection.java | 40 --------- .../src/main/java/utils/ScopeRefRange.java | 84 +++++++++++++++++++ .../test/java/utils/ScopeRefRangeTest.java | 60 +++++++++++++ 11 files changed, 222 insertions(+), 77 deletions(-) create mode 100644 backend/src/main/java/utils/ScopeRefRange.java create mode 100644 backend/src/test/java/utils/ScopeRefRangeTest.java diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3c7de05..bd607da 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -30,6 +30,9 @@ jobs: - name: Set up Gradle uses: gradle/actions/setup-gradle@v6 + - name: Run Tests + run: ./gradlew :backend:test + - name: Build Plugin run: ./gradlew buildPlugin diff --git a/CHANGELOG.md b/CHANGELOG.md index 3dbc1c6..943d451 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,9 @@ locally or over Remote Development. - Fixed [Right-click -> Show in Project doesn't switch to Project window](https://github.com/comod/git-scope-pro/issues/100) - Fixed [Right-click -> Show in Project for folder selects the first changed file rather than the folder](https://github.com/comod/git-scope-pro/issues/101) +- Fixed ["Only Changes Since Common Ancestor" collected every commit in the range instead of the pull-request diff](https://github.com/comod/git-scope-pro/issues/104) + - The scope is now `git diff ...HEAD`: files reverted within the range disappear, and each + file's diff, gutter markers and rollback use the merge base rather than a single intermediate commit. ## [2026.1.4] diff --git a/backend/build.gradle.kts b/backend/build.gradle.kts index 3846f31..7232339 100644 --- a/backend/build.gradle.kts +++ b/backend/build.gradle.kts @@ -31,4 +31,5 @@ dependencies { implementation(project(":shared")) compileOnly("com.google.code.gson:gson:2.14.0") + testImplementation("junit:junit:4.13.2") } diff --git a/backend/src/main/java/implementation/compare/ChangesService.java b/backend/src/main/java/implementation/compare/ChangesService.java index adaebfb..524aa6b 100644 --- a/backend/src/main/java/implementation/compare/ChangesService.java +++ b/backend/src/main/java/implementation/compare/ChangesService.java @@ -15,11 +15,9 @@ import com.intellij.openapi.vcs.changes.ChangesUtil; import com.intellij.openapi.vcs.changes.CurrentContentRevision; import com.intellij.openapi.vfs.VirtualFile; -import git4idea.GitCommit; import git4idea.GitReference; import git4idea.GitRevisionNumber; import git4idea.actions.GitCompareWithRefAction; -import git4idea.history.GitHistoryUtils; import git4idea.repo.GitRepository; import model.TargetBranchMap; import org.jetbrains.annotations.NotNull; @@ -28,6 +26,7 @@ import system.Defs; import utils.PlatformApiReflection; import utils.GitUtil; +import utils.ScopeRefRange; import java.util.*; import java.util.concurrent.ConcurrentHashMap; @@ -290,19 +289,6 @@ private Collection filterLocalChanges(Collection localChanges, S return filtered; } - @NotNull - public Collection getChangesByHistory(Project project, GitRepository repo, String branchToCompare) throws VcsException { - List commits = GitHistoryUtils.history(project, repo.getRoot(), branchToCompare); - Map changeMap = new HashMap<>(); - for (GitCommit commit : commits) { - for (Change change : PlatformApiReflection.getCommitChanges(commit)) { - FilePath path = ChangesUtil.getFilePath(change); - changeMap.put(path, change); - } - } - return new ArrayList<>(changeMap.values()); - } - /** * Collects local changes for HEAD (uncommitted changes) filtered by repository. * @@ -357,8 +343,13 @@ public RepoChangesResult doCollectChanges(Project project, GitRepository repo, S } // Diff Changes - these are the pure scope changes - if (scopeRef.contains("..")) { - scopeChanges = getChangesByHistory(project, repo, scopeRef); + GitRevisionNumber revisionNumber; + if (ScopeRefRange.isRange(scopeRef)) { + // A range scope ("main..HEAD") asks for everything on HEAD since it diverged from the + // selected ref, so the base is their merge base. An unsupported range yields no ref + // and falls through to ERROR_STATE rather than being misread as a different diff. + String selectedRef = ScopeRefRange.selectedRef(scopeRef); + revisionNumber = selectedRef == null ? null : GitUtil.resolveMergeBase(repo, selectedRef); } else { GitReference gitReference; @@ -369,7 +360,6 @@ public RepoChangesResult doCollectChanges(Project project, GitRepository repo, S gitReference = PlatformApiReflection.findTagByName(repo, scopeRef); } - GitRevisionNumber revisionNumber; if (gitReference == null) { // Finally resort to try a generic reference (HEAD~2, , ...) revisionNumber = GitUtil.resolveGitReference(repo, scopeRef); @@ -377,16 +367,17 @@ public RepoChangesResult doCollectChanges(Project project, GitRepository repo, S else { revisionNumber = new GitRevisionNumber(gitReference.getFullName()); } + } - if (revisionNumber != null) { - // We have a valid GitReference - scopeChanges = GitUtil.getDiffChanges(repo, file, revisionNumber); - LOG.debug("ChangesService - Repository: " + repoPath + ", Scope: " + scopeRef + ", scopeChanges count: " + scopeChanges.size()); - } - else { - // We do not have a valid GitReference => return ERROR_STATE - return new RepoChangesResult(ERROR_STATE, new ArrayList<>(), new ArrayList<>()); - } + if (revisionNumber != null) { + // Diff a single base tree against HEAD. Range scopes pass their merge base, which makes + // the result the net pull-request diff instead of a union of every commit's changes. + scopeChanges = GitUtil.getDiffChanges(repo, file, revisionNumber); + LOG.debug("ChangesService - Repository: " + repoPath + ", Scope: " + scopeRef + ", base: " + revisionNumber.asString() + ", scopeChanges count: " + scopeChanges.size()); + } + else { + // We do not have a valid GitReference => return ERROR_STATE + return new RepoChangesResult(ERROR_STATE, new ArrayList<>(), new ArrayList<>()); } // Log what we collected diff --git a/backend/src/main/java/listener/MyTreeSelectionListener.java b/backend/src/main/java/listener/MyTreeSelectionListener.java index b83fc63..50d316e 100644 --- a/backend/src/main/java/listener/MyTreeSelectionListener.java +++ b/backend/src/main/java/listener/MyTreeSelectionListener.java @@ -11,6 +11,7 @@ import state.State; import toolwindow.elements.BranchTreeEntry; import service.ViewService; +import utils.ScopeRefRange; public class MyTreeSelectionListener implements TreeSelectionListener { private final Tree tree; @@ -32,10 +33,10 @@ public void valueChanged(TreeSelectionEvent treeSelectionEvent) { Object object = node.getUserObject(); if (object instanceof BranchTreeEntry favLabel) { String branchName = favLabel.getName(); - if (this.state.getTwoDotsCheckbox()) { - String twoDots = ".."; - String head = "HEAD"; - branchName = branchName + twoDots + head; + // Skip refs that already carry a range, otherwise a manually entered "a..b" would + // become the unresolvable "a..b...HEAD". + if (this.state.getTwoDotsCheckbox() && !ScopeRefRange.isRange(branchName)) { + branchName = branchName + "..." + GitService.BRANCH_HEAD; } // Check if this is HEAD - if so, close current tab and switch to HEAD tab diff --git a/backend/src/main/java/model/MyModel.java b/backend/src/main/java/model/MyModel.java index 8e8a709..b2617bc 100644 --- a/backend/src/main/java/model/MyModel.java +++ b/backend/src/main/java/model/MyModel.java @@ -5,6 +5,7 @@ import git4idea.repo.GitRepository; import org.jetbrains.annotations.Nullable; import service.GitService; +import utils.ScopeRefRange; import java.util.Collection; import java.util.HashMap; @@ -79,15 +80,13 @@ public String getDisplayName() { } /** - * Returns the scope reference similar to getName(), but strips the optional "..HEAD" suffix if present. - * Example: "feature/foo..HEAD" -> "feature/foo" + * Returns the scope reference similar to getName(), but strips the optional range suffix if present. + * Example: "feature/foo...HEAD" -> "feature/foo" */ @Nullable public String getScopeRef() { String name = getName(); - if (name == null) return null; - String suffix = ".." + GitService.BRANCH_HEAD; - return name.endsWith(suffix) ? name.substring(0, name.length() - suffix.length()) : name; + return name == null ? null : ScopeRefRange.stripRange(name); } // Getter and setter for custom tab name diff --git a/backend/src/main/java/toolwindow/BranchSelectView.java b/backend/src/main/java/toolwindow/BranchSelectView.java index bb9a6f4..0526c46 100644 --- a/backend/src/main/java/toolwindow/BranchSelectView.java +++ b/backend/src/main/java/toolwindow/BranchSelectView.java @@ -108,7 +108,15 @@ public BranchSelectView(Project project) { JPanel help = new JPanel(); help.setLayout(new FlowLayout(FlowLayout.LEFT)); - JCheckBox checkBox = new JCheckBox("Only Changes Since Common Ancestor (git diff ..HEAD)"); + JCheckBox checkBox = new JCheckBox("Only Changes Since Common Ancestor (git diff ...HEAD)"); + checkBox.setToolTipText( + "" + + "Compares HEAD against its common ancestor with the selection,
" + + "so changes made on the selected branch since you branched off are excluded.
" + + "This is the diff a pull request shows.
" + + "Unchecked, the selection is compared directly to HEAD." + + "" + ); checkBox.setSelected(this.state.getTwoDotsCheckbox()); checkBox.setBorder(JBUI.Borders.empty(1)); // top, left, bottom, right padding checkBox.addActionListener(e -> this.state.setTwoDotsCheckbox(checkBox.isSelected())); diff --git a/backend/src/main/java/utils/GitUtil.java b/backend/src/main/java/utils/GitUtil.java index aa993da..5a00e88 100644 --- a/backend/src/main/java/utils/GitUtil.java +++ b/backend/src/main/java/utils/GitUtil.java @@ -69,6 +69,41 @@ public static GitRevisionNumber resolveGitReference(@NotNull GitRepository repos return null; } + /** + * Resolves the common ancestor of the given ref and the repository's current HEAD. + * This is the base Git hosting providers use for pull-request diffs, so diffing it against + * HEAD yields {@code git diff ...HEAD}. + * + * @param repository the target GitRepository + * @param selectedRef the ref the scope compares against (branch, tag, hash, ...) + * @return the merge base, or null when it cannot be resolved (unknown ref, unrelated histories) + */ + @Nullable + public static GitRevisionNumber resolveMergeBase(@NotNull GitRepository repository, + @NotNull String selectedRef) { + + Project project = repository.getProject(); + + GitLineHandler handler = new GitLineHandler(project, repository.getRoot(), GitCommand.MERGE_BASE); + handler.addParameters(selectedRef, "HEAD"); + try { + String hash = Git.getInstance().runCommand(handler) + .getOutputOrThrow() + .trim(); + if (!hash.isEmpty()) { + return new GitRevisionNumber(hash); + } + } catch (VcsException e) { + // Expected when the ref is unknown in this repository or histories are unrelated + LOG.debug("Failed to resolve merge base of '" + selectedRef + "' and HEAD: " + e.getMessage()); + } catch (Exception e) { + LOG.warn("Unexpected error resolving merge base of '" + selectedRef + "' and HEAD in repository " + + repository.getRoot().getPath(), e); + } + + return null; + } + public static @NotNull Collection getDiffChanges(@NotNull GitRepository repository, @NotNull VirtualFile file, @NotNull GitRevisionNumber revisionNumber) throws VcsException { diff --git a/backend/src/main/java/utils/PlatformApiReflection.java b/backend/src/main/java/utils/PlatformApiReflection.java index 339e327..84303e7 100644 --- a/backend/src/main/java/utils/PlatformApiReflection.java +++ b/backend/src/main/java/utils/PlatformApiReflection.java @@ -1,8 +1,6 @@ package utils; import com.intellij.openapi.diagnostic.Logger; -import com.intellij.openapi.vcs.changes.Change; -import git4idea.GitCommit; import git4idea.GitReference; import git4idea.repo.GitRepository; import org.jetbrains.annotations.NotNull; @@ -13,8 +11,6 @@ import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; import java.lang.reflect.Method; -import java.util.Collection; -import java.util.Collections; import java.util.Map; /** @@ -30,8 +26,6 @@ * *

Bridges

*
    - *
  • {@link #getCommitChanges} — {@code GitCommit.getChanges()} via its - * {@code @ApiStatus.Experimental} annotation
    • *
  • {@link #findTagByName} — tag lookup via the newer {@code getTagsHolder()} * (2026.1+) or legacy {@code getTagHolder()} (older IDEs)
    • *
@@ -43,10 +37,6 @@ public final class PlatformApiReflection { private static final Logger LOG = Defs.getLogger(PlatformApiReflection.class); - // ── GitCommit.getChanges() (@ApiStatus.Experimental) ──────────────────── - // type after adaptation: (Object receiver) -> Object - private static final @Nullable MethodHandle COMMIT_GET_CHANGES; - // ── Tag lookup (IDE 2026.1+): getTagsHolder() API ──────────────────────── // Each handle: (Object receiver [, args]) -> Object private static final @Nullable MethodHandle REPO_GET_TAGS_HOLDER; @@ -59,9 +49,6 @@ public final class PlatformApiReflection { private static final @Nullable MethodHandle TAG_HOLDER_GET_TAG; // (Object, Object name) -> Object static { - // ── GitCommit.getChanges() ───────────────────────────────────────── - COMMIT_GET_CHANGES = resolvePublicVirtual(GitCommit.class, "getChanges"); - // ── New tag path ─────────────────────────────────────────────────── REPO_GET_TAGS_HOLDER = resolvePublicVirtual(GitRepository.class, "getTagsHolder"); TAGS_HOLDER_GET_STATE = @@ -116,33 +103,6 @@ private PlatformApiReflection() {} // ── Public API ──────────────────────────────────────────────────────────── - /** - * Calls {@code commit.getChanges()} via a cached {@link MethodHandle}. - * The method is annotated {@code @ApiStatus.Experimental} and therefore accessed - * reflectively to avoid verifyPlugin warnings. - * - * @return the commit's changes, or an empty list when the handle is unavailable - */ - @NotNull - @SuppressWarnings("unchecked") - public static Collection getCommitChanges(@NotNull GitCommit commit) { - if (COMMIT_GET_CHANGES == null) { - LOG.warn("PlatformApiReflection: getChanges handle unavailable"); - return Collections.emptyList(); - } - try { - Object result = COMMIT_GET_CHANGES.invoke(commit); - if (result instanceof Collection c) { - return (Collection) c; - } - LOG.warn("PlatformApiReflection: getChanges returned unexpected type: " - + (result == null ? "null" : result.getClass().getName())); - } catch (Throwable t) { - LOG.error("PlatformApiReflection: getCommitChanges invocation failed", t); - } - return Collections.emptyList(); - } - /** * Finds a Git tag by name on the given repository. * diff --git a/backend/src/main/java/utils/ScopeRefRange.java b/backend/src/main/java/utils/ScopeRefRange.java new file mode 100644 index 0000000..dbfa5db --- /dev/null +++ b/backend/src/main/java/utils/ScopeRefRange.java @@ -0,0 +1,84 @@ +package utils; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * Parses the Git range syntax a scope can carry: the {@code ..HEAD} produced by the + * "Only Changes Since Common Ancestor" checkbox, and anything a user types manually. + * + *

A scope has exactly two meanings, and a range expresses the second one: + *

    + *
  • a bare ref ({@code main}) — compare that ref directly to HEAD, i.e. {@code git diff main HEAD}
    • + *
  • a range against HEAD ({@code main..HEAD}, {@code main...HEAD}) — everything on HEAD since it + * diverged from the ref, i.e. {@code git diff main...HEAD}, the diff a pull request shows
    • + *
+ * + *

Both dot forms are accepted for the second meaning: a strict two-dot reading would be identical + * to selecting the bare ref, which the UI already offers, and older saved scopes use two dots. The + * right-hand side must be HEAD (or empty, which Git itself defaults to HEAD) — any other target is a + * comparison this plugin cannot express, and is reported rather than silently misread as + * "since the common ancestor with HEAD". + * + *

Git ref names may not contain two consecutive dots (see {@code git check-ref-format}), so the + * first {@code ".."} is an unambiguous separator. + * + *

Deliberately free of IntelliJ Platform types so it can be unit tested without an IDE. + */ +public final class ScopeRefRange { + + private static final String SEPARATOR = ".."; + private static final String HEAD = "HEAD"; + + private ScopeRefRange() {} + + /** + * Returns whether the scope uses Git range syntax at all, supported or not. + */ + public static boolean isRange(@Nullable String scopeRef) { + return scopeRef != null && scopeRef.contains(SEPARATOR); + } + + /** + * Returns the ref on the left of a supported range, whose merge base with HEAD is the diff base. + * + * @return the selected ref, or {@code null} when the scope is not a range, has no left side, or + * targets something other than HEAD + */ + @Nullable + public static String selectedRef(@Nullable String scopeRef) { + if (scopeRef == null) { + return null; + } + + int separator = scopeRef.indexOf(SEPARATOR); + if (separator < 0) { + return null; + } + + String selectedRef = scopeRef.substring(0, separator).trim(); + if (selectedRef.isEmpty()) { + return null; + } + + String target = scopeRef.substring(separator + SEPARATOR.length()); + if (target.startsWith(".")) { + // Three-dot form: drop the extra dot to expose the target ref. + target = target.substring(1); + } + target = target.trim(); + + // An omitted target means HEAD in Git, so "main.." is the same request as "main..HEAD". + return target.isEmpty() || HEAD.equals(target) ? selectedRef : null; + } + + /** + * Strips a supported {@code ..HEAD}/{@code ...HEAD} suffix so the scope can be shown or used as a + * plain ref. Refs without a range, and ranges this plugin does not support, are returned unchanged. + */ + @NotNull + public static String stripRange(@NotNull String scopeRef) { + String selectedRef = selectedRef(scopeRef); + return selectedRef == null ? scopeRef : selectedRef; + } +} diff --git a/backend/src/test/java/utils/ScopeRefRangeTest.java b/backend/src/test/java/utils/ScopeRefRangeTest.java new file mode 100644 index 0000000..f69777a --- /dev/null +++ b/backend/src/test/java/utils/ScopeRefRangeTest.java @@ -0,0 +1,60 @@ +package utils; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +public class ScopeRefRangeTest { + + @Test + public void detectsRangeSyntax() { + assertTrue(ScopeRefRange.isRange("main..HEAD")); + assertTrue(ScopeRefRange.isRange("main...HEAD")); + assertTrue(ScopeRefRange.isRange("v1.0..v2.0")); + assertFalse(ScopeRefRange.isRange("main")); + assertFalse(ScopeRefRange.isRange("HEAD~3")); + assertFalse(ScopeRefRange.isRange("release-1.2")); + assertFalse(ScopeRefRange.isRange(null)); + } + + @Test + public void extractsSelectedRefFromSupportedRanges() { + assertEquals("main", ScopeRefRange.selectedRef("main..HEAD")); + assertEquals("main", ScopeRefRange.selectedRef("main...HEAD")); + assertEquals("origin/release", ScopeRefRange.selectedRef("origin/release...HEAD")); + assertEquals("HEAD~3", ScopeRefRange.selectedRef("HEAD~3..HEAD")); + assertEquals("abc123", ScopeRefRange.selectedRef("abc123...HEAD")); + assertEquals("feature/a.b", ScopeRefRange.selectedRef("feature/a.b...HEAD")); + // Git defaults an omitted side to HEAD + assertEquals("main", ScopeRefRange.selectedRef("main..")); + assertEquals("main", ScopeRefRange.selectedRef("main...")); + } + + @Test + public void rejectsRangesThatDoNotTargetHead() { + assertNull(ScopeRefRange.selectedRef("v1.0..v2.0")); + assertNull(ScopeRefRange.selectedRef("main...feature")); + assertNull(ScopeRefRange.selectedRef("a..b..HEAD")); + assertNull(ScopeRefRange.selectedRef("main....HEAD")); + } + + @Test + public void rejectsScopesWithoutSelectedRef() { + assertNull(ScopeRefRange.selectedRef("..HEAD")); + assertNull(ScopeRefRange.selectedRef("...HEAD")); + assertNull(ScopeRefRange.selectedRef("HEAD")); + assertNull(ScopeRefRange.selectedRef("main")); + assertNull(ScopeRefRange.selectedRef(null)); + } + + @Test + public void stripsSupportedRangesOnly() { + assertEquals("main", ScopeRefRange.stripRange("main...HEAD")); + assertEquals("main", ScopeRefRange.stripRange("main..HEAD")); + assertEquals("main", ScopeRefRange.stripRange("main")); + assertEquals("v1.0..v2.0", ScopeRefRange.stripRange("v1.0..v2.0")); + } +} From 3b447bac3c6df17dfd9cfe2406dc78416cb5cc5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20W=C3=A5llberg?= Date: Sat, 15 Aug 2026 15:07:47 +0200 Subject: [PATCH 02/14] fix: make split-mode gutter updates survive slow and unstable connections The RPC transport applies credit-based backpressure, so a slow frontend or network legitimately stops the stream from being drained. The backend bridge broke under exactly that condition: listener callbacks pushed payloads into a callbackFlow via trySend, which silently discards events once the 64-element buffer fills, and DataUpdated events are state-bearing with no retransmission, so every discarded event was a file whose gutter never rendered. Restructure the producer around conflation by file: listeners only mark a file dirty and wake the emit loop, which reads the latest snapshot at send time and emits with a suspending call, so intermediate states collapse into the newest one, pending memory is bounded at one entry per file, and the final state always arrives. On the frontend, supervise the subscription. durable only retries RPC-level failures; two other terminations used to kill the gutter silently until IDE restart, namely the backend answering with an empty flow because it does not know the project yet, and any exception escaping event handling. Exceptions applying a single event are now logged and skipped, and a completed or failed stream is re-subscribed after a short delay. --- .../src/main/java/rpc/BackendGutterRpcImpl.kt | 99 +++++++++++++++---- .../main/java/rpc/FrontendGutterListeners.kt | 47 ++++++++- 2 files changed, 123 insertions(+), 23 deletions(-) diff --git a/backend/src/main/java/rpc/BackendGutterRpcImpl.kt b/backend/src/main/java/rpc/BackendGutterRpcImpl.kt index 381a2e1..2340f20 100644 --- a/backend/src/main/java/rpc/BackendGutterRpcImpl.kt +++ b/backend/src/main/java/rpc/BackendGutterRpcImpl.kt @@ -5,40 +5,101 @@ import com.intellij.platform.project.ProjectId import com.intellij.platform.project.findProjectOrNull import com.intellij.platform.rpc.backend.RemoteApiProvider import fleet.rpc.remoteApiDescriptor -import kotlinx.coroutines.channels.awaitClose +import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.callbackFlow +import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.flow.flow import service.GutterDataService import settings.GitScopeSettings class BackendGutterRpcImpl : GutterRpcApi { + + /** + * Streams gutter updates with conflation by file. + * + *

The RPC transport applies credit-based backpressure: a slow frontend or network stops the + * stream from being drained. Listener callbacks therefore never push payloads into the stream — + * they only mark a file dirty and wake the emit loop, which reads the latest snapshot + * from [GutterDataService] at send time and emits with a suspending call. Under backpressure, + * intermediate states of a file collapse into the newest one instead of being dropped (the + * previous implementation lost events irrecoverably via {@code trySend} once its 64-element + * buffer filled), memory is bounded at one pending entry per file, and the final state always + * arrives. + * + *

The initial replay goes through the same dirty set, so a reconnecting consumer (the + * frontend re-subscribes via {@code durable} after every connection loss) never queues more + * than one snapshot per file no matter how often the link drops. + */ override suspend fun getGutterUpdates(projectId: ProjectId): Flow { - val project = projectId.findProjectOrNull() ?: return kotlinx.coroutines.flow.emptyFlow() + val project = projectId.findProjectOrNull() ?: return emptyFlow() val gds = project.service() - return callbackFlow { - // Register listener FIRST to avoid missing events during replay - val listener = object : GutterDataService.Listener { - override fun onDataUpdated(filePath: String, data: GutterDataService.GutterFileData) { - trySend(GutterUpdateEvent.DataUpdated(data.toDto(filePath, gds.scopeDisplayName))) - } + return flow { + // Conflated: always accepts, coalesces repeated wake-ups; listeners never block or fail. + val signal = Channel(Channel.CONFLATED) + val lock = Any() + var allCleared = false + val dirty = LinkedHashSet() - override fun onDataCleared(filePath: String) { - trySend(GutterUpdateEvent.DataCleared(filePath)) + fun markDirty(filePath: String?) { + synchronized(lock) { + if (filePath == null) { + allCleared = true + // Dirt older than the clear is obsolete; later updates re-add themselves. + dirty.clear() + } else { + dirty.add(filePath) + } } + signal.trySend(Unit) + } - override fun onAllCleared() { - trySend(GutterUpdateEvent.AllCleared) - } + val listener = object : GutterDataService.Listener { + override fun onDataUpdated(filePath: String, data: GutterDataService.GutterFileData) = + markDirty(filePath) + + override fun onDataCleared(filePath: String) = markDirty(filePath) + + override fun onAllCleared() = markDirty(null) } + // Register the listener before the replay so no update between replay and registration + // is missed; a double-send of the same file is harmless (the frontend overwrites). gds.addListener(listener) + try { + for (path in gds.getAllData().keys) { + markDirty(path) + } - // Then replay all current data (duplicates are harmless — frontend overwrites) - for (entry in gds.getAllData().entries) { - send(GutterUpdateEvent.DataUpdated(entry.value.toDto(entry.key, gds.scopeDisplayName))) - } + while (true) { + signal.receive() + while (true) { + val clearAll: Boolean + val paths: List + synchronized(lock) { + clearAll = allCleared + allCleared = false + paths = dirty.toList() + dirty.clear() + } + if (!clearAll && paths.isEmpty()) break - awaitClose { gds.removeListener(listener) } + if (clearAll) { + emit(GutterUpdateEvent.AllCleared) + } + for (path in paths) { + // Read the freshest snapshot at send time, not at event time. A file + // cleared while queued yields null and becomes a DataCleared. + val data = gds.getData(path) + emit( + if (data != null) GutterUpdateEvent.DataUpdated(data.toDto(path, gds.scopeDisplayName)) + else GutterUpdateEvent.DataCleared(path) + ) + } + } + } + } finally { + gds.removeListener(listener) + } } } diff --git a/frontend/src/main/java/rpc/FrontendGutterListeners.kt b/frontend/src/main/java/rpc/FrontendGutterListeners.kt index c490d20..0ec1526 100644 --- a/frontend/src/main/java/rpc/FrontendGutterListeners.kt +++ b/frontend/src/main/java/rpc/FrontendGutterListeners.kt @@ -7,26 +7,65 @@ import com.intellij.openapi.startup.ProjectActivity import com.intellij.platform.project.projectId import fleet.rpc.client.durable import implementation.gutter.Range +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.delay +import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import service.GutterDataService import settings.GitScopeSettings +import system.Defs @Service(Service.Level.PROJECT) class FrontendGutterSubscriptions( private val project: Project, private val coroutineScope: CoroutineScope ) { + companion object { + private val LOG = Defs.getLogger(FrontendGutterSubscriptions::class.java) + + /** + * Delay before re-subscribing after the stream ended without an RPC failure. Two ways here: + * the backend answered with an empty flow because it does not know the project yet (a race + * won by slow backends — the subscription used to die silently for good in that case), or a + * non-RPC error escaped, which `durable` deliberately does not retry. + */ + private const val RESUBSCRIBE_DELAY_MS = 5_000L + } + init { // Only subscribe to gutter updates via RPC in split mode — in monolith, // the frontend GutterDataService IS the backend GutterDataService (same instance), // so re-publishing would cause an infinite loop. if (!com.intellij.platform.ide.productMode.IdeProductMode.isMonolith) { coroutineScope.launch { - durable { - GutterRpcApi.getInstance() - .getGutterUpdates(project.projectId()) - .collect { event -> handleEvent(event) } + while (isActive) { + try { + durable { + GutterRpcApi.getInstance() + .getGutterUpdates(project.projectId()) + .collect { event -> + // One bad event must not kill the subscription: the stream is + // the only source of gutter data, so letting an exception + // propagate here would leave the gutter permanently empty. + try { + handleEvent(event) + } catch (e: CancellationException) { + throw e + } catch (t: Throwable) { + LOG.warn("Failed to apply gutter update, skipping event", t) + } + } + } + LOG.debug("Gutter update stream completed, re-subscribing") + } catch (e: CancellationException) { + throw e + } catch (t: Throwable) { + // durable already retries RPC-level failures with its own backoff; anything + // arriving here is unexpected, so log it visibly and keep the subscription alive. + LOG.warn("Gutter update subscription failed, re-subscribing", t) + } + delay(RESUBSCRIBE_DELAY_MS) } } } From 87e24ba0af28055c92587090638a8ebd486f9e83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20W=C3=A5llberg?= Date: Sat, 15 Aug 2026 15:09:35 +0200 Subject: [PATCH 03/14] perf: stop resending unchanged file contents in split-mode gutter updates Every DataUpdated message carried the full base and HEAD contents of the file, JSON-encoded, although the ranges themselves are a handful of ints. The contents rarely change -- typing, local-change recomputation and settings republishes all produce new ranges against identical contents -- so on a slow link almost all of the transferred bytes were repeats. Track per subscription which contents each file was last sent with and omit them when unchanged; a contentsIncluded flag distinguishes "unchanged, reuse your cache" from a genuinely absent headContent, and the frontend falls back to the contents it already holds. A new subscription starts with no memory and therefore always sends contents on first contact, which keeps the durable re-subscribe after a connection drop correct, and the memory is cleared together with AllCleared since the frontend drops its cache at that point. Contents are still pushed rather than fetched on demand because the frontend needs baseContent synchronously for live range recomputation while typing. What remains per ordinary update is O(ranges) instead of O(file size). --- .../src/main/java/rpc/BackendGutterRpcImpl.kt | 48 +++++++++++++------ .../main/java/rpc/FrontendGutterListeners.kt | 28 ++++++++++- shared/src/main/java/rpc/GutterTopics.kt | 12 ++++- 3 files changed, 70 insertions(+), 18 deletions(-) diff --git a/backend/src/main/java/rpc/BackendGutterRpcImpl.kt b/backend/src/main/java/rpc/BackendGutterRpcImpl.kt index 2340f20..8d9ca53 100644 --- a/backend/src/main/java/rpc/BackendGutterRpcImpl.kt +++ b/backend/src/main/java/rpc/BackendGutterRpcImpl.kt @@ -35,6 +35,11 @@ class BackendGutterRpcImpl : GutterRpcApi { val gds = project.service() return flow { + // Contents last sent to THIS subscriber, per file. Contents dominate the message size, + // so they are resent only when they actually changed; a new subscription starts empty + // and therefore always sends contents on first contact with a file. + val lastSentContents = HashMap>() + // Conflated: always accepts, coalesces repeated wake-ups; listeners never block or fail. val signal = Channel(Channel.CONFLATED) val lock = Any() @@ -84,16 +89,27 @@ class BackendGutterRpcImpl : GutterRpcApi { if (!clearAll && paths.isEmpty()) break if (clearAll) { + // The frontend drops its cache on AllCleared, so nothing previously + // sent may be referred to afterwards. + lastSentContents.clear() emit(GutterUpdateEvent.AllCleared) } for (path in paths) { // Read the freshest snapshot at send time, not at event time. A file // cleared while queued yields null and becomes a DataCleared. val data = gds.getData(path) - emit( - if (data != null) GutterUpdateEvent.DataUpdated(data.toDto(path, gds.scopeDisplayName)) - else GutterUpdateEvent.DataCleared(path) - ) + if (data == null) { + lastSentContents.remove(path) + emit(GutterUpdateEvent.DataCleared(path)) + } else { + val contents = data.baseContent to data.headContent + val includeContents = lastSentContents[path] != contents + if (includeContents) { + lastSentContents[path] = contents + } + emit(GutterUpdateEvent.DataUpdated( + data.toDto(path, gds.scopeDisplayName, includeContents))) + } } } } @@ -103,16 +119,20 @@ class BackendGutterRpcImpl : GutterRpcApi { } } - private fun GutterDataService.GutterFileData.toDto(filePath: String, scopeDisplayName: String) = - GutterFileDataDto( - filePath = filePath, - ranges = ranges.map { GutterRangeDto(it.line1, it.line2, it.vcsLine1, it.vcsLine2) }, - baseContent = baseContent, - headContent = headContent, - scopeRanges = scopeRanges?.map { GutterRangeDto(it.line1, it.line2, it.vcsLine1, it.vcsLine2) }, - scopeDisplayName = scopeDisplayName, - separateGutterRendering = GitScopeSettings.getInstance().isSeparateGutterRendering - ) + private fun GutterDataService.GutterFileData.toDto( + filePath: String, + scopeDisplayName: String, + includeContents: Boolean, + ) = GutterFileDataDto( + filePath = filePath, + ranges = ranges.map { GutterRangeDto(it.line1, it.line2, it.vcsLine1, it.vcsLine2) }, + baseContent = if (includeContents) baseContent else null, + headContent = if (includeContents) headContent else null, + scopeRanges = scopeRanges?.map { GutterRangeDto(it.line1, it.line2, it.vcsLine1, it.vcsLine2) }, + scopeDisplayName = scopeDisplayName, + separateGutterRendering = GitScopeSettings.getInstance().isSeparateGutterRendering, + contentsIncluded = includeContents + ) } class BackendGutterRpcProvider : RemoteApiProvider { diff --git a/frontend/src/main/java/rpc/FrontendGutterListeners.kt b/frontend/src/main/java/rpc/FrontendGutterListeners.kt index 0ec1526..0f5fe9b 100644 --- a/frontend/src/main/java/rpc/FrontendGutterListeners.kt +++ b/frontend/src/main/java/rpc/FrontendGutterListeners.kt @@ -78,10 +78,34 @@ class FrontendGutterSubscriptions( val dto = event.data gds.scopeDisplayName = dto.scopeDisplayName GitScopeSettings.getInstance().isSeparateGutterRendering = dto.separateGutterRendering + + // Contents are omitted when unchanged since the backend's last message for this + // file; the previously published data then carries them. + val baseContent: String + val headContent: String? + if (dto.contentsIncluded) { + baseContent = dto.baseContent + ?: run { + LOG.warn("Gutter update for ${dto.filePath} claims contents but carries none, skipping") + return + } + headContent = dto.headContent + } else { + val cached = gds.getData(dto.filePath) + if (cached == null) { + // Should not happen: the backend only omits contents it already sent within + // this subscription, and clears that memory together with AllCleared. + LOG.warn("Gutter update for ${dto.filePath} references cached contents that are missing, skipping") + return + } + baseContent = cached.baseContent + headContent = cached.headContent + } + val data = GutterDataService.GutterFileData( dto.ranges.map { Range(it.line1, it.line2, it.vcsLine1, it.vcsLine2) }, - dto.baseContent, - dto.headContent, + baseContent, + headContent, dto.scopeRanges?.map { Range(it.line1, it.line2, it.vcsLine1, it.vcsLine2) } ) gds.publish(dto.filePath, data) diff --git a/shared/src/main/java/rpc/GutterTopics.kt b/shared/src/main/java/rpc/GutterTopics.kt index 1dd9ebc..4b7cf58 100644 --- a/shared/src/main/java/rpc/GutterTopics.kt +++ b/shared/src/main/java/rpc/GutterTopics.kt @@ -14,9 +14,17 @@ data class GutterRangeDto( data class GutterFileDataDto( val filePath: String, val ranges: List, - val baseContent: String, + /** + * Full contents of the diff base / HEAD revision. Contents dominate the message size (the + * ranges are a handful of ints), so the backend includes them only when they changed since the + * last message for this file within the same subscription; the frontend caches the previous + * values and reuses them. [contentsIncluded] distinguishes "unchanged, reuse cache" from a + * genuinely null [headContent]. + */ + val baseContent: String? = null, val headContent: String? = null, val scopeRanges: List? = null, val scopeDisplayName: String = "", - val separateGutterRendering: Boolean = false + val separateGutterRendering: Boolean = false, + val contentsIncluded: Boolean = true ) From 6514f9ba158917e17441e0215d4b7744e0527522 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20W=C3=A5llberg?= Date: Sat, 15 Aug 2026 15:26:54 +0200 Subject: [PATCH 04/14] refactor: keep the plugin silent unless gitscope logging is enabled The plugin wrote 67 always-on statements to idea.log, worst on the gutter data path, where a 200-file scope logged around 800 lines every time the scope changed. Demote all 11 INFO calls to DEBUG; demote 11 of the 12 LOG.error calls, since Logger.error feeds the platform's error-report mechanism and raised an "IDE internal error" for conditions the plugin already recovers from, the one kept being VcsTree's fallback failure, which does mean the tool window is broken with no recovery left; and demote 16 WARN calls that fire on normal outcomes such as teardown races and tabs the user cannot move. Debug calls whose argument construction is not free on hot paths are now guarded, and everything stays reachable through the existing single #gitscope switch. With the noise gone, add the debug coverage the refresh pipeline was missing, so a scope that stops picking up filesystem changes can be diagnosed from the log instead of guessed at: MyBulkFileListener now reports the VFS batch size and which project it refreshes, ViewService.collectChanges logs the two early exits that abandon a refresh without scheduling anything, ChangesService logs collections abandoned or interrupted by a newer generation, an empty repository list, cache hits, final counts and unresolvable scopes and now warns from onThrowable, and MyLineStatusTrackerImpl logs updates skipped, superseded or matching no open editor. Steady-state output is still zero lines; 31 WARN and 1 ERROR remain for genuine failures. --- .../compare/ChangesService.java | 36 +++++++++++- .../MyLineStatusTrackerImpl.java | 57 +++++++++++++------ .../java/listener/MyBulkFileListener.java | 24 ++++++-- .../listener/MyDynamicPluginListener.java | 2 +- .../java/listener/MyTabContentListener.java | 2 +- .../java/service/ChangeNavigationService.java | 4 +- .../src/main/java/service/ViewService.java | 6 +- .../java/state/WindowPositionTracker.java | 4 +- .../toolwindow/actions/TabMoveActions.java | 14 ++--- .../java/toolwindow/elements/VcsTree.java | 6 +- .../frontend/GutterRenderingService.java | 29 ++++++---- .../implementation/gutter/ScopeDiffViewer.kt | 4 +- .../gutter/ScopeGutterHighlighterManager.kt | 14 ++--- .../gutter/ScopeGutterPopupPanel.kt | 6 +- .../main/java/service/GutterDataService.java | 17 +++--- 15 files changed, 154 insertions(+), 71 deletions(-) diff --git a/backend/src/main/java/implementation/compare/ChangesService.java b/backend/src/main/java/implementation/compare/ChangesService.java index 524aa6b..6e25b55 100644 --- a/backend/src/main/java/implementation/compare/ChangesService.java +++ b/backend/src/main/java/implementation/compare/ChangesService.java @@ -101,8 +101,13 @@ public void collectChangesWithCallback(TargetBranchMap targetBranchByRepo, Consu public void run(@NotNull ProgressIndicator indicator) { currentIndicator.set(indicator); try { - // Early exit if disposing or superseded by a newer collection request + // Early exit if disposing or superseded by a newer collection request. + // Nothing is applied and no callback runs, so this is a silent no-update: worth a + // line when tracking down a scope that stopped refreshing. if (disposing.get() || indicator.isCanceled() || collectionGeneration.get() != gen) { + LOG.debug("Collection " + gen + " abandoned before start (disposing=" + disposing.get() + + ", cancelled=" + indicator.isCanceled() + + ", latestGeneration=" + collectionGeneration.get() + ")"); return; } @@ -112,6 +117,11 @@ public void run(@NotNull ProgressIndicator indicator) { List errorRepos = new ArrayList<>(); Collection repositories = currentGitService.getRepositories(); + if (repositories.isEmpty()) { + // Happens before the VCS mapping is ready; the scope stays empty until some + // later event triggers another collection. + LOG.debug("Collection " + gen + ": no git repositories registered yet"); + } // Clear cache if checkFs is true (force fresh fetch) if (checkFs) { @@ -119,7 +129,12 @@ public void run(@NotNull ProgressIndicator indicator) { } repositories.forEach(repo -> { - if (indicator.isCanceled() || collectionGeneration.get() != gen) return; + if (indicator.isCanceled() || collectionGeneration.get() != gen) { + LOG.debug("Collection " + gen + " interrupted at " + repo.getRoot().getPath() + + " (cancelled=" + indicator.isCanceled() + + ", latestGeneration=" + collectionGeneration.get() + ")"); + return; + } try { String branchToCompare = getBranchToCompare(targetBranchByRepo, repo); @@ -129,8 +144,10 @@ public void run(@NotNull ProgressIndicator indicator) { RepoChangesResult repoResult; if (!checkFs && changesCache.containsKey(cacheKey)) { - // Use cached result (includes merged, scope, and local changes) + // Use cached result (includes merged, scope, and local changes). + // A cache hit means the filesystem was NOT re-read for this repository. repoResult = changesCache.get(cacheKey); + LOG.debug("Collection " + gen + ": cache hit for " + cacheKey); } else { // Fetch fresh changes repoResult = doCollectChanges(currentProject, repo, branchToCompare); @@ -185,8 +202,15 @@ public void run(@NotNull ProgressIndicator indicator) { // Return ERROR_STATE only if ALL repositories failed (e.g. commit hash not found in any repo). // Individual repo failures are expected in multi-repo setups where a commit exists in only one repo. if (!errorRepos.isEmpty() && errorRepos.size() == repositories.size()) { + LOG.debug("Collection " + gen + ": all " + errorRepos.size() + + " repositories failed -> ERROR_STATE"); result = new ChangesResult(ERROR_STATE, new ArrayList<>(), new ArrayList<>()); } else { + if (LOG.isDebugEnabled()) { + LOG.debug("Collection " + gen + " finished: merged=" + _changes.size() + + ", scope=" + _scopeChanges.size() + ", local=" + _localChanges.size() + + ", failedRepos=" + errorRepos.size()); + } result = new ChangesResult(_changes, _scopeChanges, _localChanges); } } finally { @@ -201,12 +225,16 @@ public void onSuccess() { // Double-check the project is still valid if (!currentProject.isDisposed() && callBack != null && this.result != null) { callBack.accept(this.result); + } else if (this.result == null) { + // The run() above returned early; no update reaches the model from here. + LOG.debug("Collection " + gen + " produced no result, nothing applied"); } }, ModalityState.defaultModalityState(), __ -> disposing.get()); } @Override public void onThrowable(@NotNull Throwable error) { + LOG.warn("Change collection " + gen + " failed, scope shows an error state", error); ApplicationManager.getApplication().invokeLater(() -> { if (!currentProject.isDisposed() && callBack != null) { callBack.accept(new ChangesResult(ERROR_STATE, new ArrayList<>(), new ArrayList<>())); @@ -377,6 +405,8 @@ public RepoChangesResult doCollectChanges(Project project, GitRepository repo, S } else { // We do not have a valid GitReference => return ERROR_STATE + LOG.debug("ChangesService - Repository: " + repoPath + ", Scope: " + scopeRef + + " could not be resolved to a revision -> ERROR_STATE"); return new RepoChangesResult(ERROR_STATE, new ArrayList<>(), new ArrayList<>()); } diff --git a/backend/src/main/java/implementation/lineStatusTracker/MyLineStatusTrackerImpl.java b/backend/src/main/java/implementation/lineStatusTracker/MyLineStatusTrackerImpl.java index b8a540e..64c85df 100644 --- a/backend/src/main/java/implementation/lineStatusTracker/MyLineStatusTrackerImpl.java +++ b/backend/src/main/java/implementation/lineStatusTracker/MyLineStatusTrackerImpl.java @@ -89,13 +89,21 @@ public void fileClosed(@NotNull FileEditorManager source, @NotNull VirtualFile f * Computes ranges on background threads and publishes results to GutterDataService. */ public void update(Map scopeChangesMap, Map localChangesMap) { - if (scopeChangesMap == null || disposing.get()) return; + if (scopeChangesMap == null || disposing.get()) { + LOG.debug("Gutter update skipped (scopeChangesMap=" + (scopeChangesMap == null ? "null" : "present") + + ", disposing=" + disposing.get() + ")"); + return; + } final DisposalToken token = this.disposalToken; final long gen = updateGeneration.incrementAndGet(); updateExecutor.execute(() -> { - if (token.disposed || updateGeneration.get() != gen) return; + if (token.disposed || updateGeneration.get() != gen) { + LOG.debug("Gutter update " + gen + " superseded before start (latest=" + + updateGeneration.get() + ")"); + return; + } Editor[] editors = EditorFactory.getInstance().getAllEditors(); @@ -111,7 +119,13 @@ public void update(Map scopeChangesMap, Map loca } } - if (editorsToUpdate.isEmpty()) return; + if (editorsToUpdate.isEmpty()) { + // No open editor matches the scope, so nothing is published and the gutter keeps + // whatever it last showed. + LOG.debug("Gutter update " + gen + ": none of the " + editors.length + + " open editor(s) are in scope (" + scopeChangesMap.size() + " changed file(s))"); + return; + } Map updates = new ConcurrentHashMap<>(); CountDownLatch latch = new CountDownLatch(editorsToUpdate.size()); @@ -184,15 +198,17 @@ private UpdateInfo prepareUpdateForEditor(Editor editor, Map sco String currentContent; if (changeForFile != null && changeForFile.getBeforeRevision() != null) { - LOG.debug("MyLineStatusTrackerImpl - File: " + filePath + ", beforeRevision: " + - (changeForFile.getBeforeRevision() != null ? changeForFile.getBeforeRevision().getRevisionNumber() : "null") + - ", afterRevision: " + - (changeForFile.getAfterRevision() != null ? changeForFile.getAfterRevision().getRevisionNumber() : "null")); + if (LOG.isDebugEnabled()) { + LOG.debug("MyLineStatusTrackerImpl - File: " + filePath + ", beforeRevision: " + + changeForFile.getBeforeRevision().getRevisionNumber() + + ", afterRevision: " + + (changeForFile.getAfterRevision() != null ? changeForFile.getAfterRevision().getRevisionNumber() : "null")); + } try { baseContent = changeForFile.getBeforeRevision().getContent(); } catch (VcsException e) { - LOG.warn("Error getting content for revision: " + filePath, e); + LOG.debug("Error getting content for revision: " + filePath, e); baseContent = null; } @@ -213,10 +229,13 @@ private UpdateInfo prepareUpdateForEditor(Editor editor, Map sco String normalizedBase = StringUtil.convertLineSeparators(baseContent); String normalizedCurrent = StringUtil.convertLineSeparators(currentContent); - LOG.debug("MyLineStatusTrackerImpl - File: " + filePath + - ", normalizedBase lines: " + normalizedBase.split("\n").length + - ", normalizedCurrent lines: " + normalizedCurrent.split("\n").length + - ", hasLocalChanges: " + hasLocalChanges); + if (LOG.isDebugEnabled()) { + // split() on whole file contents: only pay for it when the log is actually on + LOG.debug("MyLineStatusTrackerImpl - File: " + filePath + + ", normalizedBase lines: " + normalizedBase.split("\n").length + + ", normalizedCurrent lines: " + normalizedCurrent.split("\n").length + + ", hasLocalChanges: " + hasLocalChanges); + } String headContent = null; if (hasLocalChanges) { @@ -228,7 +247,7 @@ private UpdateInfo prepareUpdateForEditor(Editor editor, Map sco headContent = StringUtil.convertLineSeparators(headContent); } } catch (VcsException e) { - LOG.warn("MyLineStatusTrackerImpl - Error caching HEAD content: " + e.getMessage()); + LOG.debug("MyLineStatusTrackerImpl - Error caching HEAD content: " + e.getMessage()); } } } @@ -248,13 +267,15 @@ private UpdateInfo prepareUpdateForEditor(Editor editor, Map sco ranges = RangesBuilder.INSTANCE.createRanges(normalizedCurrent, normalizedBase); } - LOG.debug("MyLineStatusTrackerImpl - File: " + filePath + ", final ranges: " + ranges.size()); - for (Range range : ranges) { - LOG.debug("MyLineStatusTrackerImpl - Range: line1=" + range.getLine1() + ", line2=" + range.getLine2() + - ", vcsLine1=" + range.getVcsLine1() + ", vcsLine2=" + range.getVcsLine2() + ", type=" + range.getType()); + if (LOG.isDebugEnabled()) { + LOG.debug("MyLineStatusTrackerImpl - File: " + filePath + ", final ranges: " + ranges.size()); + for (Range range : ranges) { + LOG.debug("MyLineStatusTrackerImpl - Range: line1=" + range.getLine1() + ", line2=" + range.getLine2() + + ", vcsLine1=" + range.getVcsLine1() + ", vcsLine2=" + range.getVcsLine2() + ", type=" + range.getType()); + } } } catch (Exception e) { - LOG.error("Error precomputing ranges for: " + filePath, e); + LOG.warn("Error precomputing ranges for: " + filePath, e); ranges = Collections.emptyList(); } diff --git a/backend/src/main/java/listener/MyBulkFileListener.java b/backend/src/main/java/listener/MyBulkFileListener.java index a6c5df7..01a3d6b 100644 --- a/backend/src/main/java/listener/MyBulkFileListener.java +++ b/backend/src/main/java/listener/MyBulkFileListener.java @@ -1,16 +1,25 @@ package listener; +import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; import com.intellij.openapi.project.ProjectManager; import com.intellij.openapi.vfs.newvfs.BulkFileListener; import com.intellij.openapi.vfs.newvfs.events.VFileEvent; import org.jetbrains.annotations.NotNull; import service.ViewService; +import system.Defs; import java.util.List; +/** + * Entry point of the refresh pipeline for filesystem changes. When the scope appears "stuck" this + * is the first thing to check: if no line is logged here the event never reached the plugin, and + * everything downstream is irrelevant. + */ public class MyBulkFileListener implements BulkFileListener { + private static final Logger LOG = Defs.getLogger(MyBulkFileListener.class); + @Override public void after(@NotNull List events) { if (events.isEmpty()) return; @@ -20,10 +29,17 @@ public void after(@NotNull List events) { if (project.isDisposed()) continue; ViewService viewService = project.getService(ViewService.class); - if (viewService != null) { - // TODO: collectChanges: bulk file event (disabled) - viewService.collectChanges(true); + if (viewService == null) { + LOG.debug("VFS batch of " + events.size() + " event(s): no ViewService for project " + + project.getName() + ", nothing refreshed"); + continue; + } + if (LOG.isDebugEnabled()) { + LOG.debug("VFS batch of " + events.size() + " event(s) -> collectChanges for project " + + project.getName() + ", first=" + events.get(0).getPath()); } + // TODO: collectChanges: bulk file event (disabled) + viewService.collectChanges(true); } } -} \ No newline at end of file +} diff --git a/backend/src/main/java/listener/MyDynamicPluginListener.java b/backend/src/main/java/listener/MyDynamicPluginListener.java index 43688fe..57f5a78 100644 --- a/backend/src/main/java/listener/MyDynamicPluginListener.java +++ b/backend/src/main/java/listener/MyDynamicPluginListener.java @@ -44,7 +44,7 @@ public void beforePluginUnload(@NotNull IdeaPluginDescriptor pluginDescriptor, b changesService.clearCache(); } } catch (Exception e) { - LOG.error("Error preparing project for plugin unload: " + project.getName(), e); + LOG.warn("Error preparing project for plugin unload: " + project.getName(), e); } } } diff --git a/backend/src/main/java/listener/MyTabContentListener.java b/backend/src/main/java/listener/MyTabContentListener.java index 0424382..41d4c13 100644 --- a/backend/src/main/java/listener/MyTabContentListener.java +++ b/backend/src/main/java/listener/MyTabContentListener.java @@ -73,7 +73,7 @@ public void selectionChanged(@NotNull ContentManagerEvent event) { vcsTree.onTabSwitched(); } } catch (Exception e) { - LOG.error("MyTabContentListener: Error notifying VcsTree about tab switch: " + e.getMessage()); + LOG.warn("MyTabContentListener: Error notifying VcsTree about tab switch: " + e.getMessage()); } }); } diff --git a/backend/src/main/java/service/ChangeNavigationService.java b/backend/src/main/java/service/ChangeNavigationService.java index d067cc9..7b1118c 100644 --- a/backend/src/main/java/service/ChangeNavigationService.java +++ b/backend/src/main/java/service/ChangeNavigationService.java @@ -341,7 +341,7 @@ private List computeRanges(String path, @Nullable Change change, String n try { baseContent = change.getBeforeRevision().getContent(); } catch (VcsException e) { - LOG.warn("ChangeNavigation: error getting base content for " + path, e); + LOG.debug("ChangeNavigation: error getting base content for " + path, e); return Collections.emptyList(); } if (baseContent == null) return Collections.emptyList(); @@ -350,7 +350,7 @@ private List computeRanges(String path, @Nullable Change change, String n try { return RangesBuilder.INSTANCE.createRanges(normalizedCurrent, normalizedBase); } catch (Exception e) { - LOG.warn("ChangeNavigation: error computing ranges for " + path, e); + LOG.debug("ChangeNavigation: error computing ranges for " + path, e); return Collections.emptyList(); } } diff --git a/backend/src/main/java/service/ViewService.java b/backend/src/main/java/service/ViewService.java index a866ac7..487a586 100644 --- a/backend/src/main/java/service/ViewService.java +++ b/backend/src/main/java/service/ViewService.java @@ -656,6 +656,7 @@ private void ensureHeadTabInitializedAsync(MyModel model, Runnable onComplete) { public CompletableFuture collectChanges(MyModel model, boolean checkFs) { CompletableFuture done = new CompletableFuture<>(); if (model == null) { + LOG.debug("collectChanges skipped: no current model"); done.complete(null); return done; } @@ -664,6 +665,9 @@ public CompletableFuture collectChanges(MyModel model, boolean checkFs) { ensureHeadTabInitializedAsync(model, () -> { TargetBranchMap targetBranchMap = model.getTargetBranchMap(); if (targetBranchMap == null) { + // Repositories not registered yet, or the tab has no target branch: nothing can be + // collected and no later event necessarily retries, so the scope stays as it was. + LOG.debug("collectChanges skipped for tab '" + model.getDisplayName() + "': no target branch map"); done.complete(null); return; } @@ -863,7 +867,7 @@ private void rebuildCollectionFromTabOrder() { if (model != null && !model.isHeadTab()) { newCollection.add(model); } else { - LOG.warn("Model not found for tab at index " + i + ": " + content.getTabName()); + LOG.debug("Model not found for tab at index " + i + ": " + content.getTabName()); } } } diff --git a/backend/src/main/java/state/WindowPositionTracker.java b/backend/src/main/java/state/WindowPositionTracker.java index 42f77a6..c2f12c2 100644 --- a/backend/src/main/java/state/WindowPositionTracker.java +++ b/backend/src/main/java/state/WindowPositionTracker.java @@ -156,7 +156,7 @@ public void attachScrollListeners(Component component) { if (LOG.isDebugEnabled()) { LOG.debug("Error attaching scroll listeners for tab " + currentTabId + ": " + e.getMessage()); } - LOG.warn("Error attaching scroll listeners", e); + LOG.debug("Error attaching scroll listeners", e); } }); } @@ -237,7 +237,7 @@ else if (hasUserActivity) { if (LOG.isDebugEnabled()) { LOG.debug("Error attaching scroll listeners" + logPrefix + " for tab " + currentTabId + ": " + e.getMessage()); } - LOG.warn("Error attaching scroll listeners", e); + LOG.debug("Error attaching scroll listeners", e); } } diff --git a/backend/src/main/java/toolwindow/actions/TabMoveActions.java b/backend/src/main/java/toolwindow/actions/TabMoveActions.java index e501484..41dec53 100644 --- a/backend/src/main/java/toolwindow/actions/TabMoveActions.java +++ b/backend/src/main/java/toolwindow/actions/TabMoveActions.java @@ -232,33 +232,33 @@ public void update(@NotNull AnActionEvent e) { private static void moveTab(Project project, ContentManager contentManager, Content content, int oldIndex, int newIndex) { ViewService viewService = null; try { - LOG.info("Moving tab from index " + oldIndex + " to " + newIndex); + LOG.debug("Moving tab from index " + oldIndex + " to " + newIndex); // Additional validation to prevent moving + tab or moving to + tab position int lastIndex = contentManager.getContentCount() - 1; // Cannot move HEAD tab (index 0) if (oldIndex == 0) { - LOG.warn("Cannot move HEAD tab"); + LOG.debug("Cannot move HEAD tab"); return; } // Cannot move + tab (last index) if (oldIndex == lastIndex || PLUS_TAB_LABEL.equals(content.getTabName())) { - LOG.warn("Cannot move + tab"); + LOG.debug("Cannot move + tab"); return; } // Cannot move to position 0 (before HEAD) or to last position (where + tab is) if (newIndex < 1 || newIndex >= lastIndex) { - LOG.warn("Invalid target index: " + newIndex); + LOG.debug("Invalid target index: " + newIndex); return; } // Verify the + tab is still at the last position Content lastContent = contentManager.getContent(lastIndex); if (lastContent == null || !PLUS_TAB_LABEL.equals(lastContent.getTabName())) { - LOG.error("+ tab is not at expected position!"); + LOG.warn("+ tab is not at expected position!"); return; } @@ -282,9 +282,9 @@ private static void moveTab(Project project, ContentManager contentManager, Cont viewService.onTabReordered(oldIndex, newIndex); } - LOG.info("Tab moved successfully"); + LOG.debug("Tab moved successfully"); } catch (Exception e) { - LOG.error("Error moving tab: " + e.getMessage(), e); + LOG.warn("Error moving tab: " + e.getMessage(), e); } finally { // Always clear the flag if (viewService != null) { diff --git a/backend/src/main/java/toolwindow/elements/VcsTree.java b/backend/src/main/java/toolwindow/elements/VcsTree.java index 3b51193..5ec7d71 100644 --- a/backend/src/main/java/toolwindow/elements/VcsTree.java +++ b/backend/src/main/java/toolwindow/elements/VcsTree.java @@ -89,7 +89,7 @@ public void onTabSwitched() { positionTracker.setScrollPositionRestored(true); } } catch (Exception e) { - LOG.error("Error re-attaching scroll listeners after tab switch", e); + LOG.debug("Error re-attaching scroll listeners after tab switch", e); positionTracker.setScrollPositionRestored(true); } }); @@ -124,7 +124,7 @@ private String getCurrentTabId() { return project.getName() + "_tab_" + tabIndex; } } catch (Exception e) { - LOG.warn("Failed to get current tab ID", e); + LOG.debug("Failed to get current tab ID", e); } return project.getName() + "_default_tab"; } @@ -388,7 +388,7 @@ private void setComponent(Component component) { }); } catch (Exception e) { - LOG.error("Error updating VcsTree component", e); + LOG.warn("Error updating VcsTree component", e); try { this.removeAll(); this.add(component, BorderLayout.CENTER); diff --git a/frontend/src/main/java/gitscope/frontend/GutterRenderingService.java b/frontend/src/main/java/gitscope/frontend/GutterRenderingService.java index 14717e0..aa96df6 100644 --- a/frontend/src/main/java/gitscope/frontend/GutterRenderingService.java +++ b/frontend/src/main/java/gitscope/frontend/GutterRenderingService.java @@ -61,7 +61,7 @@ private static final class RendererInfo { public GutterRenderingService(Project project) { this.project = project; this.gutterDataService = project.getService(GutterDataService.class); - LOG.info("GutterRenderingService created, registering as listener on GutterDataService"); + LOG.debug("GutterRenderingService created, registering as listener on GutterDataService"); this.gutterDataService.addListener(this); this.messageBusConnection = project.getMessageBus().connect(); @@ -102,7 +102,9 @@ public void fileClosed(@NotNull FileEditorManager source, @NotNull VirtualFile f @Override public void onDataUpdated(@NotNull String filePath, @NotNull GutterDataService.GutterFileData data) { if (disposed.get()) return; - LOG.info("GutterRenderingService.onDataUpdated: file=" + filePath + ", ranges=" + data.ranges.size()); + if (LOG.isDebugEnabled()) { + LOG.debug("GutterRenderingService.onDataUpdated: file=" + filePath + ", ranges=" + data.ranges.size()); + } ApplicationManager.getApplication().invokeLater(() -> { if (disposed.get()) return; @@ -114,14 +116,15 @@ public void onDataUpdated(@NotNull String filePath, @NotNull GutterDataService.G Document doc = editor.getDocument(); VirtualFile file = FileDocumentManager.getInstance().getFile(doc); if (file != null && file.getPath().equals(filePath)) { - LOG.info("GutterRenderingService: found editor for " + filePath + ", updating renderer"); + LOG.debug("GutterRenderingService: found editor for " + filePath + ", updating renderer"); updateRenderer(doc, file, data); found = true; break; } } if (!found) { - LOG.info("GutterRenderingService: NO editor found for " + filePath); + // Routine: data is published for every changed file, most of which are not open. + LOG.debug("GutterRenderingService: NO editor found for " + filePath); } }, ModalityState.defaultModalityState(), __ -> disposed.get()); } @@ -160,7 +163,7 @@ private synchronized void updateRenderer(@NotNull Document document, @NotNull Vi RendererInfo info = renderers.get(document); if (info == null) { - LOG.info("GutterRenderingService.updateRenderer: CREATING new renderer for " + file.getPath()); + LOG.debug("GutterRenderingService.updateRenderer: CREATING new renderer for " + file.getPath()); ScopeLineStatusMarkerRenderer renderer = new ScopeLineStatusMarkerRenderer( project, document, file, this); info = new RendererInfo(renderer, data.baseContent); @@ -177,7 +180,9 @@ private synchronized void updateRenderer(@NotNull Document document, @NotNull Vi info.scopeRanges = data.scopeRanges; info.renderer.setVcsBaseContent(data.baseContent); info.renderer.updateRanges(data.ranges); - LOG.info("GutterRenderingService.updateRenderer: applied " + data.ranges.size() + " ranges to " + file.getPath()); + if (LOG.isDebugEnabled()) { + LOG.debug("GutterRenderingService.updateRenderer: applied " + data.ranges.size() + " ranges to " + file.getPath()); + } } private DocumentListener createDocumentListener(@NotNull Document document, @NotNull RendererInfo info) { @@ -229,7 +234,9 @@ private void recalculateRangesAsync(@NotNull Document document, @NotNull Rendere } }, ModalityState.defaultModalityState()); } catch (Exception e) { - LOG.error("Error recalculating ranges", e); + // Runs on every document change; a failure here costs one stale repaint, and the next + // keystroke retries. Not worth an error report. + LOG.debug("Error recalculating ranges", e); } } @@ -342,16 +349,18 @@ private void emitScopeSegment(List result, } } + // Teardown races with the platform disposing editors and documents underneath us, so failures + // here are expected and already recovered from by dropping the renderer. private synchronized void releaseRenderer(@NotNull Document document) { RendererInfo info = renderers.remove(document); if (info != null) { if (info.documentListener != null) { try { document.removeDocumentListener(info.documentListener); } - catch (Exception e) { LOG.warn("Error removing document listener", e); } + catch (Exception e) { LOG.debug("Error removing document listener", e); } } if (info.renderer != null) { try { info.renderer.dispose(); } - catch (Exception e) { LOG.warn("Error disposing renderer", e); } + catch (Exception e) { LOG.debug("Error disposing renderer", e); } } } } @@ -361,7 +370,7 @@ private void releaseAllRenderers() { RendererInfo info = entry.getValue(); if (info != null && info.renderer != null) { try { info.renderer.dispose(); } - catch (Exception e) { LOG.warn("Error disposing renderer", e); } + catch (Exception e) { LOG.debug("Error disposing renderer", e); } } } renderers.clear(); diff --git a/frontend/src/main/java/implementation/gutter/ScopeDiffViewer.kt b/frontend/src/main/java/implementation/gutter/ScopeDiffViewer.kt index f8f15cf..856ff50 100644 --- a/frontend/src/main/java/implementation/gutter/ScopeDiffViewer.kt +++ b/frontend/src/main/java/implementation/gutter/ScopeDiffViewer.kt @@ -64,7 +64,7 @@ class ScopeDiffViewer( // Show diff in a dialog DiffManager.getInstance().showDiff(project, request) } catch (e: Exception) { - LOG.error("Error showing diff for range", e) + LOG.warn("Error showing diff for range", e) } } @@ -83,7 +83,7 @@ class ScopeDiffViewer( fun getVcsContentForRange(range: Range, includeContext: Boolean = true): String { val baseContent = vcsBaseContent if (baseContent == null) { - if (range.type == Range.DELETED || range.type == Range.MODIFIED) LOG.warn("VCS base content not available") + if (range.type == Range.DELETED || range.type == Range.MODIFIED) LOG.debug("VCS base content not available") return "" } val lines = baseContent.split("\n") diff --git a/frontend/src/main/java/implementation/gutter/ScopeGutterHighlighterManager.kt b/frontend/src/main/java/implementation/gutter/ScopeGutterHighlighterManager.kt index af94e30..518693b 100644 --- a/frontend/src/main/java/implementation/gutter/ScopeGutterHighlighterManager.kt +++ b/frontend/src/main/java/implementation/gutter/ScopeGutterHighlighterManager.kt @@ -60,7 +60,7 @@ internal class ScopeGutterHighlighterManager( try { if (highlighter.isValid) markupModel?.removeHighlighter(highlighter) } catch (e: Exception) { - LOG.warn("Error removing highlighter", e) + LOG.debug("Error removing highlighter", e) } } gutterHighlighter = null @@ -68,7 +68,7 @@ internal class ScopeGutterHighlighterManager( try { if (highlighter.isValid) markupModel?.removeHighlighter(highlighter) } catch (e: Exception) { - LOG.warn("Error removing error stripe highlighter", e) + LOG.debug("Error removing error stripe highlighter", e) } } errorStripeHighlighters.clear() @@ -81,7 +81,7 @@ internal class ScopeGutterHighlighterManager( if (editor is EditorEx) editor.gutterComponentEx.repaint() } } catch (e: Exception) { - LOG.warn("Error repainting gutter", e) + LOG.debug("Error repainting gutter", e) } } @@ -94,7 +94,7 @@ internal class ScopeGutterHighlighterManager( ?.removeHighlighter(old) } } catch (e: Exception) { - LOG.warn("Error removing old highlighter", e) + LOG.debug("Error removing old highlighter", e) } } try { @@ -109,7 +109,7 @@ internal class ScopeGutterHighlighterManager( rh.editorFilter = MarkupEditorFilterFactory.createIsNotDiffFilter() } } catch (e: Exception) { - LOG.error("Error creating gutter highlighter", e) + LOG.warn("Error creating gutter highlighter", e) } } @@ -160,7 +160,7 @@ internal class ScopeGutterHighlighterManager( try { if (highlighter.isValid) markupModel.removeHighlighter(highlighter) } catch (e: Exception) { - LOG.warn("Error removing error stripe highlighter", e) + LOG.debug("Error removing error stripe highlighter", e) } } errorStripeHighlighters.clear() @@ -190,7 +190,7 @@ internal class ScopeGutterHighlighterManager( } errorStripeHighlighters.add(highlighter) } catch (e: Exception) { - LOG.warn("Error creating error stripe highlighter for range $range", e) + LOG.debug("Error creating error stripe highlighter for range $range", e) } } } diff --git a/frontend/src/main/java/implementation/gutter/ScopeGutterPopupPanel.kt b/frontend/src/main/java/implementation/gutter/ScopeGutterPopupPanel.kt index b718555..010b08d 100644 --- a/frontend/src/main/java/implementation/gutter/ScopeGutterPopupPanel.kt +++ b/frontend/src/main/java/implementation/gutter/ScopeGutterPopupPanel.kt @@ -341,7 +341,7 @@ internal class ScopeGutterPopupPanel( } catch (e: com.intellij.openapi.progress.ProcessCanceledException) { throw e } catch (e: Exception) { - LOG.warn("Error computing word diff for editor highlights", e) + LOG.debug("Error computing word diff for editor highlights", e) } } @@ -445,7 +445,7 @@ internal class ScopeGutterPopupPanel( } catch (e: com.intellij.openapi.progress.ProcessCanceledException) { throw e } catch (e: Exception) { - LOG.warn("Error applying word diff highlighting", e) + LOG.debug("Error applying word diff highlighting", e) } } } @@ -546,7 +546,7 @@ internal class ScopeGutterPopupPanel( } LOG.debug("Successfully rolled back range at line ${range.line1}") } catch (e: Exception) { - LOG.error("Error rolling back range", e) + LOG.warn("Error rolling back range", e) } } } diff --git a/shared/src/main/java/service/GutterDataService.java b/shared/src/main/java/service/GutterDataService.java index f0412b5..9b97ed9 100644 --- a/shared/src/main/java/service/GutterDataService.java +++ b/shared/src/main/java/service/GutterDataService.java @@ -78,17 +78,19 @@ public GutterDataService(Project project) { public void publish(@NotNull String filePath, @NotNull GutterFileData data) { fileDataMap.put(filePath, data); - LOG.info("GutterDataService.publish: file=" + filePath + - ", ranges=" + data.ranges.size() + - ", listeners=" + listeners.size() + - ", hasBaseContent=" + (data.baseContent != null && !data.baseContent.isEmpty()) + - ", hasHeadContent=" + (data.headContent != null)); + if (LOG.isDebugEnabled()) { + LOG.debug("GutterDataService.publish: file=" + filePath + + ", ranges=" + data.ranges.size() + + ", listeners=" + listeners.size() + + ", hasBaseContent=" + (data.baseContent != null && !data.baseContent.isEmpty()) + + ", hasHeadContent=" + (data.headContent != null)); + } for (Listener l : listeners) l.onDataUpdated(filePath, data); } public void clear(@NotNull String filePath) { fileDataMap.remove(filePath); - LOG.info("GutterDataService.clear: file=" + filePath); + LOG.debug("GutterDataService.clear: file=" + filePath); for (Listener l : listeners) l.onDataCleared(filePath); } @@ -103,6 +105,7 @@ public void clearAll() { * without waiting for the next scope/document update. */ public void republishAll() { + LOG.debug("GutterDataService.republishAll: files=" + fileDataMap.size() + ", listeners=" + listeners.size()); for (Map.Entry e : fileDataMap.entrySet()) { for (Listener l : listeners) l.onDataUpdated(e.getKey(), e.getValue()); } @@ -126,7 +129,7 @@ public void setScopeDisplayName(@NotNull String name) { public void addListener(@NotNull Listener listener) { listeners.add(listener); - LOG.info("GutterDataService.addListener: " + listener.getClass().getSimpleName() + ", total=" + listeners.size()); + LOG.debug("GutterDataService.addListener: " + listener.getClass().getSimpleName() + ", total=" + listeners.size()); } public void removeListener(@NotNull Listener listener) { From e340399db503c42a250a9e044ae425bcfc9fcdbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20W=C3=A5llberg?= Date: Sat, 15 Aug 2026 19:56:14 +0200 Subject: [PATCH 05/14] fix: stop file-open events from discarding fresh scope collections Three compounding races left the Git Scope window showing pre-git-operation state, with conflict files staying red after a resolved rebase as the reported symptom. The window is a few hundred milliseconds wide and needs a file to be opened inside it, which is why it reproduced on real branches but never in minimal tests: a collection during the conflict state caches red results, then finishing the rebase schedules a fresh collection, but opening any file bumped the apply generation, causing that fresh collection to abandon before its run() reached the cache clear, and scheduled a cache-permitted collection that served the conflict-era entries under the now-current generation. One fix per link in the chain. fileOpened no longer bumps the apply generation, since opening a file changes no scope input and the bump only discarded whichever fresh collection was in flight; the call remains as a cache-warmer. The cache is cleared when a fresh collection is scheduled rather than inside its run(), so pre-operation entries cannot outlive the operation even if that collection is abandoned. And the collection callback is now always invoked exactly once, with null when superseded or cancelled, so chained UI work such as the file-colors refresh after a tab switch is never silently lost. Refs #78 --- .../compare/ChangesService.java | 44 ++++++++++++++----- .../listener/MyFileEditorManagerListener.java | 7 ++- .../src/main/java/service/ViewService.java | 9 ++++ 3 files changed, 48 insertions(+), 12 deletions(-) diff --git a/backend/src/main/java/implementation/compare/ChangesService.java b/backend/src/main/java/implementation/compare/ChangesService.java index 6e25b55..d269b1a 100644 --- a/backend/src/main/java/implementation/compare/ChangesService.java +++ b/backend/src/main/java/implementation/compare/ChangesService.java @@ -87,12 +87,29 @@ private static String getBranchToCompare(TargetBranchMap targetBranchByRepo, Git // Cache for storing changes per repository (stores RepoChangesResult to preserve scope/local separation) private final Map changesCache = new ConcurrentHashMap<>(); + /** + * Collects changes and reports them to {@code callBack}. + * + *

The callback is always invoked exactly once (unless the project is disposed + * first): with the collected result, or with {@code null} when this collection was superseded + * or cancelled by a newer one. Callers chain UI work on the callback, so dropping it — as a + * cancelled {@code Task} used to — silently lost that work (e.g. the file-colors refresh + * after a tab switch). + */ public void collectChangesWithCallback(TargetBranchMap targetBranchByRepo, Consumer callBack, boolean checkFs) { // Capture the current project reference to ensure consistency final Project currentProject = this.project; final GitService currentGitService = this.git; final long gen = collectionGeneration.incrementAndGet(); + // Clear stale results at scheduling time, not inside run(): a task superseded before its + // run() started never reached the clear, so entries cached DURING a git operation + // (e.g. conflict-state changes mid-rebase) survived it and were served to the next + // cache-permitted collection (issue #78). + if (checkFs) { + changesCache.clear(); + } + task = new Task.Backgroundable(currentProject, "Collecting " + Defs.APPLICATION_NAME, true) { private ChangesResult result; @@ -123,11 +140,6 @@ public void run(@NotNull ProgressIndicator indicator) { LOG.debug("Collection " + gen + ": no git repositories registered yet"); } - // Clear cache if checkFs is true (force fresh fetch) - if (checkFs) { - changesCache.clear(); - } - repositories.forEach(repo -> { if (indicator.isCanceled() || collectionGeneration.get() != gen) { LOG.debug("Collection " + gen + " interrupted at " + repo.getRoot().getPath() @@ -223,12 +235,24 @@ public void onSuccess() { // Ensure result is accessed only on the UI thread to update the UI component ApplicationManager.getApplication().invokeLater(() -> { // Double-check the project is still valid - if (!currentProject.isDisposed() && callBack != null && this.result != null) { - callBack.accept(this.result); - } else if (this.result == null) { - // The run() above returned early; no update reaches the model from here. - LOG.debug("Collection " + gen + " produced no result, nothing applied"); + if (currentProject.isDisposed() || callBack == null) return; + if (this.result == null) { + // The run() above returned early (superseded); a newer collection owns the + // model. Complete the callback with null so chained work still runs. + LOG.debug("Collection " + gen + " superseded, completing callback without data"); } + callBack.accept(this.result); + }, ModalityState.defaultModalityState(), __ -> disposing.get()); + } + + @Override + public void onCancel() { + // Queueing a newer collection cancels this one's indicator; the platform then calls + // onCancel instead of onSuccess. The callback chain must still complete. + ApplicationManager.getApplication().invokeLater(() -> { + if (currentProject.isDisposed() || callBack == null) return; + LOG.debug("Collection " + gen + " cancelled, completing callback without data"); + callBack.accept(null); }, ModalityState.defaultModalityState(), __ -> disposing.get()); } diff --git a/backend/src/main/java/listener/MyFileEditorManagerListener.java b/backend/src/main/java/listener/MyFileEditorManagerListener.java index ecf74c2..84d55af 100644 --- a/backend/src/main/java/listener/MyFileEditorManagerListener.java +++ b/backend/src/main/java/listener/MyFileEditorManagerListener.java @@ -17,8 +17,11 @@ public MyFileEditorManagerListener(Project project) { @Override public void fileOpened(@NotNull FileEditorManager source, @NotNull VirtualFile vFile) { - // TODO: collectChanges: File Opened event - viewService.incrementUpdate(); + // Opening a file changes no scope input, so this deliberately does NOT bump the apply + // generation: doing so discarded the result of any in-flight fresh collection (e.g. one + // triggered by a just-finished rebase) and replaced it with this cache-served one -- + // the scope then showed pre-operation state, stuck until the next tab switch (issue #78). + // This call only warms the model when nothing has been collected yet. viewService.collectChanges(false); } } diff --git a/backend/src/main/java/service/ViewService.java b/backend/src/main/java/service/ViewService.java index 487a586..5e0221e 100644 --- a/backend/src/main/java/service/ViewService.java +++ b/backend/src/main/java/service/ViewService.java @@ -689,6 +689,15 @@ private void collectChangesInternal(MyModel model, TargetBranchMap targetBranchM final DisposalToken token = this.disposalToken; changesExecutor.execute(() -> { changesService.collectChangesWithCallback(finalTargetBranchMap, result -> { + if (result == null) { + // Superseded or cancelled by a newer collection, which owns the model now. + // Complete the future anyway so chained UI work (e.g. the file-colors refresh + // after a tab switch) is never silently dropped. + LOG.debug("Collection for generation " + gen + " superseded, completing without apply"); + done.complete(null); + return; + } + // Build maps on background thread to avoid slow file system operations on EDT Map mergedChangesMap = MyModel.buildChangesByPathMap(result.mergedChanges()); Map scopeChangesMap = MyModel.buildChangesByPathMap(result.scopeChanges()); From cf7128118bd3cb41e6c5c47d36b7ed245a1adac3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20W=C3=A5llberg?= Date: Sat, 15 Aug 2026 19:56:56 +0200 Subject: [PATCH 06/14] fix: refresh file colors when a collection changes the scope statuses GitScopeFileStatusProvider answers from the current scope map, but the platform caches those answers until fileStatusesChanged() is called, which only tab switches, boot and the settings dialog did. A collection that changed the scope, such as a file leaving it after a rebase or a status changing after a commit, therefore updated the tool window but left Project-view and editor-tab colors showing the previous scope until the user switched tabs. Refresh the colors when an applied collection materially changed the scope map, compared as path -> FileStatus. The comparison guards the common no-change apply, because the refresh goes through FileStatusManager.fileStatusesChanged(), which can disturb line status trackers and is not free. Refs #78 --- .../src/main/java/service/ViewService.java | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/backend/src/main/java/service/ViewService.java b/backend/src/main/java/service/ViewService.java index 5e0221e..c027c9b 100644 --- a/backend/src/main/java/service/ViewService.java +++ b/backend/src/main/java/service/ViewService.java @@ -708,9 +708,21 @@ private void collectChangesInternal(MyModel model, TargetBranchMap targetBranchM long currentGen = applyGeneration.get(); if (!project.isDisposed() && !token.disposed && currentGen == gen) { LOG.debug("Applying changes for generation " + gen); + Map previousScopeMap = model.getScopeChangesMap(); model.setChangesWithMap(result.mergedChanges(), mergedChangesMap); model.setScopeChangesWithMap(result.scopeChanges(), scopeChangesMap); model.setLocalChangesWithMap(result.localChanges(), localChangesMap); + + // GitScopeFileStatusProvider answers from the scope map, but the + // platform caches its answers until fileStatusesChanged() -- which + // previously only tab switches triggered, so Project-view colors kept + // showing the pre-collection scope (e.g. conflict-era statuses after a + // rebase). Refresh when the statuses materially changed; the guard + // avoids the LST-disturbing refresh on the common no-change apply. + if (model.isActive() && !sameScopeStatuses(previousScopeMap, scopeChangesMap)) { + LOG.debug("Scope statuses changed for generation " + gen + ", refreshing file colors"); + refreshFileColors(); + } } else { LOG.debug("Discarding changes for generation " + gen + " (current generation is " + currentGen + ")"); } @@ -722,6 +734,23 @@ private void collectChangesInternal(MyModel model, TargetBranchMap targetBranchM }); } + /** + * Whether two scope maps would produce the same file colors: same files, same statuses. + * Used to skip the file-status refresh on the common apply where nothing changed. + */ + private static boolean sameScopeStatuses(Map previous, Map current) { + if (previous == current) return true; + if (previous == null || current == null) return false; + if (previous.size() != current.size()) return false; + for (Map.Entry entry : previous.entrySet()) { + Change other = current.get(entry.getKey()); + if (other == null || !entry.getValue().getFileStatus().equals(other.getFileStatus())) { + return false; + } + } + return true; + } + // helper to enqueue UI work strictly after the currently queued collections public void runAfterCurrentChangeCollection(Runnable uiTask) { if (isDisposed) return; From 2574962cbe83d12ff63a333f5cbabeacb881aa66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20W=C3=A5llberg?= Date: Sat, 15 Aug 2026 19:57:51 +0200 Subject: [PATCH 07/14] perf: only refresh projects whose repositories the VFS batch touches MyBulkFileListener re-collected every open project on every VFS batch anywhere: build outputs, caches, files of unrelated projects. Beyond the wasted git work, each spurious collection cancels whichever collection is currently in flight, so busy phases such as a rebase writing dozens of files kept restarting the very collections that were about to deliver fresh state, widening the race windows behind issue #78. Skip projects where no event path lies under any of their git repository roots. Everything under a repository root still triggers, including .git internals, so no existing refresh source is lost; repositories not being registered yet also skips, since there is nothing to collect against until the VCS mapping listener fires. Refs #78 --- .../java/listener/MyBulkFileListener.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/backend/src/main/java/listener/MyBulkFileListener.java b/backend/src/main/java/listener/MyBulkFileListener.java index 01a3d6b..46c2b74 100644 --- a/backend/src/main/java/listener/MyBulkFileListener.java +++ b/backend/src/main/java/listener/MyBulkFileListener.java @@ -5,6 +5,7 @@ import com.intellij.openapi.project.ProjectManager; import com.intellij.openapi.vfs.newvfs.BulkFileListener; import com.intellij.openapi.vfs.newvfs.events.VFileEvent; +import git4idea.repo.GitRepository; import org.jetbrains.annotations.NotNull; import service.ViewService; import system.Defs; @@ -28,6 +29,14 @@ public void after(@NotNull List events) { for (Project project : openProjects) { if (project.isDisposed()) continue; + // Only refresh projects that one of the changed files actually belongs to. Without + // this, every VFS batch anywhere (build outputs, unrelated projects, IDE internals) + // re-collected every open project, and the resulting collection churn repeatedly + // cancelled in-flight collections during busy phases like a rebase. + if (!touchesRepository(project, events)) { + continue; + } + ViewService viewService = project.getService(ViewService.class); if (viewService == null) { LOG.debug("VFS batch of " + events.size() + " event(s): no ViewService for project " @@ -42,4 +51,25 @@ public void after(@NotNull List events) { viewService.collectChanges(true); } } + + /** + * Whether any event path lies under one of the project's git repository roots. Repositories + * not being registered yet also means there is nothing to collect against — the VCS mapping + * listener triggers the initial collection when they arrive. + */ + private static boolean touchesRepository(@NotNull Project project, @NotNull List events) { + List repositories = git4idea.GitUtil.getRepositoryManager(project).getRepositories(); + if (repositories.isEmpty()) return false; + + for (VFileEvent event : events) { + String path = event.getPath(); + for (GitRepository repository : repositories) { + String root = repository.getRoot().getPath(); + if (path.startsWith(root) && (path.length() == root.length() || path.charAt(root.length()) == '/')) { + return true; + } + } + } + return false; + } } From ce94403bba564a138a0e1e1a44d3a2d1f4c508b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20W=C3=A5llberg?= Date: Sat, 15 Aug 2026 21:42:30 +0200 Subject: [PATCH 08/14] fix: restore the tab context menu in split mode Rename Tab, Reset Tab Name and Move Tab Left/Right were all absent from the tool window's tab context menu over Remote Development. The cause was registration, not logic: the actions were declared in gitscope.backend.xml, so they existed only in the host's action registry, while in split mode the tab strip is rendered by the frontend and its context menu is built from the frontend registry. Register the four actions once in the frontend module for both modes -- monolith loads gitscope.frontend too, and UtilRpcApi resolves there as well -- so a single registration serves both modes with no second implementation to keep in step. This also removes three copies of a helper that read ContentTabLabel.myContent via getDeclaredField, which does not search superclasses and so already hid the actions in monolith; ToolWindowContextMenuActionBase replaces it. The tool window, its ContentManager and the scope models are backend state, so the actions decide enablement from what the tab strip shows and delegate over UtilRpcApi to a new TabActionService, which owns the rules and re-validates every request. Reset Tab Name additionally needs to know whether a tab carries a custom name, which is model state the frontend cannot see, so the backend publishes the custom-named tab indices as a StateFlow, republished after every rename, reset and move and once when a frontend subscribes. The frontend mirrors it in a service the action reads synchronously, since update() cannot suspend. Unknown state counts as enabled, so the worst case is an action that runs and finds nothing to reset, never one that is missing. --- .../src/main/java/rpc/BackendUtilRpcImpl.kt | 23 ++ .../src/main/java/rpc/UtilCommandService.kt | 14 + .../main/java/service/TabActionService.java | 214 +++++++++++++ .../toolwindow/actions/RenameTabAction.java | 125 -------- .../actions/ResetTabNameAction.java | 133 -------- .../toolwindow/actions/TabMoveActions.java | 295 ------------------ .../src/main/resources/gitscope.backend.xml | 16 +- .../frontend/actions/TabContextActions.kt | 132 ++++++++ .../main/java/rpc/FrontendTabStateService.kt | 76 +++++ .../src/main/resources/gitscope.frontend.xml | 25 ++ shared/src/main/java/rpc/UtilRpcApi.kt | 32 ++ shared/src/main/java/system/Defs.java | 6 + 12 files changed, 526 insertions(+), 565 deletions(-) create mode 100644 backend/src/main/java/service/TabActionService.java delete mode 100644 backend/src/main/java/toolwindow/actions/RenameTabAction.java delete mode 100644 backend/src/main/java/toolwindow/actions/ResetTabNameAction.java delete mode 100644 backend/src/main/java/toolwindow/actions/TabMoveActions.java create mode 100644 frontend/src/main/java/gitscope/frontend/actions/TabContextActions.kt create mode 100644 frontend/src/main/java/rpc/FrontendTabStateService.kt diff --git a/backend/src/main/java/rpc/BackendUtilRpcImpl.kt b/backend/src/main/java/rpc/BackendUtilRpcImpl.kt index 3fdc170..44674ed 100644 --- a/backend/src/main/java/rpc/BackendUtilRpcImpl.kt +++ b/backend/src/main/java/rpc/BackendUtilRpcImpl.kt @@ -34,6 +34,29 @@ class BackendUtilRpcImpl : UtilRpcApi { val project = projectId.findProjectOrNull() ?: return project.service().showDiff(currentFilePath) } + + override suspend fun renameTab(projectId: ProjectId, tabIndex: Int, newName: String) { + val project = projectId.findProjectOrNull() ?: return + project.service().renameTab(tabIndex, newName) + } + + override suspend fun resetTabName(projectId: ProjectId, tabIndex: Int) { + val project = projectId.findProjectOrNull() ?: return + project.service().resetTabName(tabIndex) + } + + override suspend fun moveTab(projectId: ProjectId, tabIndex: Int, direction: TabMoveDirection) { + val project = projectId.findProjectOrNull() ?: return + project.service().moveTab(tabIndex, direction) + } + + override suspend fun getCustomNamedTabs(projectId: ProjectId): Flow> { + val project = projectId.findProjectOrNull() ?: return emptyFlow() + // Publish the current state before the subscriber starts collecting, so a frontend that + // connects after the tabs were restored still gets the truth rather than the initial empty. + project.service().publishCustomNamedTabs() + return project.service().customNamedTabs + } } class BackendUtilRpcProvider : RemoteApiProvider { diff --git a/backend/src/main/java/rpc/UtilCommandService.kt b/backend/src/main/java/rpc/UtilCommandService.kt index 45faa74..d18a22e 100644 --- a/backend/src/main/java/rpc/UtilCommandService.kt +++ b/backend/src/main/java/rpc/UtilCommandService.kt @@ -2,7 +2,9 @@ package rpc import com.intellij.openapi.components.Service import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.asSharedFlow +import kotlinx.coroutines.flow.asStateFlow import java.util.concurrent.atomic.AtomicReference @Service(Service.Level.PROJECT) @@ -17,6 +19,18 @@ class UtilCommandService { */ private val previewTabEnabled = AtomicReference(null) + /** + * Indices of tabs carrying a custom name. A StateFlow rather than a command, so the frontend can + * read the current value at any time — including right after it subscribes — to decide whether + * "Reset Tab Name" has anything to reset. + */ + private val _customNamedTabs = MutableStateFlow>(emptyList()) + val customNamedTabs = _customNamedTabs.asStateFlow() + + fun setCustomNamedTabs(indices: List) { + _customNamedTabs.value = indices + } + fun selectInProject(filePath: String) { _commands.tryEmit(UtilCommand.SelectInProject(filePath)) } diff --git a/backend/src/main/java/service/TabActionService.java b/backend/src/main/java/service/TabActionService.java new file mode 100644 index 0000000..eef1723 --- /dev/null +++ b/backend/src/main/java/service/TabActionService.java @@ -0,0 +1,214 @@ +package service; + +import com.intellij.openapi.application.ApplicationManager; +import com.intellij.openapi.diagnostic.Logger; +import com.intellij.openapi.project.Project; +import com.intellij.openapi.wm.ToolWindow; +import com.intellij.ui.content.Content; +import com.intellij.ui.content.ContentManager; +import model.MyModel; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import system.Defs; + +import java.util.ArrayList; +import java.util.List; + +/** + * Backend-side tab operations addressed by tab index: rename, reset name, move. + * + *

The tool window, its ContentManager and the scope models are backend state, while the context + * menu actions are registered on the frontend for both modes, so every tab change arrives here over + * {@code UtilRpcApi}. This is the only place the tab rules live. + * + *

All operations validate the index themselves and no-op when it does not address a tab the + * operation applies to: the frontend decides enablement from the tab strip alone and cannot see the + * scope models, so its requests are treated as advisory. + */ +public class TabActionService { + + private static final Logger LOG = Defs.getLogger(TabActionService.class); + + private final Project project; + + public TabActionService(Project project) { + this.project = project; + } + + public void renameTab(int tabIndex, @NotNull String newName) { + if (newName.isEmpty()) return; + runOnEdt(() -> { + ContentManager contentManager = getContentManager(); + if (contentManager == null) return; + + Content content = getRenameableContent(contentManager, tabIndex); + if (content == null) { + LOG.debug("renameTab: index " + tabIndex + " is not a renameable tab"); + return; + } + + content.setDisplayName(newName); + + ViewService viewService = project.getService(ViewService.class); + if (viewService == null) return; + viewService.onTabRenamed(tabIndex, newName); + + MyModel model = getModelForTab(viewService, tabIndex); + if (model != null) { + project.getService(ToolWindowServiceInterface.class).setupTabTooltip(model); + } + publishCustomNamedTabs(); + }); + } + + public void resetTabName(int tabIndex) { + runOnEdt(() -> { + ContentManager contentManager = getContentManager(); + if (contentManager == null) return; + + Content content = getRenameableContent(contentManager, tabIndex); + if (content == null) { + LOG.debug("resetTabName: index " + tabIndex + " is not a renameable tab"); + return; + } + + ViewService viewService = project.getService(ViewService.class); + if (viewService == null) return; + + MyModel model = getModelForTab(viewService, tabIndex); + if (model == null || model.getCustomTabName() == null || model.getCustomTabName().isEmpty()) { + // Expected: the frontend cannot see the models, so it offers the action for every + // renameable tab and relies on this check. + LOG.debug("resetTabName: tab " + tabIndex + " has no custom name"); + return; + } + + // Clearing the custom name restores the branch-based default name. + model.setCustomTabName(null); + viewService.save(); + + TargetBranchService targetBranchService = project.getService(TargetBranchService.class); + targetBranchService.getTargetBranchDisplayAsync(model.getTargetBranchMap(), branchName -> + ApplicationManager.getApplication().invokeLater(() -> { + if (project.isDisposed()) return; + content.setDisplayName(branchName); + content.setDescription(null); + })); + publishCustomNamedTabs(); + }); + } + + public void moveTab(int tabIndex, @NotNull rpc.TabMoveDirection direction) { + runOnEdt(() -> { + ContentManager contentManager = getContentManager(); + if (contentManager == null) return; + + int newIndex = direction == rpc.TabMoveDirection.LEFT ? tabIndex - 1 : tabIndex + 1; + Content content = getMovableContent(contentManager, tabIndex, newIndex); + if (content == null) { + LOG.debug("moveTab: " + tabIndex + " -> " + newIndex + " is not a valid move"); + return; + } + + LOG.debug("Moving tab from index " + tabIndex + " to " + newIndex); + ViewService viewService = project.getService(ViewService.class); + try { + // Set the flag BEFORE moving so the content listener does not treat the + // remove/add pair as a user-initiated tab change. + if (viewService != null) { + viewService.setProcessingTabReorder(true); + } + + contentManager.removeContent(content, false); + contentManager.addContent(content, newIndex); + contentManager.setSelectedContent(content, true); + + if (viewService != null) { + viewService.onTabReordered(tabIndex, newIndex); + } + } finally { + if (viewService != null) { + viewService.setProcessingTabReorder(false); + } + } + // Indices shifted, so the previously published set no longer addresses the same tabs. + publishCustomNamedTabs(); + }); + } + + /** + * Republishes which tabs carry a custom name, so the frontend can disable "Reset Tab Name" where + * there is nothing to reset. Called after every operation that can change the answer, and once + * when a frontend subscribes. + */ + public void publishCustomNamedTabs() { + ApplicationManager.getApplication().invokeLater(() -> { + if (project.isDisposed()) return; + + ContentManager contentManager = getContentManager(); + ViewService viewService = project.getService(ViewService.class); + if (contentManager == null || viewService == null) return; + + List indices = new ArrayList<>(); + for (int index = 1; index < contentManager.getContentCount(); index++) { + if (!isRenameableTab(contentManager, index)) continue; + MyModel model = getModelForTab(viewService, index); + if (model != null && model.getCustomTabName() != null && !model.getCustomTabName().isEmpty()) { + indices.add(index); + } + } + LOG.debug("publishCustomNamedTabs: " + indices); + project.getService(rpc.UtilCommandService.class).setCustomNamedTabs(indices); + }); + } + + // --- rules, shared by every caller --- + + /** Whether the tab at {@code index} may be renamed: not HEAD (index 0) and not the "+" tab. */ + public static boolean isRenameableTab(@NotNull ContentManager contentManager, int index) { + if (index <= 0 || index >= contentManager.getContentCount()) return false; + Content content = contentManager.getContent(index); + return content != null && !Defs.PLUS_TAB_LABEL.equals(content.getTabName()); + } + + /** Whether the tab at {@code index} may move to {@code newIndex} (never onto HEAD or "+"). */ + public static boolean isMovableTab(@NotNull ContentManager contentManager, int index, int newIndex) { + if (!isRenameableTab(contentManager, index)) return false; + int lastIndex = contentManager.getContentCount() - 1; + return newIndex >= 1 && newIndex < lastIndex; + } + + private static @Nullable Content getRenameableContent(@NotNull ContentManager contentManager, int index) { + return isRenameableTab(contentManager, index) ? contentManager.getContent(index) : null; + } + + private static @Nullable Content getMovableContent(@NotNull ContentManager contentManager, int index, int newIndex) { + return isMovableTab(contentManager, index, newIndex) ? contentManager.getContent(index) : null; + } + + // --- helpers --- + + private @Nullable ContentManager getContentManager() { + ToolWindowServiceInterface toolWindowService = project.getService(ToolWindowServiceInterface.class); + if (toolWindowService == null) return null; + ToolWindow toolWindow = toolWindowService.getToolWindow(); + return toolWindow == null ? null : toolWindow.getContentManager(); + } + + private static @Nullable MyModel getModelForTab(@NotNull ViewService viewService, int tabIndex) { + int modelIndex = viewService.getModelIndex(tabIndex); + if (modelIndex < 0 || modelIndex >= viewService.getCollection().size()) return null; + return viewService.getCollection().get(modelIndex); + } + + private void runOnEdt(@NotNull Runnable action) { + ApplicationManager.getApplication().invokeLater(() -> { + if (project.isDisposed()) return; + try { + action.run(); + } catch (Exception e) { + LOG.warn("Tab operation failed", e); + } + }); + } +} diff --git a/backend/src/main/java/toolwindow/actions/RenameTabAction.java b/backend/src/main/java/toolwindow/actions/RenameTabAction.java deleted file mode 100644 index 173a67d..0000000 --- a/backend/src/main/java/toolwindow/actions/RenameTabAction.java +++ /dev/null @@ -1,125 +0,0 @@ -package toolwindow.actions; - -import com.intellij.openapi.actionSystem.*; -import com.intellij.openapi.application.ApplicationManager; -import com.intellij.openapi.project.Project; -import com.intellij.openapi.ui.Messages; -import com.intellij.openapi.wm.ToolWindow; -import com.intellij.ui.content.Content; -import com.intellij.ui.content.ContentManager; -import model.MyModel; -import org.jetbrains.annotations.NotNull; -import service.ToolWindowServiceInterface; -import service.ViewService; -import system.Defs; - -import java.awt.*; -import java.lang.reflect.Field; - -/** - * Action to rename a tab in the Git Scope tool window. - * Registered in plugin.xml and works across all projects. - */ -public class RenameTabAction extends AnAction { - - @Override - public void actionPerformed(@NotNull AnActionEvent e) { - Project project = e.getProject(); - if (project == null) return; - - Content targetContent = getContentFromContextMenuEvent(e); - if (targetContent == null) return; - - ToolWindowServiceInterface toolWindowService = project.getService(ToolWindowServiceInterface.class); - ToolWindow toolWindow = toolWindowService.getToolWindow(); - if (toolWindow == null) return; - - ContentManager contentManager = toolWindow.getContentManager(); - int index = contentManager.getIndexOfContent(targetContent); - String currentName = targetContent.getDisplayName(); - - // Don't allow renaming special tabs - if (index == 0 || currentName.equals(ViewService.PLUS_TAB_LABEL)) { - return; - } - - String newName = Messages.showInputDialog( - contentManager.getComponent(), - "Enter new tab name:", - "Rename Tab", - Messages.getQuestionIcon(), - currentName, - null - ); - - if (newName != null && !newName.isEmpty()) { - targetContent.setDisplayName(newName); - - // Update the model - ViewService viewService = project.getService(ViewService.class); - if (viewService != null) { - viewService.onTabRenamed(index, newName); - - int modelIndex = viewService.getModelIndex(index); - if (modelIndex >= 0 && modelIndex < viewService.getCollection().size()) { - MyModel model = viewService.getCollection().get(modelIndex); - toolWindowService.setupTabTooltip(model); - } - } - } - } - - @Override - public void update(@NotNull AnActionEvent e) { - // By default, hide the action - e.getPresentation().setEnabledAndVisible(false); - - Project project = e.getProject(); - if (project == null) { - return; - } - - // Check if this is our tool window - ToolWindow toolWindow = e.getData(PlatformDataKeys.TOOL_WINDOW); - if (toolWindow == null || !Defs.TOOL_WINDOW_NAME.equals(toolWindow.getId())) { - return; - } - - // Get the content that was right-clicked - Content targetContent = getContentFromContextMenuEvent(e); - if (targetContent != null) { - ContentManager contentManager = toolWindow.getContentManager(); - int index = contentManager.getIndexOfContent(targetContent); - String currentName = targetContent.getDisplayName(); - - // Don't allow renaming special tabs (HEAD tab or PLUS tab) - boolean enabled = index > 0 && !ViewService.PLUS_TAB_LABEL.equals(currentName); - e.getPresentation().setEnabledAndVisible(enabled); - } - } - - @Override - public @NotNull ActionUpdateThread getActionUpdateThread() { - return ActionUpdateThread.EDT; - } - - /** - * Gets the Content that was right-clicked in a context menu event - */ - private Content getContentFromContextMenuEvent(AnActionEvent e) { - Component contextComponent = e.getData(PlatformDataKeys.CONTEXT_COMPONENT); - if (contextComponent == null) { - return null; - } - try { - Field myContentField = contextComponent.getClass().getDeclaredField("myContent"); - myContentField.setAccessible(true); - Object myContentObject = myContentField.get(contextComponent); - if (myContentObject instanceof Content) { - return (Content) myContentObject; - } - } catch (NoSuchFieldException | IllegalAccessException ignored) { - } - return null; - } -} diff --git a/backend/src/main/java/toolwindow/actions/ResetTabNameAction.java b/backend/src/main/java/toolwindow/actions/ResetTabNameAction.java deleted file mode 100644 index 1d3267c..0000000 --- a/backend/src/main/java/toolwindow/actions/ResetTabNameAction.java +++ /dev/null @@ -1,133 +0,0 @@ -package toolwindow.actions; - -import com.intellij.openapi.actionSystem.*; -import com.intellij.openapi.application.ApplicationManager; -import com.intellij.openapi.project.Project; -import com.intellij.openapi.wm.ToolWindow; -import com.intellij.ui.content.Content; -import com.intellij.ui.content.ContentManager; -import model.MyModel; -import org.jetbrains.annotations.NotNull; -import service.TargetBranchService; -import service.ViewService; -import system.Defs; - -import java.awt.*; -import java.lang.reflect.Field; - -/** - * Action to reset a tab name to its original branch-based name. - * Registered in plugin.xml and works across all projects. - */ -public class ResetTabNameAction extends AnAction { - - @Override - public void actionPerformed(@NotNull AnActionEvent e) { - Project project = e.getProject(); - if (project == null) return; - - Content targetContent = getContentFromContextMenuEvent(e); - if (targetContent == null) return; - - ToolWindow toolWindow = e.getData(PlatformDataKeys.TOOL_WINDOW); - if (toolWindow == null || !Defs.TOOL_WINDOW_NAME.equals(toolWindow.getId())) { - return; - } - - ContentManager contentManager = toolWindow.getContentManager(); - int index = contentManager.getIndexOfContent(targetContent); - - // Don't allow resetting special tabs - if (index == 0 || targetContent.getDisplayName().equals(ViewService.PLUS_TAB_LABEL)) { - return; - } - - ViewService viewService = project.getService(ViewService.class); - if (viewService != null) { - int modelIndex = viewService.getModelIndex(index); - if (modelIndex >= 0 && modelIndex < viewService.getCollection().size()) { - MyModel model = viewService.getCollection().get(modelIndex); - - // Clear the custom name - this effectively resets to the default branch-based name - model.setCustomTabName(null); - - // Save the change - viewService.save(); - - // Update the UI with the branch-based name - TargetBranchService targetBranchService = project.getService(TargetBranchService.class); - targetBranchService.getTargetBranchDisplayAsync(model.getTargetBranchMap(), branchName -> { - ApplicationManager.getApplication().invokeLater(() -> { - // Update the tab name in the UI - targetContent.setDisplayName(branchName); - // Clear the tooltip - targetContent.setDescription(null); - }); - }); - } - } - } - - @Override - public void update(@NotNull AnActionEvent e) { - // By default, hide the action - e.getPresentation().setEnabledAndVisible(false); - - Project project = e.getProject(); - if (project == null) { - return; - } - - // Check if this is our tool window - ToolWindow toolWindow = e.getData(PlatformDataKeys.TOOL_WINDOW); - if (toolWindow == null || !Defs.TOOL_WINDOW_NAME.equals(toolWindow.getId())) { - return; - } - - // Get the content that was right-clicked - Content targetContent = getContentFromContextMenuEvent(e); - if (targetContent != null) { - ContentManager contentManager = toolWindow.getContentManager(); - int index = contentManager.getIndexOfContent(targetContent); - String currentName = targetContent.getDisplayName(); - - // Enable only for non-special tabs that have a custom name - boolean isSpecialTab = index == 0 || ViewService.PLUS_TAB_LABEL.equals(currentName); - if (!isSpecialTab) { - // Check if this tab has a custom name - ViewService viewService = project.getService(ViewService.class); - int modelIndex = viewService.getModelIndex(index); - if (modelIndex >= 0 && modelIndex < viewService.getCollection().size()) { - MyModel model = viewService.getCollection().get(modelIndex); - boolean hasCustomName = model.getCustomTabName() != null && !model.getCustomTabName().isEmpty(); - e.getPresentation().setEnabledAndVisible(hasCustomName); - } - } - } - } - - @Override - public @NotNull ActionUpdateThread getActionUpdateThread() { - return ActionUpdateThread.EDT; - } - - /** - * Gets the Content that was right-clicked in a context menu event - */ - private Content getContentFromContextMenuEvent(AnActionEvent e) { - Component contextComponent = e.getData(PlatformDataKeys.CONTEXT_COMPONENT); - if (contextComponent == null) { - return null; - } - try { - Field myContentField = contextComponent.getClass().getDeclaredField("myContent"); - myContentField.setAccessible(true); - Object myContentObject = myContentField.get(contextComponent); - if (myContentObject instanceof Content) { - return (Content) myContentObject; - } - } catch (NoSuchFieldException | IllegalAccessException ignored) { - } - return null; - } -} diff --git a/backend/src/main/java/toolwindow/actions/TabMoveActions.java b/backend/src/main/java/toolwindow/actions/TabMoveActions.java deleted file mode 100644 index 41dec53..0000000 --- a/backend/src/main/java/toolwindow/actions/TabMoveActions.java +++ /dev/null @@ -1,295 +0,0 @@ -package toolwindow.actions; - -import com.intellij.openapi.actionSystem.ActionUpdateThread; -import com.intellij.openapi.actionSystem.AnAction; -import com.intellij.openapi.actionSystem.AnActionEvent; -import com.intellij.openapi.actionSystem.PlatformDataKeys; -import com.intellij.openapi.project.Project; -import com.intellij.openapi.wm.ToolWindow; -import com.intellij.ui.content.Content; -import com.intellij.ui.content.ContentManager; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; -import service.ToolWindowServiceInterface; -import service.ViewService; -import system.Defs; - -import java.awt.*; -import java.lang.reflect.Field; - -import static service.ViewService.PLUS_TAB_LABEL; - -/** - * Actions to move tabs left and right in the Git Scope tool window. - */ -public class TabMoveActions { - private static final com.intellij.openapi.diagnostic.Logger LOG = Defs.getLogger(TabMoveActions.class); - - /** - * Gets the Content that was right-clicked in a context menu event - */ - @Nullable - private static Content getContentFromContextMenuEvent(AnActionEvent e) { - Component contextComponent = e.getData(PlatformDataKeys.CONTEXT_COMPONENT); - if (contextComponent == null) { - return null; - } - try { - Field myContentField = contextComponent.getClass().getDeclaredField("myContent"); - myContentField.setAccessible(true); - Object myContentObject = myContentField.get(contextComponent); - if (myContentObject instanceof Content) { - return (Content) myContentObject; - } - } catch (NoSuchFieldException | IllegalAccessException ignored) { - } - return null; - } - - /** - * Helper class to hold validation result for update() and actionPerformed() methods - */ - private static class UpdateContext { - final ContentManager contentManager; - final Content targetContent; - final int currentIndex; - final String tabName; - - UpdateContext(ContentManager contentManager, Content targetContent, int currentIndex, String tabName) { - this.contentManager = contentManager; - this.targetContent = targetContent; - this.currentIndex = currentIndex; - this.tabName = tabName; - } - } - - /** - * Helper class to hold action context including project and validated context - */ - private static class ActionContext { - final Project project; - final ContentManager contentManager; - final Content targetContent; - final int currentIndex; - - ActionContext(Project project, ContentManager contentManager, Content targetContent, int currentIndex) { - this.project = project; - this.contentManager = contentManager; - this.targetContent = targetContent; - this.currentIndex = currentIndex; - } - } - - /** - * Shared validation logic for actionPerformed() methods. - * Returns ActionContext if validation passes, null otherwise. - */ - @Nullable - private static ActionContext validateActionContext(AnActionEvent e) { - Project project = e.getProject(); - if (project == null) return null; - - // Get the right-clicked tab content - Content targetContent = getContentFromContextMenuEvent(e); - if (targetContent == null) return null; - - ToolWindowServiceInterface toolWindowService = project.getService(ToolWindowServiceInterface.class); - ContentManager contentManager = toolWindowService.getToolWindow().getContentManager(); - - int currentIndex = contentManager.getIndexOfContent(targetContent); - - return new ActionContext(project, contentManager, targetContent, currentIndex); - } - - /** - * Shared validation logic for update() methods. - * Returns UpdateContext if validation passes, null otherwise. - */ - @Nullable - private static UpdateContext validateUpdateContext(AnActionEvent e) { - Project project = e.getProject(); - if (project == null) { - return null; - } - - // Check if this is our tool window - ToolWindow toolWindow = e.getData(PlatformDataKeys.TOOL_WINDOW); - if (toolWindow == null || !Defs.TOOL_WINDOW_NAME.equals(toolWindow.getId())) { - return null; - } - - // Get the right-clicked tab content - Content targetContent = getContentFromContextMenuEvent(e); - if (targetContent == null) { - return null; - } - - ContentManager contentManager = toolWindow.getContentManager(); - int currentIndex = contentManager.getIndexOfContent(targetContent); - String tabName = targetContent.getTabName(); - - return new UpdateContext(contentManager, targetContent, currentIndex, tabName); - } - - /** - * Action to move the current tab to the left. - * Registered in plugin.xml and works across all projects. - */ - public static class MoveTabLeft extends AnAction { - @Override - public void actionPerformed(@NotNull AnActionEvent e) { - ActionContext ctx = validateActionContext(e); - if (ctx == null) return; - - int newIndex = ctx.currentIndex - 1; - - // Cannot move HEAD tab (index 0) or move before HEAD tab - if (ctx.currentIndex <= 1 || newIndex < 1) { - return; - } - - // Cannot move + tab - if (PLUS_TAB_LABEL.equals(ctx.targetContent.getTabName())) { - return; - } - - moveTab(ctx.project, ctx.contentManager, ctx.targetContent, ctx.currentIndex, newIndex); - } - - @Override - public void update(@NotNull AnActionEvent e) { - // By default, hide the action - e.getPresentation().setEnabledAndVisible(false); - - UpdateContext ctx = validateUpdateContext(e); - if (ctx == null) { - return; - } - - // Enable only if not HEAD tab (index 0), not + tab, and can move left (index > 1) - boolean enabled = ctx.currentIndex > 1 && !PLUS_TAB_LABEL.equals(ctx.tabName); - e.getPresentation().setEnabledAndVisible(enabled); - } - - @Override - public @NotNull ActionUpdateThread getActionUpdateThread() { - return ActionUpdateThread.EDT; - } - } - - /** - * Action to move the current tab to the right. - * Registered in plugin.xml and works across all projects. - */ - public static class MoveTabRight extends AnAction { - @Override - public void actionPerformed(@NotNull AnActionEvent e) { - ActionContext ctx = validateActionContext(e); - if (ctx == null) return; - - int newIndex = ctx.currentIndex + 1; - int lastIndex = ctx.contentManager.getContentCount() - 1; - - // Cannot move HEAD tab (index 0) or move past + tab - if (ctx.currentIndex == 0 || newIndex >= lastIndex) { - return; - } - - // Cannot move + tab - if (PLUS_TAB_LABEL.equals(ctx.targetContent.getTabName())) { - return; - } - - moveTab(ctx.project, ctx.contentManager, ctx.targetContent, ctx.currentIndex, newIndex); - } - - @Override - public void update(@NotNull AnActionEvent e) { - // By default, hide the action - e.getPresentation().setEnabledAndVisible(false); - - UpdateContext ctx = validateUpdateContext(e); - if (ctx == null) { - return; - } - - int lastIndex = ctx.contentManager.getContentCount() - 1; - - // Enable only if not HEAD tab (index 0), not + tab, and can move right (not already at second-to-last position) - boolean enabled = ctx.currentIndex > 0 && ctx.currentIndex < lastIndex - 1 && !PLUS_TAB_LABEL.equals(ctx.tabName); - e.getPresentation().setEnabledAndVisible(enabled); - } - - @Override - public @NotNull ActionUpdateThread getActionUpdateThread() { - return ActionUpdateThread.EDT; - } - } - - /** - * Helper method to move a tab from one position to another - */ - private static void moveTab(Project project, ContentManager contentManager, Content content, int oldIndex, int newIndex) { - ViewService viewService = null; - try { - LOG.debug("Moving tab from index " + oldIndex + " to " + newIndex); - - // Additional validation to prevent moving + tab or moving to + tab position - int lastIndex = contentManager.getContentCount() - 1; - - // Cannot move HEAD tab (index 0) - if (oldIndex == 0) { - LOG.debug("Cannot move HEAD tab"); - return; - } - - // Cannot move + tab (last index) - if (oldIndex == lastIndex || PLUS_TAB_LABEL.equals(content.getTabName())) { - LOG.debug("Cannot move + tab"); - return; - } - - // Cannot move to position 0 (before HEAD) or to last position (where + tab is) - if (newIndex < 1 || newIndex >= lastIndex) { - LOG.debug("Invalid target index: " + newIndex); - return; - } - - // Verify the + tab is still at the last position - Content lastContent = contentManager.getContent(lastIndex); - if (lastContent == null || !PLUS_TAB_LABEL.equals(lastContent.getTabName())) { - LOG.warn("+ tab is not at expected position!"); - return; - } - - // IMPORTANT: Set the flag BEFORE moving tabs to prevent listener interference - viewService = project.getService(ViewService.class); - if (viewService != null) { - viewService.setProcessingTabReorder(true); - } - - // Remove content from old position - contentManager.removeContent(content, false); - - // Add content at new position - contentManager.addContent(content, newIndex); - - // Select the moved tab - contentManager.setSelectedContent(content, true); - - // Now rebuild collection and save - if (viewService != null) { - viewService.onTabReordered(oldIndex, newIndex); - } - - LOG.debug("Tab moved successfully"); - } catch (Exception e) { - LOG.warn("Error moving tab: " + e.getMessage(), e); - } finally { - // Always clear the flag - if (viewService != null) { - viewService.setProcessingTabReorder(false); - } - } - } -} diff --git a/backend/src/main/resources/gitscope.backend.xml b/backend/src/main/resources/gitscope.backend.xml index 8b611ad..0efd928 100644 --- a/backend/src/main/resources/gitscope.backend.xml +++ b/backend/src/main/resources/gitscope.backend.xml @@ -47,6 +47,7 @@ + @@ -68,17 +69,8 @@ relative-to-action="Vcs.CopyRevisionNumberAction"/> - - - - - - - - - - - - + diff --git a/frontend/src/main/java/gitscope/frontend/actions/TabContextActions.kt b/frontend/src/main/java/gitscope/frontend/actions/TabContextActions.kt new file mode 100644 index 0000000..4f6a6f4 --- /dev/null +++ b/frontend/src/main/java/gitscope/frontend/actions/TabContextActions.kt @@ -0,0 +1,132 @@ +package gitscope.frontend.actions + +import com.intellij.openapi.actionSystem.AnActionEvent +import com.intellij.openapi.components.service +import com.intellij.openapi.diagnostic.Logger +import com.intellij.openapi.project.Project +import com.intellij.openapi.ui.Messages +import com.intellij.openapi.wm.ToolWindow +import com.intellij.openapi.wm.ToolWindowContextMenuActionBase +import com.intellij.platform.project.ProjectId +import com.intellij.platform.project.projectId +import com.intellij.ui.content.Content +import com.intellij.ui.content.ContentManager +import kotlinx.coroutines.launch +import rpc.FrontendTabStateService +import rpc.TabMoveDirection +import rpc.UtilRpcApi +import system.Defs + +/** + * Tab context-menu actions for the Git Scope tool window, registered on the frontend. + * + *

Registered here for both modes. In split mode the tab strip is rendered by the frontend, so + * its context menu is built from the frontend's action registry and backend-registered actions + * never appear in it — which is why renaming, resetting and moving tabs were all missing over + * Remote Development. Monolith loads this module too, so one registration serves both and there is + * no second implementation to keep in step. + * + *

The tool window, its ContentManager and the scope models are backend state, so these actions + * decide enablement from what is visible here (tab index and label) and delegate the operation over + * [UtilRpcApi] to TabActionService, which owns the rules and re-validates every request. + */ +sealed class TabContextAction : ToolWindowContextMenuActionBase() { + + final override fun update(e: AnActionEvent, toolWindow: ToolWindow, content: Content?) { + e.presentation.isEnabledAndVisible = false + + if (e.project == null || toolWindow.id != Defs.TOOL_WINDOW_NAME || content == null) return + + val contentManager = toolWindow.contentManager + val index = contentManager.getIndexOfContent(content) + if (index < 0) return + + e.presentation.isEnabledAndVisible = isEnabled(e.project!!, contentManager, index, content) + } + + final override fun actionPerformed(e: AnActionEvent, toolWindow: ToolWindow, content: Content?) { + val project = e.project ?: return + if (content == null) return + + val index = toolWindow.contentManager.getIndexOfContent(content) + if (index < 0) return + + perform(project, index, content) + } + + protected abstract fun isEnabled(project: Project, contentManager: ContentManager, index: Int, content: Content): Boolean + + protected abstract fun perform(project: Project, index: Int, content: Content) + + /** Tab is neither the HEAD tab (index 0) nor the trailing "+" tab. */ + protected fun isRegularTab(index: Int, content: Content): Boolean = + index > 0 && content.tabName != Defs.PLUS_TAB_LABEL + + protected fun sendToBackend(project: Project, description: String, call: suspend (UtilRpcApi, ProjectId) -> Unit) { + // Resolve the project id on the caller's (EDT) side, like the navigation actions do. + val projectId = project.projectId() + project.service().scope.launch { + try { + call(UtilRpcApi.getInstance(), projectId) + } catch (t: Throwable) { + LOG.warn("Tab action '$description' failed", t) + } + } + } + + companion object { + @JvmStatic + protected val LOG: Logger = Defs.getLogger(TabContextAction::class.java) + } +} + +class FrontendRenameTabAction : TabContextAction() { + override fun isEnabled(project: Project, contentManager: ContentManager, index: Int, content: Content) = + isRegularTab(index, content) + + override fun perform(project: Project, index: Int, content: Content) { + // Prompt on the frontend, where the UI is, then send only the result over RPC. + val newName = Messages.showInputDialog( + project, + "Enter new tab name:", + "Rename Tab", + Messages.getQuestionIcon(), + content.displayName, + null + ) + if (newName.isNullOrEmpty()) return + + sendToBackend(project, "rename") { api, pid -> api.renameTab(pid, index, newName) } + } +} + +class FrontendResetTabNameAction : TabContextAction() { + // Whether a tab carries a custom name is backend model state, mirrored by + // FrontendTabStateService. While that state is still unknown the action stays enabled and the + // backend no-ops if there is nothing to reset. + override fun isEnabled(project: Project, contentManager: ContentManager, index: Int, content: Content) = + isRegularTab(index, content) && + project.service().hasCustomNameOrUnknown(index) + + override fun perform(project: Project, index: Int, content: Content) { + sendToBackend(project, "reset name") { api, pid -> api.resetTabName(pid, index) } + } +} + +class FrontendMoveTabLeftAction : TabContextAction() { + override fun isEnabled(project: Project, contentManager: ContentManager, index: Int, content: Content) = + isRegularTab(index, content) && index > 1 + + override fun perform(project: Project, index: Int, content: Content) { + sendToBackend(project, "move left") { api, pid -> api.moveTab(pid, index, TabMoveDirection.LEFT) } + } +} + +class FrontendMoveTabRightAction : TabContextAction() { + override fun isEnabled(project: Project, contentManager: ContentManager, index: Int, content: Content) = + isRegularTab(index, content) && index < contentManager.contentCount - 2 + + override fun perform(project: Project, index: Int, content: Content) { + sendToBackend(project, "move right") { api, pid -> api.moveTab(pid, index, TabMoveDirection.RIGHT) } + } +} diff --git a/frontend/src/main/java/rpc/FrontendTabStateService.kt b/frontend/src/main/java/rpc/FrontendTabStateService.kt new file mode 100644 index 0000000..23f1ffc --- /dev/null +++ b/frontend/src/main/java/rpc/FrontendTabStateService.kt @@ -0,0 +1,76 @@ +package rpc + +import com.intellij.openapi.components.Service +import com.intellij.openapi.components.service +import com.intellij.openapi.diagnostic.Logger +import com.intellij.openapi.project.Project +import com.intellij.openapi.startup.ProjectActivity +import com.intellij.platform.project.projectId +import fleet.rpc.client.durable +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.delay +import kotlinx.coroutines.isActive +import kotlinx.coroutines.launch +import system.Defs + +/** + * Mirrors the backend's set of tabs that carry a custom name, so "Reset Tab Name" can be disabled + * for tabs with nothing to reset. + * + *

Action update() cannot suspend, so the answer has to be available synchronously — hence a + * pushed StateFlow cached here rather than a request per menu build, which would also put a network + * round-trip in the way of opening a context menu. + * + *

Until the first value arrives the answer is "unknown", and callers treat unknown as enabled: + * the worst case is then an action that runs and finds nothing to reset, never one that is missing. + */ +@Service(Service.Level.PROJECT) +class FrontendTabStateService( + private val project: Project, + private val coroutineScope: CoroutineScope +) { + @Volatile + private var customNamedTabs: Set? = null + + init { + coroutineScope.launch { + while (isActive) { + try { + durable { + UtilRpcApi.getInstance() + .getCustomNamedTabs(project.projectId()) + .collect { indices -> + LOG.debug("Custom-named tabs: $indices") + customNamedTabs = indices.toSet() + } + } + } catch (e: CancellationException) { + throw e + } catch (t: Throwable) { + LOG.warn("Tab state subscription failed, re-subscribing", t) + } + // Fall back to "unknown" so the menu stays usable while disconnected. + customNamedTabs = null + delay(RESUBSCRIBE_DELAY_MS) + } + } + } + + /** True when the tab is known to have a custom name, or when the state is not known yet. */ + fun hasCustomNameOrUnknown(tabIndex: Int): Boolean { + val known = customNamedTabs ?: return true + return tabIndex in known + } + + companion object { + private val LOG: Logger = Defs.getLogger(FrontendTabStateService::class.java) + private const val RESUBSCRIBE_DELAY_MS = 5_000L + } +} + +class FrontendTabStateStartup : ProjectActivity { + override suspend fun execute(project: Project) { + project.service() + } +} diff --git a/frontend/src/main/resources/gitscope.frontend.xml b/frontend/src/main/resources/gitscope.frontend.xml index ee3ef56..f776e98 100644 --- a/frontend/src/main/resources/gitscope.frontend.xml +++ b/frontend/src/main/resources/gitscope.frontend.xml @@ -8,10 +8,12 @@ + + @@ -35,5 +37,28 @@ class="gitscope.frontend.actions.ShowDiffAction" text="Show Diff" description="Show the Git Scope diff for the current file in an editor tab"/> + + + + + + + + + + + + + + diff --git a/shared/src/main/java/rpc/UtilRpcApi.kt b/shared/src/main/java/rpc/UtilRpcApi.kt index 0e0a307..3373635 100644 --- a/shared/src/main/java/rpc/UtilRpcApi.kt +++ b/shared/src/main/java/rpc/UtilRpcApi.kt @@ -23,6 +23,13 @@ enum class ChangeNavDirection { PREVIOUS_FILE } +/** Direction for moving a scope tab within the tool window. */ +@Serializable +enum class TabMoveDirection { + LEFT, + RIGHT +} + @Rpc interface UtilRpcApi : RemoteApi { suspend fun getCommands(projectId: ProjectId): Flow @@ -54,6 +61,31 @@ interface UtilRpcApi : RemoteApi { */ suspend fun showDiff(projectId: ProjectId, currentFilePath: String?) + /** + * Tab operations for the tool window's tab context menu, addressed by tab index. + * + *

In split mode the tab strip is rendered by the frontend, so the context menu is built from + * the frontend's action registry — backend-registered actions never appear in it. The frontend + * actions therefore delegate here, because the tool window, its ContentManager and the scope + * models all live on the backend. All three are no-ops when the index does not address a + * renameable/movable tab, so the backend stays authoritative over the rules. + */ + suspend fun renameTab(projectId: ProjectId, tabIndex: Int, newName: String) + + suspend fun resetTabName(projectId: ProjectId, tabIndex: Int) + + suspend fun moveTab(projectId: ProjectId, tabIndex: Int, direction: TabMoveDirection) + + /** + * Indices of tabs that currently carry a custom name, so "Reset Tab Name" can be disabled for + * tabs that have nothing to reset. Whether a tab was renamed is model state the frontend cannot + * see, and action update() cannot suspend, so the backend pushes it instead. + * + *

A [kotlinx.coroutines.flow.StateFlow] on the backend: a new subscriber immediately receives + * the current set, and every rename, reset, reorder or tab load republishes it. + */ + suspend fun getCustomNamedTabs(projectId: ProjectId): Flow> + companion object { suspend fun getInstance(): UtilRpcApi { return RemoteApiProviderService.resolve(remoteApiDescriptor()) diff --git a/shared/src/main/java/system/Defs.java b/shared/src/main/java/system/Defs.java index 3a7dede..d6367eb 100644 --- a/shared/src/main/java/system/Defs.java +++ b/shared/src/main/java/system/Defs.java @@ -10,6 +10,12 @@ public class Defs { public static String TOOL_WINDOW_NAME = "Git Scope"; public static Icon ICON = AllIcons.Actions.Diff; + /** + * Label of the trailing "add a scope" tab. Lives here rather than in ViewService because the + * frontend needs it too, to recognise the special tabs in the tab context menu. + */ + public static final String PLUS_TAB_LABEL = "+"; + /** * Global logger category for Git Scope plugin. * To enable debug logging for all Git Scope components, add this to Debug Log Settings: From e1d171005cb4bb7f5df5eaa17b0f20e15c74c131 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20W=C3=A5llberg?= Date: Sat, 15 Aug 2026 22:05:37 +0200 Subject: [PATCH 09/14] fix: show the original tab name as a tooltip reliably in both modes The tooltip on a renamed tab was missing right after boot in monolith, and always missing in split mode, for two independent reasons. At boot it is built from the tab's branch-based name, which resolves to an empty string before the VCS mapping is ready, and nothing retried afterwards, so a tab restored with a custom name kept no tooltip for the rest of the session. In split mode the tab strip is rendered against the frontend's Content objects while setDescription is called on the backend's copy, which never reaches the label. Both are fixed by making the original names part of the state the backend already pushes for tab actions: the flow now carries a map of tab index to branch-based name rather than a list of renamed indices, and the frontend applies it as the tooltip on its own contents. Reset Tab Name's enablement comes from the same map, since presence in it means the tab has a custom name. The map is republished after every rename, reset and move, when a frontend subscribes, after the tabs are restored, and after each change collection, the last being what finally supplies names that were unresolvable at boot. Tabs whose branch name is still unresolvable are left out rather than published with a blank tooltip. --- .../src/main/java/rpc/BackendUtilRpcImpl.kt | 6 +- .../src/main/java/rpc/UtilCommandService.kt | 14 ++--- .../main/java/service/TabActionService.java | 59 +++++++++++++++---- .../src/main/java/service/ViewService.java | 7 +++ .../main/java/rpc/FrontendTabStateService.kt | 53 +++++++++++++---- shared/src/main/java/rpc/UtilRpcApi.kt | 15 +++-- 6 files changed, 114 insertions(+), 40 deletions(-) diff --git a/backend/src/main/java/rpc/BackendUtilRpcImpl.kt b/backend/src/main/java/rpc/BackendUtilRpcImpl.kt index 44674ed..c53a86d 100644 --- a/backend/src/main/java/rpc/BackendUtilRpcImpl.kt +++ b/backend/src/main/java/rpc/BackendUtilRpcImpl.kt @@ -50,12 +50,12 @@ class BackendUtilRpcImpl : UtilRpcApi { project.service().moveTab(tabIndex, direction) } - override suspend fun getCustomNamedTabs(projectId: ProjectId): Flow> { + override suspend fun getRenamedTabs(projectId: ProjectId): Flow> { val project = projectId.findProjectOrNull() ?: return emptyFlow() // Publish the current state before the subscriber starts collecting, so a frontend that // connects after the tabs were restored still gets the truth rather than the initial empty. - project.service().publishCustomNamedTabs() - return project.service().customNamedTabs + project.service().publishRenamedTabs() + return project.service().renamedTabs } } diff --git a/backend/src/main/java/rpc/UtilCommandService.kt b/backend/src/main/java/rpc/UtilCommandService.kt index d18a22e..c98af9c 100644 --- a/backend/src/main/java/rpc/UtilCommandService.kt +++ b/backend/src/main/java/rpc/UtilCommandService.kt @@ -20,15 +20,15 @@ class UtilCommandService { private val previewTabEnabled = AtomicReference(null) /** - * Indices of tabs carrying a custom name. A StateFlow rather than a command, so the frontend can - * read the current value at any time — including right after it subscribes — to decide whether - * "Reset Tab Name" has anything to reset. + * Renamed tabs: tab index -> the branch-based name it would revert to. A StateFlow rather than a + * command, so the frontend can read the current value at any time — including right after it + * subscribes — and so republishing an unchanged map costs subscribers nothing. */ - private val _customNamedTabs = MutableStateFlow>(emptyList()) - val customNamedTabs = _customNamedTabs.asStateFlow() + private val _renamedTabs = MutableStateFlow>(emptyMap()) + val renamedTabs = _renamedTabs.asStateFlow() - fun setCustomNamedTabs(indices: List) { - _customNamedTabs.value = indices + fun setRenamedTabs(tabs: Map) { + _renamedTabs.value = tabs } fun selectInProject(filePath: String) { diff --git a/backend/src/main/java/service/TabActionService.java b/backend/src/main/java/service/TabActionService.java index eef1723..0a01265 100644 --- a/backend/src/main/java/service/TabActionService.java +++ b/backend/src/main/java/service/TabActionService.java @@ -11,8 +11,10 @@ import org.jetbrains.annotations.Nullable; import system.Defs; -import java.util.ArrayList; -import java.util.List; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicInteger; /** * Backend-side tab operations addressed by tab index: rename, reset name, move. @@ -57,7 +59,7 @@ public void renameTab(int tabIndex, @NotNull String newName) { if (model != null) { project.getService(ToolWindowServiceInterface.class).setupTabTooltip(model); } - publishCustomNamedTabs(); + publishRenamedTabs(); }); } @@ -94,7 +96,7 @@ public void resetTabName(int tabIndex) { content.setDisplayName(branchName); content.setDescription(null); })); - publishCustomNamedTabs(); + publishRenamedTabs(); }); } @@ -132,16 +134,21 @@ public void moveTab(int tabIndex, @NotNull rpc.TabMoveDirection direction) { } } // Indices shifted, so the previously published set no longer addresses the same tabs. - publishCustomNamedTabs(); + publishRenamedTabs(); }); } /** - * Republishes which tabs carry a custom name, so the frontend can disable "Reset Tab Name" where - * there is nothing to reset. Called after every operation that can change the answer, and once - * when a frontend subscribes. + * Republishes the renamed tabs as index -> branch-based name, which the frontend uses both to + * decide whether "Reset Tab Name" has anything to reset and to show the original name as the + * tab's tooltip. + * + *

Called after every operation that can change the answer, when a frontend subscribes, and + * after a change collection — the branch names cannot be resolved until repositories are + * registered, which is why a tab renamed in a previous session had no tooltip right after boot. + * Publishing an unchanged map is free: the state flow conflates equal values. */ - public void publishCustomNamedTabs() { + public void publishRenamedTabs() { ApplicationManager.getApplication().invokeLater(() -> { if (project.isDisposed()) return; @@ -149,19 +156,45 @@ public void publishCustomNamedTabs() { ViewService viewService = project.getService(ViewService.class); if (contentManager == null || viewService == null) return; - List indices = new ArrayList<>(); + Map renamed = new LinkedHashMap<>(); for (int index = 1; index < contentManager.getContentCount(); index++) { if (!isRenameableTab(contentManager, index)) continue; MyModel model = getModelForTab(viewService, index); if (model != null && model.getCustomTabName() != null && !model.getCustomTabName().isEmpty()) { - indices.add(index); + renamed.put(index, model); } } - LOG.debug("publishCustomNamedTabs: " + indices); - project.getService(rpc.UtilCommandService.class).setCustomNamedTabs(indices); + + if (renamed.isEmpty()) { + publish(Map.of()); + return; + } + + // One async branch-name resolution per renamed tab; publish once they have all answered. + TargetBranchService targetBranchService = project.getService(TargetBranchService.class); + Map resolved = new ConcurrentHashMap<>(); + AtomicInteger pending = new AtomicInteger(renamed.size()); + for (Map.Entry entry : renamed.entrySet()) { + targetBranchService.getTargetBranchDisplayAsync(entry.getValue().getTargetBranchMap(), branchName -> { + // Empty means repositories are not resolvable yet; leave the tab out rather than + // publish a blank tooltip, and a later collection will republish with the name. + if (branchName != null && !branchName.isEmpty()) { + resolved.put(entry.getKey(), branchName); + } + if (pending.decrementAndGet() == 0) { + publish(Map.copyOf(resolved)); + } + }); + } }); } + private void publish(@NotNull Map renamedTabs) { + if (project.isDisposed()) return; + LOG.debug("publishRenamedTabs: " + renamedTabs); + project.getService(rpc.UtilCommandService.class).setRenamedTabs(renamedTabs); + } + // --- rules, shared by every caller --- /** Whether the tab at {@code index} may be renamed: not HEAD (index 0) and not the "+" tab. */ diff --git a/backend/src/main/java/service/ViewService.java b/backend/src/main/java/service/ViewService.java index c027c9b..c552cba 100644 --- a/backend/src/main/java/service/ViewService.java +++ b/backend/src/main/java/service/ViewService.java @@ -412,6 +412,9 @@ public void initTabsSequentially() { } } + // Publish tooltips for tabs restored with a custom name. + project.getService(TabActionService.class).publishRenamedTabs(); + // Step 4: Add the listener after all tabs are initialized toolWindowService.addListener(); @@ -723,6 +726,10 @@ private void collectChangesInternal(MyModel model, TargetBranchMap targetBranchM LOG.debug("Scope statuses changed for generation " + gen + ", refreshing file colors"); refreshFileColors(); } + + // Repositories are resolvable by now, so tab tooltips that could not be + // built at boot (empty branch name) can finally be published. + project.getService(TabActionService.class).publishRenamedTabs(); } else { LOG.debug("Discarding changes for generation " + gen + " (current generation is " + currentGen + ")"); } diff --git a/frontend/src/main/java/rpc/FrontendTabStateService.kt b/frontend/src/main/java/rpc/FrontendTabStateService.kt index 23f1ffc..7f7d253 100644 --- a/frontend/src/main/java/rpc/FrontendTabStateService.kt +++ b/frontend/src/main/java/rpc/FrontendTabStateService.kt @@ -4,7 +4,9 @@ import com.intellij.openapi.components.Service import com.intellij.openapi.components.service import com.intellij.openapi.diagnostic.Logger import com.intellij.openapi.project.Project +import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.startup.ProjectActivity +import com.intellij.openapi.wm.ToolWindowManager import com.intellij.platform.project.projectId import fleet.rpc.client.durable import kotlinx.coroutines.CancellationException @@ -15,13 +17,18 @@ import kotlinx.coroutines.launch import system.Defs /** - * Mirrors the backend's set of tabs that carry a custom name, so "Reset Tab Name" can be disabled - * for tabs with nothing to reset. + * Mirrors the backend's renamed tabs — tab index -> the branch-based name the tab would revert to. * - *

Action update() cannot suspend, so the answer has to be available synchronously — hence a - * pushed StateFlow cached here rather than a request per menu build, which would also put a network + *

Serves two purposes. "Reset Tab Name" is disabled for tabs with nothing to reset, and action + * update() cannot suspend, so the answer has to be available synchronously — hence a pushed + * StateFlow cached here rather than a request per menu build, which would also put a network * round-trip in the way of opening a context menu. * + *

And the original name is applied as the tab's tooltip here, on the frontend's own Content + * objects. The backend sets it too, but in split mode the tab strip is rendered against frontend + * contents, and the description written on the backend copy never reaches the label. In monolith + * both sides share the same Content, so this is simply an idempotent re-set. + * *

Until the first value arrives the answer is "unknown", and callers treat unknown as enabled: * the worst case is then an action that runs and finds nothing to reset, never one that is missing. */ @@ -31,7 +38,7 @@ class FrontendTabStateService( private val coroutineScope: CoroutineScope ) { @Volatile - private var customNamedTabs: Set? = null + private var renamedTabs: Map? = null init { coroutineScope.launch { @@ -39,10 +46,11 @@ class FrontendTabStateService( try { durable { UtilRpcApi.getInstance() - .getCustomNamedTabs(project.projectId()) - .collect { indices -> - LOG.debug("Custom-named tabs: $indices") - customNamedTabs = indices.toSet() + .getRenamedTabs(project.projectId()) + .collect { tabs -> + LOG.debug("Renamed tabs: $tabs") + renamedTabs = tabs + applyTooltips(tabs) } } } catch (e: CancellationException) { @@ -51,7 +59,7 @@ class FrontendTabStateService( LOG.warn("Tab state subscription failed, re-subscribing", t) } // Fall back to "unknown" so the menu stays usable while disconnected. - customNamedTabs = null + renamedTabs = null delay(RESUBSCRIBE_DELAY_MS) } } @@ -59,10 +67,33 @@ class FrontendTabStateService( /** True when the tab is known to have a custom name, or when the state is not known yet. */ fun hasCustomNameOrUnknown(tabIndex: Int): Boolean { - val known = customNamedTabs ?: return true + val known = renamedTabs ?: return true return tabIndex in known } + /** + * Shows the original name as the tooltip of each renamed tab, and clears it elsewhere. The tab + * label reads Content.getDescription(), and the tool window content UI refreshes it on the + * resulting property change. + */ + private fun applyTooltips(tabs: Map) { + ApplicationManager.getApplication().invokeLater { + if (project.isDisposed) return@invokeLater + + val toolWindow = ToolWindowManager.getInstance(project).getToolWindow(Defs.TOOL_WINDOW_NAME) + ?: return@invokeLater // not created yet; the next publish re-applies + val contentManager = toolWindow.contentManager + + for (index in 0 until contentManager.contentCount) { + val content = contentManager.getContent(index) ?: continue + val tooltip = tabs[index] + if (content.description != tooltip) { + content.description = tooltip + } + } + } + } + companion object { private val LOG: Logger = Defs.getLogger(FrontendTabStateService::class.java) private const val RESUBSCRIBE_DELAY_MS = 5_000L diff --git a/shared/src/main/java/rpc/UtilRpcApi.kt b/shared/src/main/java/rpc/UtilRpcApi.kt index 3373635..5c5fc2c 100644 --- a/shared/src/main/java/rpc/UtilRpcApi.kt +++ b/shared/src/main/java/rpc/UtilRpcApi.kt @@ -77,14 +77,17 @@ interface UtilRpcApi : RemoteApi { suspend fun moveTab(projectId: ProjectId, tabIndex: Int, direction: TabMoveDirection) /** - * Indices of tabs that currently carry a custom name, so "Reset Tab Name" can be disabled for - * tabs that have nothing to reset. Whether a tab was renamed is model state the frontend cannot - * see, and action update() cannot suspend, so the backend pushes it instead. + * Renamed tabs, as tab index -> the branch-based name the tab would revert to. Only tabs with a + * custom name appear, so presence answers "can this be reset?" and the value is the tooltip to + * show on the renamed tab. * - *

A [kotlinx.coroutines.flow.StateFlow] on the backend: a new subscriber immediately receives - * the current set, and every rename, reset, reorder or tab load republishes it. + *

Both are backend model state the frontend cannot reach, and action update() cannot suspend, + * so the backend pushes them. A [kotlinx.coroutines.flow.StateFlow]: a new subscriber + * immediately receives the current map, and it is republished whenever it can have changed -- + * after a rename, reset or move, and after a change collection, which is when repositories + * first become resolvable and the branch names stop being empty. */ - suspend fun getCustomNamedTabs(projectId: ProjectId): Flow> + suspend fun getRenamedTabs(projectId: ProjectId): Flow> companion object { suspend fun getInstance(): UtilRpcApi { From 959805844965c2e12b5ee81c7511fa7ee71eee50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20W=C3=A5llberg?= Date: Sat, 15 Aug 2026 22:24:32 +0200 Subject: [PATCH 10/14] fix: handle tabs reordered by dragging instead of deleting their models The platform finishes a tab drag by re-adding the content at its new index, which reaches the plugin as a content removal indistinguishable from closing a tab. MyTabContentListener answered it by removing the model, so the dragged tab lost its scope, the collection order no longer matched the tab order, and save() persisted the result, which is why the tab was gone after restarting the IDE. Only monolith is affected, since the platform gates tab dragging on AppMode.isMonolith(). There is no "content moved" event to listen for, so repair rather than prevent. The removal still happens exactly as before, since that path also runs when tabs are torn down in bulk on project close, but the removed Content is remembered, and if the very same Content is added back, contentAdded recognises the move and rebuildCollectionFromTabOrder() derives the collection from the tabs themselves. Detection is deliberately event-driven rather than timed, because the platform can complete a drag several EDT ticks after the removal; the remembered contents are held weakly, so a Content that really was closed is collected without further bookkeeping. HEAD is moved back to the front and "+" back to the end before the collection is rebuilt, and the renamed-tab map is republished because the indices it addresses have shifted. --- .../java/listener/MyTabContentListener.java | 41 +++++++++++- .../src/main/java/service/ViewService.java | 62 +++++++++++++++++++ 2 files changed, 101 insertions(+), 2 deletions(-) diff --git a/backend/src/main/java/listener/MyTabContentListener.java b/backend/src/main/java/listener/MyTabContentListener.java index 41d4c13..7fbfb41 100644 --- a/backend/src/main/java/listener/MyTabContentListener.java +++ b/backend/src/main/java/listener/MyTabContentListener.java @@ -2,6 +2,7 @@ import com.intellij.openapi.project.Project; import com.intellij.openapi.util.NlsContexts; +import com.intellij.ui.content.Content; import com.intellij.ui.content.ContentManagerEvent; import com.intellij.ui.content.ContentManagerListener; import org.jetbrains.annotations.NotNull; @@ -11,7 +12,10 @@ import toolwindow.elements.VcsTree; import javax.swing.*; +import java.util.Collections; import java.util.Objects; +import java.util.Set; +import java.util.WeakHashMap; import static service.ViewService.PLUS_TAB_LABEL; @@ -41,7 +45,28 @@ private ToolWindowServiceInterface getToolWindowService() { return toolWindowService; } + /** + * Contents that were removed while the plugin was not reordering them. A drag re-adds the very + * same Content object, so seeing one come back identifies a move; a genuinely closed Content is + * never added again, and the weak set lets it be collected without bookkeeping. + */ + private final Set removedContents = + Collections.newSetFromMap(new WeakHashMap<>()); + public void contentAdded(@NotNull ContentManagerEvent event) { + // Deliberately not time-based: the platform can complete a drag several EDT ticks after the + // removal, so anything that checks "is it back yet?" on a timer misses the slow cases and + // leaves the tab's model deleted. + if (!removedContents.remove(event.getContent())) { + return; + } + + ViewService viewService = getViewService(); + if (viewService == null || viewService.isDisposed()) return; + + LOG.debug("Tab '" + event.getContent().getTabName() + "' came back after removal - " + + "it was dragged, not closed; resyncing tab order"); + viewService.onTabsDragged(); } public void selectionChanged(@NotNull ContentManagerEvent event) { @@ -87,8 +112,20 @@ public void contentRemoved(@NotNull ContentManagerEvent event) { // Don't remove the model if we're just reordering tabs ViewService viewService = getViewService(); - if (viewService != null && !viewService.isProcessingTabReorder()) { - viewService.removeTab(event.getIndex()); + if (viewService == null || viewService.isProcessingTabReorder()) { + return; } + + // Treat it as a close, exactly as before: this path also runs when tabs are torn down in + // bulk (project close, tool window re-init), and its index check is what keeps those from + // wiping the saved collection. + viewService.removeTab(event.getIndex()); + + // A tab dragged within the header is re-added at its new index, which arrives here as a + // removal indistinguishable from a close, and the platform has no "content moved" event. + // Remember the content: if it is added back, contentAdded recognises the move and rebuilds + // the collection from the tab order, which restores the model removed just above -- it is + // still reachable through the content, so nothing is lost. + removedContents.add(event.getContent()); } } \ No newline at end of file diff --git a/backend/src/main/java/service/ViewService.java b/backend/src/main/java/service/ViewService.java index c552cba..5a0eb56 100644 --- a/backend/src/main/java/service/ViewService.java +++ b/backend/src/main/java/service/ViewService.java @@ -882,6 +882,7 @@ public void removeTab(int tabIndex) { } } + public void onTabReordered(int oldIndex, int newIndex) { // Note: The isProcessingTabReorder flag should already be set by the caller // before any UI changes are made, to prevent listener interference @@ -894,6 +895,67 @@ public void onTabReordered(int oldIndex, int newIndex) { save(); } + /** + * Handles a tab reorder the plugin did not perform — the platform lets tabs be dragged in the + * tool window header (monolith only; split mode disables tab drag entirely). The drag ends by + * re-adding the content at a new index, which reaches us as a removal, so without this the + * model would be deleted while its tab stayed on screen. + * + *

HEAD and "+" are restored to the ends first: the drag helper cannot be told to leave them + * alone, so the invariant the popup actions enforce up front is enforced here after the fact. + */ + public void onTabsDragged() { + if (isDisposed || toolWindowService == null) return; + + ToolWindow toolWindow = toolWindowService.getToolWindow(); + if (toolWindow == null) return; + ContentManager contentManager = toolWindow.getContentManager(); + + isProcessingTabReorder = true; + try { + restoreSpecialTabPositions(contentManager); + rebuildCollectionFromTabOrder(); + save(); + } finally { + isProcessingTabReorder = false; + } + + // Indices changed, so tooltips and the "can be reset" set no longer address the same tabs. + project.getService(TabActionService.class).publishRenamedTabs(); + } + + /** Moves the HEAD tab back to the front and the "+" tab back to the end if a drag moved them. */ + private void restoreSpecialTabPositions(@NotNull ContentManager contentManager) { + Content headContent = null; + Content plusContent = null; + for (int index = 0; index < contentManager.getContentCount(); index++) { + Content content = contentManager.getContent(index); + if (content == null) continue; + if (PLUS_TAB_LABEL.equals(content.getTabName())) { + plusContent = content; + continue; + } + MyModel model = toolWindowService.getModelForContent(content); + if (model != null && model.isHeadTab()) { + headContent = content; + } + } + + moveContentTo(contentManager, headContent, 0); + moveContentTo(contentManager, plusContent, contentManager.getContentCount() - 1); + } + + private void moveContentTo(@NotNull ContentManager contentManager, Content content, int targetIndex) { + if (content == null) return; + int currentIndex = contentManager.getIndexOfContent(content); + if (currentIndex < 0 || currentIndex == targetIndex) return; + + LOG.debug("Restoring special tab '" + content.getTabName() + "' from index " + currentIndex + + " to " + targetIndex); + contentManager.removeContent(content, false); + contentManager.addContent(content, targetIndex); + } + /** * Rebuilds the collection based on the current tab order in ContentManager. * This ensures that collection[i] corresponds to tab[i+1] (accounting for HEAD at index 0). From b31f67225a267e66bbff4c35b53e6777e6d5f886 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20W=C3=A5llberg?= Date: Sun, 16 Aug 2026 10:23:45 +0200 Subject: [PATCH 11/14] fix: order change navigation by the tool window's display order Next/Previous Change and Next/Previous Changed File visit the changed files in the order the Git Scope tree shows them, read from the tree itself so that whichever grouping is active -- module, repository or directory -- is followed. When the tool window has not built a tree yet, files are ordered hierarchically instead: into a subdirectory before the files beside it, names compared case-insensitively with digit runs read as numbers. Entries that cannot hold a caret are left out of the order rather than failing when opened: files deleted in the scope, directories recorded as changes such as submodules, and binary files. Each opened file is logged with its file type under #gitscope. --- .../java/service/ChangeNavigationService.java | 107 +++++++++++++++++- .../main/java/service/ToolWindowService.java | 21 ++++ .../service/ToolWindowServiceInterface.java | 6 + .../java/toolwindow/elements/VcsTree.java | 24 ++++ .../src/main/java/utils/FileTreeOrder.java | 64 +++++++++++ .../test/java/utils/FileTreeOrderTest.java | 99 ++++++++++++++++ 6 files changed, 315 insertions(+), 6 deletions(-) create mode 100644 backend/src/main/java/utils/FileTreeOrder.java create mode 100644 backend/src/test/java/utils/FileTreeOrderTest.java diff --git a/backend/src/main/java/service/ChangeNavigationService.java b/backend/src/main/java/service/ChangeNavigationService.java index 7b1118c..932d73d 100644 --- a/backend/src/main/java/service/ChangeNavigationService.java +++ b/backend/src/main/java/service/ChangeNavigationService.java @@ -19,6 +19,7 @@ import rpc.ChangeNavDirection; import system.Defs; import utils.FileOpener; +import utils.FileTreeOrder; import java.util.ArrayList; import java.util.Collections; @@ -81,13 +82,15 @@ public void navigate(@Nullable String currentFilePath, int caretLine, @NotNull C return; } - // Ordered, stable list of changed files: the union of scope changes and local (working-tree - // vs HEAD) changes, so navigation visits every file that shows a gutter marker — both the + // Ordered list of changed files: the union of scope changes and local (working-tree vs + // HEAD) changes, so navigation visits every file that shows a gutter marker — both the // scope markers we paint and the local markers the IDE paints. - java.util.TreeSet fileSet = new java.util.TreeSet<>(); - fileSet.addAll(scopeChanges.keySet()); - fileSet.addAll(localChanges.keySet()); - List files = new ArrayList<>(fileSet); + List files = orderedFiles(scopeChanges, localChanges); + if (files.isEmpty()) { + // Everything in the scope is deleted, a directory, or otherwise not openable. + LOG.debug("ChangeNavigation: no navigable files in scope"); + return; + } // Fall back to our last navigated position when the frontend has no focused editor // (or the focused editor isn't one of the changed files). This keeps sequential @@ -270,6 +273,10 @@ private void open(String path, int line) { LOG.debug("ChangeNavigation: could not resolve file " + path); return; } + if (LOG.isDebugEnabled()) { + LOG.debug("ChangeNavigation: opening " + displayPath(path) + + " [" + file.getFileType().getName() + "] at line " + (line + 1)); + } lastNavigatedFile = path; lastNavigatedLine = line; // Request focus so the opened editor becomes the focus owner and the frontend reports the @@ -355,6 +362,94 @@ private List computeRanges(String path, @Nullable Change change, String n } } + // --- file ordering --- + + /** + * The changed files in the order navigation should visit them: the order the Git Scope tree + * displays, when it has one. + * + *

The tree's order depends on its grouping — by module, repository or directory, switchable + * from the toolbar — so it cannot be derived from the paths alone. Grouping by module puts a + * repository-root file such as {@code .gitignore} before the files of a nested source module, + * even though on disk they are siblings; sorting paths produced the opposite, which is what made + * navigation appear to jump around when crossing a file boundary. + * + *

Falls back to plain file-tree ordering when the tool window has not built its tree (never + * opened this session), and appends any changed file the tree does not show, so navigation can + * never silently skip a file that has gutter markers. + * + *

Entries that cannot be opened are left out — see {@link #navigable}. + */ + private List orderedFiles(Map scopeChanges, Map localChanges) { + java.util.TreeSet changedFiles = new java.util.TreeSet<>(FileTreeOrder.INSTANCE); + changedFiles.addAll(scopeChanges.keySet()); + changedFiles.addAll(localChanges.keySet()); + + ToolWindowServiceInterface toolWindowService = project.getService(ToolWindowServiceInterface.class); + List displayed = toolWindowService == null + ? Collections.emptyList() + : toolWindowService.getDisplayOrderedPaths(); + if (displayed.isEmpty()) { + LOG.debug("ChangeNavigation: tree order unavailable, ordering by file tree"); + return navigable(changedFiles); + } + + java.util.LinkedHashSet ordered = new java.util.LinkedHashSet<>(); + for (String path : displayed) { + if (changedFiles.contains(path)) ordered.add(path); + } + ordered.addAll(changedFiles); + return navigable(ordered); + } + + /** + * Keeps only the entries navigation can actually put a caret in. + * + *

A scope contains things that are not text a caret can move through: + *

    + *
  • a file deleted in the scope, which has no content left to show;
    • + *
  • a directory recorded as a change of its own — a submodule, whose changed commit hash + * shows up as a change on the checked-out folder, or a folder added or removed;
    • + *
  • a binary file, which opens in a viewer with no lines to step through.
    • + *
+ * + *

These used to stay in the list, and stepping onto one made navigation stop where it stood, + * because opening resolved nothing and returned without moving; pressing again from the + * unchanged position then jumped somewhere unrelated. + * + *

Dropping them here rather than when opening also keeps the index arithmetic honest: "the + * next file" and the wrap at either end are computed over files that can be reached. + */ + private List navigable(java.util.Collection paths) { + List result = new ArrayList<>(paths.size()); + for (String path : paths) { + VirtualFile file = LocalFileSystem.getInstance().findFileByPath(path); + if (file != null && file.isValid() && !file.isDirectory() && !isBinary(file)) { + result.add(path); + } + } + return result; + } + + /** File type resolution touches the VFS, so never let a failure here abort navigation. */ + private static boolean isBinary(@NotNull VirtualFile file) { + try { + return file.getFileType().isBinary(); + } catch (Exception e) { + LOG.debug("ChangeNavigation: could not determine file type of " + file.getPath(), e); + return false; + } + } + + /** Project-relative path when possible, so the log lines up with the tree. */ + private String displayPath(String path) { + String base = project.getBasePath(); + if (base != null && path.length() > base.length() + 1 && path.startsWith(base)) { + return path.substring(base.length() + 1); + } + return path; + } + // --- small utilities --- /** First line strictly greater than {@code line}, or null if none. */ diff --git a/backend/src/main/java/service/ToolWindowService.java b/backend/src/main/java/service/ToolWindowService.java index f7de2b9..fa8f379 100644 --- a/backend/src/main/java/service/ToolWindowService.java +++ b/backend/src/main/java/service/ToolWindowService.java @@ -1,6 +1,8 @@ package service; import com.intellij.openapi.Disposable; +import com.intellij.openapi.application.ApplicationManager; +import com.intellij.openapi.application.ModalityState; import com.intellij.openapi.components.Service; import com.intellij.openapi.project.Project; import com.intellij.openapi.vfs.VirtualFile; @@ -23,6 +25,8 @@ @Service(Service.Level.PROJECT) public final class ToolWindowService implements ToolWindowServiceInterface, Disposable { + private static final com.intellij.openapi.diagnostic.Logger LOG = Defs.getLogger(ToolWindowService.class); + private final Project project; private final Map contentToViewMap = new HashMap<>(); private final TabOperations tabOperations; @@ -191,6 +195,23 @@ public void selectFile(VirtualFile file) { } } + @Override + public java.util.List getDisplayOrderedPaths() { + // The tree is a Swing component, so reading its model belongs on the EDT; callers (change + // navigation) run on background threads and should not have to know that. + java.util.List paths = new java.util.ArrayList<>(); + ApplicationManager.getApplication().invokeAndWait(() -> { + if (project.isDisposed()) return; + try { + VcsTree vcsTree = getVcsTree(); + if (vcsTree != null) paths.addAll(vcsTree.getDisplayOrderedPaths()); + } catch (Exception e) { + LOG.debug("Could not read the tool window's display order", e); + } + }, ModalityState.any()); + return paths; + } + @Override public MyModel getModelForContent(Content content) { ToolWindowView toolWindowView = contentToViewMap.get(content); diff --git a/backend/src/main/java/service/ToolWindowServiceInterface.java b/backend/src/main/java/service/ToolWindowServiceInterface.java index 6c9669e..f4bae39 100644 --- a/backend/src/main/java/service/ToolWindowServiceInterface.java +++ b/backend/src/main/java/service/ToolWindowServiceInterface.java @@ -30,6 +30,12 @@ public interface ToolWindowServiceInterface { void selectFile(VirtualFile file); + /** + * Paths of the displayed changes in the order the tool window shows them, or an empty list when + * it has no tree yet. Safe to call from any thread. + */ + java.util.List getDisplayOrderedPaths(); + ToolWindow getToolWindow(); MyModel getModelForContent(com.intellij.ui.content.Content content); diff --git a/backend/src/main/java/toolwindow/elements/VcsTree.java b/backend/src/main/java/toolwindow/elements/VcsTree.java index 5ec7d71..6bfc878 100644 --- a/backend/src/main/java/toolwindow/elements/VcsTree.java +++ b/backend/src/main/java/toolwindow/elements/VcsTree.java @@ -95,6 +95,30 @@ public void onTabSwitched() { }); } + /** + * Paths of the displayed changes, in the order the tree shows them. + * + *

Change navigation follows this rather than sorting paths itself, because the order depends + * on the tree's grouping — by module, repository or directory, switchable from the toolbar — and + * only the tree knows which is active. Grouping by module, for example, puts a repository-root + * file after the files of a nested source module, which no path comparison would reproduce. + * + *

Returns an empty list when the tool window has not built its tree yet; navigation then + * falls back to plain file-tree ordering. + */ + public List getDisplayOrderedPaths() { + if (currentBrowser == null) return Collections.emptyList(); + + List paths = new ArrayList<>(); + // traverse() is a pre-order DFS of the tree model, i.e. exactly the displayed order. + for (Change change : com.intellij.openapi.vcs.changes.ui.VcsTreeModelData + .all(currentBrowser.getViewer()) + .userObjects(Change.class)) { + paths.add(ChangesUtil.getFilePath(change).getPath()); + } + return paths; + } + public void selectFile(VirtualFile file) { if (currentBrowser == null) { return; } List changes = currentBrowser.getAllChanges(); diff --git a/backend/src/main/java/utils/FileTreeOrder.java b/backend/src/main/java/utils/FileTreeOrder.java new file mode 100644 index 0000000..9f960bf --- /dev/null +++ b/backend/src/main/java/utils/FileTreeOrder.java @@ -0,0 +1,64 @@ +package utils; + +import com.intellij.ide.util.treeView.FileNameComparator; +import org.jetbrains.annotations.NotNull; + +import java.util.Comparator; + +/** + * Orders file paths the way a file tree shows them: walking down into a directory before moving on + * to the next entry of the level above, subdirectories before files within each directory, and + * names compared naturally — case-insensitively, with digit runs read as numbers. + * + *

Change navigation uses this so stepping past the last change of a file lands on the file the + * Git Scope tree shows next. Sorting the absolute paths as plain strings instead — which is what + * navigation used to do — produces an order that has little to do with the tree: uppercase names + * sort before all lowercase ones, and a file sorts among the contents of a sibling directory + * whenever their names share a prefix. + * + *

This mirrors the platform's own {@code HierarchicalFilePathComparator.NATURAL}, which the + * changes tree sorts with. It is reimplemented here rather than called because that class is + * marked internal, while the name comparison it delegates to is not; comparing paths as strings + * also keeps this testable without an IDE. + */ +public final class FileTreeOrder implements Comparator { + + public static final FileTreeOrder INSTANCE = new FileTreeOrder(); + + private FileTreeOrder() {} + + @Override + public int compare(@NotNull String path1, @NotNull String path2) { + int start = 0; + while (true) { + int end1 = path1.indexOf('/', start); + int end2 = path2.indexOf('/', start); + + // A segment with more path after it is a directory; the last segment is the file. + boolean isDirectory1 = end1 != -1; + boolean isDirectory2 = end2 != -1; + if (isDirectory1 != isDirectory2) { + // Same level, one descends and one does not: directories come first, whatever the + // names are. This is what makes navigation walk a directory to its end before + // continuing with the files beside it. + return isDirectory1 ? -1 : 1; + } + + String name1 = isDirectory1 ? path1.substring(start, end1) : path1.substring(start); + String name2 = isDirectory2 ? path2.substring(start, end2) : path2.substring(start); + + int byName = FileNameComparator.getInstance().compare(name1, name2); + if (byName != 0) return byName; + + if (!isDirectory1) { + // Same name and both are the final segment: identical paths apart from case, which + // the natural comparison ignores. Fall back to an exact comparison so the order + // stays stable rather than reporting two distinct files as equal. + return path1.compareTo(path2); + } + + // Same directory name: continue with the next level. + start = end1 + 1; + } + } +} diff --git a/backend/src/test/java/utils/FileTreeOrderTest.java b/backend/src/test/java/utils/FileTreeOrderTest.java new file mode 100644 index 0000000..d3678e2 --- /dev/null +++ b/backend/src/test/java/utils/FileTreeOrderTest.java @@ -0,0 +1,99 @@ +package utils; + +import org.junit.Test; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +public class FileTreeOrderTest { + + private static List sorted(String... paths) { + List list = new ArrayList<>(Arrays.asList(paths)); + list.sort(FileTreeOrder.INSTANCE); + return list; + } + + @Test + public void walksSubdirectoriesBeforeFilesBesideThem() { + assertEquals( + Arrays.asList( + "/p/src/sub/a.java", + "/p/src/sub/b.java", + "/p/src/aaa.java", + "/p/src/zzz.java"), + sorted( + "/p/src/zzz.java", + "/p/src/sub/b.java", + "/p/src/aaa.java", + "/p/src/sub/a.java")); + } + + @Test + public void ordersNamesCaseInsensitively() { + // Plain string ordering puts every capitalised name first, which is what made navigation + // look erratic: Zebra.java would come before apple.java. + assertEquals( + Arrays.asList("/p/apple.java", "/p/Banana.java", "/p/Zebra.java"), + sorted("/p/Zebra.java", "/p/apple.java", "/p/Banana.java")); + } + + @Test + public void ordersDigitRunsNumerically() { + assertEquals( + Arrays.asList("/p/file2.java", "/p/file10.java"), + sorted("/p/file10.java", "/p/file2.java")); + } + + @Test + public void keepsSiblingDirectoriesSeparate() { + // "/p/a.java" must not land between the contents of "/p/a/", which is what happens when + // full paths are compared as strings ('.' sorts before '/'). + assertEquals( + Arrays.asList( + "/p/a/one.java", + "/p/a/two.java", + "/p/b/one.java", + "/p/a.java"), + sorted( + "/p/a.java", + "/p/b/one.java", + "/p/a/two.java", + "/p/a/one.java")); + } + + @Test + public void ordersDeepAndShallowConsistently() { + assertEquals( + Arrays.asList( + "/p/src/main/java/App.java", + "/p/src/main/resources/app.xml", + "/p/src/test/AppTest.java", + "/p/src/build.gradle", + "/p/README.md"), + sorted( + "/p/README.md", + "/p/src/build.gradle", + "/p/src/test/AppTest.java", + "/p/src/main/resources/app.xml", + "/p/src/main/java/App.java")); + } + + @Test + public void isAntisymmetricAndStable() { + String a = "/p/src/sub/a.java"; + String b = "/p/src/b.java"; + assertTrue(FileTreeOrder.INSTANCE.compare(a, b) < 0); + assertTrue(FileTreeOrder.INSTANCE.compare(b, a) > 0); + assertEquals(0, FileTreeOrder.INSTANCE.compare(a, a)); + } + + @Test + public void distinguishesPathsDifferingOnlyByCase() { + // Natural comparison reports these equal; a TreeSet would then drop one of the two files. + assertTrue(FileTreeOrder.INSTANCE.compare("/p/File.java", "/p/file.java") != 0); + } +} From 1df7aa3caf91547f4bc3a17200a7221b7f5b750e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20W=C3=A5llberg?= Date: Sun, 16 Aug 2026 11:06:57 +0200 Subject: [PATCH 12/14] fix: keep the focus in Git Scope when navigating between changes Next/Previous Change and Next/Previous Changed File asked for the opened editor to be focused. When navigation is driven from the tool window that hands the focus away, and Project View following the file leaves it there, so the next keystroke no longer reaches Git Scope. The editor is now focused only when navigation started from the editor; the caret position it would have reported is already tracked as the last navigated position. The changes tree keeps painting its selection as focused, so the file navigation moved to stays visible in the tool window while the editor holds the focus. --- .../java/service/ChangeNavigationService.java | 27 +++++++++++++--- .../main/java/service/ToolWindowService.java | 31 +++++++++++++++++++ .../service/ToolWindowServiceInterface.java | 12 +++++++ .../elements/MySimpleChangesBrowser.java | 7 +++++ 4 files changed, 72 insertions(+), 5 deletions(-) diff --git a/backend/src/main/java/service/ChangeNavigationService.java b/backend/src/main/java/service/ChangeNavigationService.java index 932d73d..e0dd866 100644 --- a/backend/src/main/java/service/ChangeNavigationService.java +++ b/backend/src/main/java/service/ChangeNavigationService.java @@ -279,15 +279,32 @@ private void open(String path, int line) { } lastNavigatedFile = path; lastNavigatedLine = line; - // Request focus so the opened editor becomes the focus owner and the frontend reports the - // correct caret position on the next navigation keystroke. - FileOpener.openAndGoToLine(project, file, line, true); - highlightInToolWindow(file); + + // Stepping through changes from the tool window should leave the focus there. Asking for the + // opened editor to be focused takes it away, and on Linux the Project View following the + // file ("Autoscroll from Source") can then keep it, so the next keystroke no longer reaches + // the tool window at all. Focusing the editor is only what we want when navigation was + // triggered from the editor in the first place. + // + // Dropping the focus request costs nothing: the frontend then reports no focused changed + // file, and navigate() continues from lastNavigatedFile/lastNavigatedLine instead. + ToolWindowServiceInterface toolWindowService = project.getService(ToolWindowServiceInterface.class); + boolean startedInToolWindow = toolWindowService != null && toolWindowService.isFocused(); + + FileOpener.openAndGoToLine(project, file, line, !startedInToolWindow); + highlightInToolWindow(toolWindowService, file); + if (startedInToolWindow) { + toolWindowService.restoreFocus(); + } } /** Selects/highlights the file's change in the Git Scope tool window tree, so the tree stays in sync. */ private void highlightInToolWindow(@NotNull VirtualFile file) { - ToolWindowServiceInterface toolWindowService = project.getService(ToolWindowServiceInterface.class); + highlightInToolWindow(project.getService(ToolWindowServiceInterface.class), file); + } + + private void highlightInToolWindow(@Nullable ToolWindowServiceInterface toolWindowService, + @NotNull VirtualFile file) { if (toolWindowService == null) return; ApplicationManager.getApplication().invokeLater(() -> { if (project.isDisposed()) return; diff --git a/backend/src/main/java/service/ToolWindowService.java b/backend/src/main/java/service/ToolWindowService.java index fa8f379..95f0d6a 100644 --- a/backend/src/main/java/service/ToolWindowService.java +++ b/backend/src/main/java/service/ToolWindowService.java @@ -212,6 +212,37 @@ public java.util.List getDisplayOrderedPaths() { return paths; } + @Override + public boolean isFocused() { + // isActive() reads window-manager state that is only consistent on the EDT; callers (change + // navigation) run on background threads and should not have to know that. + boolean[] focused = {false}; + ApplicationManager.getApplication().invokeAndWait(() -> { + if (project.isDisposed()) return; + ToolWindow toolWindow = getToolWindow(); + focused[0] = toolWindow != null && toolWindow.isVisible() && toolWindow.isActive(); + }, ModalityState.any()); + return focused[0]; + } + + @Override + public void restoreFocus() { + // Queued behind the file open the caller scheduled, so the check below sees the focus as it + // ends up rather than as it was. Focus transfers the window manager owns -- Project View + // being shown next to us on Linux, say -- are asynchronous and can land later still; there + // is no way to wait for those, which is why callers avoid moving the focus in the first + // place instead of relying on this. + ApplicationManager.getApplication().invokeLater(() -> { + if (project.isDisposed()) return; + ToolWindow toolWindow = getToolWindow(); + if (toolWindow == null || !toolWindow.isVisible() || toolWindow.isActive()) return; + LOG.debug("Returning focus to the Git Scope tool window"); + // forced=false, so a real user action that moved the focus elsewhere in the meantime + // keeps it instead of being overridden. + toolWindow.activate(null, true, false); + }, ModalityState.any()); + } + @Override public MyModel getModelForContent(Content content) { ToolWindowView toolWindowView = contentToViewMap.get(content); diff --git a/backend/src/main/java/service/ToolWindowServiceInterface.java b/backend/src/main/java/service/ToolWindowServiceInterface.java index f4bae39..181646a 100644 --- a/backend/src/main/java/service/ToolWindowServiceInterface.java +++ b/backend/src/main/java/service/ToolWindowServiceInterface.java @@ -36,6 +36,18 @@ public interface ToolWindowServiceInterface { */ java.util.List getDisplayOrderedPaths(); + /** + * Whether the Git Scope tool window is the one currently holding the focus. Safe to call from + * any thread. + */ + boolean isFocused(); + + /** + * Returns the focus to the Git Scope tool window if something took it in the meantime. Does + * nothing when the tool window is hidden or still focused. Safe to call from any thread. + */ + void restoreFocus(); + ToolWindow getToolWindow(); MyModel getModelForContent(com.intellij.ui.content.Content content); diff --git a/backend/src/main/java/toolwindow/elements/MySimpleChangesBrowser.java b/backend/src/main/java/toolwindow/elements/MySimpleChangesBrowser.java index 11d040e..59ccbe0 100644 --- a/backend/src/main/java/toolwindow/elements/MySimpleChangesBrowser.java +++ b/backend/src/main/java/toolwindow/elements/MySimpleChangesBrowser.java @@ -21,6 +21,7 @@ import com.intellij.openapi.vcs.changes.ui.ChangeDiffRequestChain; import com.intellij.openapi.vcs.changes.ui.SimpleAsyncChangesBrowser; import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.ui.render.RenderingUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import system.Defs; @@ -67,6 +68,12 @@ private MySimpleChangesBrowser(@NotNull Project project, @NotNull Collection Date: Sun, 16 Aug 2026 11:14:53 +0200 Subject: [PATCH 13/14] chore: bump the plugin version to 2026.2.1 Adds the changelog section for the release. The common-ancestor fix moves from the 2026.2 section, which was already published without it. --- CHANGELOG.md | 18 +++++++++++++++--- gradle.properties | 2 +- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 943d451..892bf8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,18 @@ +## [2026.2.1] + +### Fixes + +- Fixed ["Only Changes Since Common Ancestor" collected every commit in the range instead of the pull-request diff](https://github.com/comod/git-scope-pro/issues/104) + - The scope is now `git diff ...HEAD`: files reverted within the range disappear, and each + file's diff, gutter markers and rollback use the merge base rather than a single intermediate commit. +- Fixed [Files are red even after resolving conflicts](https://github.com/comod/git-scope-pro/issues/78) +- Fixed missing gutter markers over slow or unstable Remote Development connections +- Fixed the scope tab context menu (rename, reset, move left/right) missing in Remote Development, and the tooltip + showing the original name of a renamed tab +- Fixed reordering scope tabs by dragging them, which mixed up the tabs and lost one on the next IDE start +- Fixed the step-between-changes actions jumping erratically: they now follow the order the scope tree shows, skip + deleted, binary and directory entries, and keep the focus in the Git Scope window + ## [2026.2] Remote Development support, built on a split of the plugin into separate frontend and backend modules. Local IDE @@ -18,9 +33,6 @@ locally or over Remote Development. - Fixed [Right-click -> Show in Project doesn't switch to Project window](https://github.com/comod/git-scope-pro/issues/100) - Fixed [Right-click -> Show in Project for folder selects the first changed file rather than the folder](https://github.com/comod/git-scope-pro/issues/101) -- Fixed ["Only Changes Since Common Ancestor" collected every commit in the range instead of the pull-request diff](https://github.com/comod/git-scope-pro/issues/104) - - The scope is now `git diff ...HEAD`: files reverted within the range disappear, and each - file's diff, gutter markers and rollback use the merge base rather than a single intermediate commit. ## [2026.1.4] diff --git a/gradle.properties b/gradle.properties index 421bd6f..eb84dd0 100644 --- a/gradle.properties +++ b/gradle.properties @@ -2,7 +2,7 @@ pluginGroup=org.woelkit.plugins pluginName=Git Scope pluginRepositoryUrl=https://github.com/comod/git-scope-pro -pluginVersion=2026.2 +pluginVersion=2026.2.1 pluginSinceBuild=261 platformType=IU From cebee29c4e4b9af8bc6aad2d0cf6a7b8682e3703 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20W=C3=A5llberg?= Date: Fri, 11 Sep 2026 00:19:35 +0200 Subject: [PATCH 14/14] fix: force a full changelist rescan when VCS becomes ready ChangeListManager restores the changelist persisted in workspace.xml and then only updates incrementally, over whatever VcsDirtyScopeManager reports as dirty. An entry whose file no longer exists can never enter that dirty scope, so it survives every update, is re-persisted on close and returns on the next open. Restarting does not clear it; the scope kept showing files git reports as clean until an unrelated commit touched .git/index and forced a rescan. Mark everything dirty once per project on VcsMappingListener readiness, collect behind ChangeListManager.invokeAfterUpdate, drop changelist entries whose file no longer exists (DELETED exempt), and route untracked files through the same repository and staleness checks. --- CHANGELOG.md | 5 ++ .../compare/ChangesService.java | 51 ++++++++++++++----- .../src/main/java/service/ViewService.java | 22 ++++++++ 3 files changed, 66 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 892bf8e..d35209e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,11 @@ - Fixed reordering scope tabs by dragging them, which mixed up the tabs and lost one on the next IDE start - Fixed the step-between-changes actions jumping erratically: they now follow the order the scope tree shows, skip deleted, binary and directory entries, and keep the focus in the Git Scope window +- Fixed the scope listing files that Git reports as unchanged, which survived IDE restarts and only cleared once an + unrelated commit happened to force a refresh + - The IDE restores its changelist on startup and afterwards only re-checks files it has marked as changed, so an + entry whose file no longer exists was never looked at again. Git Scope now forces one full rescan once Git is + ready, and ignores entries whose file is gone. ## [2026.2] diff --git a/backend/src/main/java/implementation/compare/ChangesService.java b/backend/src/main/java/implementation/compare/ChangesService.java index d269b1a..313ad6a 100644 --- a/backend/src/main/java/implementation/compare/ChangesService.java +++ b/backend/src/main/java/implementation/compare/ChangesService.java @@ -270,7 +270,19 @@ public void onThrowable(@NotNull Throwable error) { if (prev != null) { prev.cancel(); } - task.queue(); + + // Collect against a settled changelist. ChangeListManager restores the list persisted in + // workspace.xml when the project opens and refreshes it only afterwards, so reading + // getAllChanges() straight away can hand back entries for files git considers clean — or + // that no longer exist at that path at all. Capture the task locally: a newer collection + // reassigns the field before this callback runs. + final Task.Backgroundable queuedTask = task; + ChangeListManager.getInstance(currentProject).invokeAfterUpdate(false, () -> { + if (disposing.get() || currentProject.isDisposed()) { + return; + } + queuedTask.queue(); + }); } @Override @@ -311,7 +323,13 @@ private Collection filterLocalChanges(Collection localChanges, S FilePath changePath = ChangesUtil.getFilePath(change); String changePathStr = changePath.getPath(); - if (!showDeletedFiles && change.getType() == Change.Type.DELETED) { + if (change.getType() == Change.Type.DELETED) { + if (!showDeletedFiles) { + continue; + } + } else if (!isPresentOnDisk(changePath)) { + // Stale changelist entry: nothing lives at this path any more, so rendering it would + // put a dead node in the tree. DELETED is exempt because absence is what it reports. continue; } @@ -341,6 +359,17 @@ private Collection filterLocalChanges(Collection localChanges, S return filtered; } + /** + * Reports whether a change still has a live file behind it. + * + *

A {@link VirtualFile} can outlive the file it points at when the deletion happened outside + * the IDE, so validity is checked alongside presence. + */ + private static boolean isPresentOnDisk(FilePath path) { + VirtualFile virtualFile = path.getVirtualFile(); + return virtualFile != null && virtualFile.isValid(); + } + /** * Collects local changes for HEAD (uncommitted changes) filtered by repository. * @@ -372,23 +401,21 @@ public RepoChangesResult doCollectChanges(Project project, GitRepository repo, S try { // Local Changes ChangeListManager changeListManager = ChangeListManager.getInstance(project); - Collection localChanges = changeListManager.getAllChanges(); + Collection localChanges = new ArrayList<>(changeListManager.getAllChanges()); String repoPath = repo.getRoot().getPath(); - // Filter local changes for this repository - repoLocalChanges = filterLocalChanges(localChanges, repoPath, null); - - // Add unversioned (untracked) files if the setting is enabled + // Add unversioned (untracked) files if the setting is enabled. They join the changelist + // entries *before* filtering so they get the same repository and staleness checks — + // appending them afterwards let untracked paths bypass both. if (GitScopeSettings.getInstance().isShowUntrackedFiles()) { for (FilePath unversionedPath : changeListManager.getUnversionedFilesPaths()) { - String filePathStr = unversionedPath.getPath(); - if (filePathStr.startsWith(repoPath)) { - Change untrackedChange = new Change(null, new CurrentContentRevision(unversionedPath), FileStatus.UNKNOWN); - repoLocalChanges.add(untrackedChange); - } + localChanges.add(new Change(null, new CurrentContentRevision(unversionedPath), FileStatus.UNKNOWN)); } } + // Filter local changes for this repository + repoLocalChanges = filterLocalChanges(localChanges, repoPath, null); + // Special handling for HEAD - return local changes only, no scope changes if (scopeRef.equals(GitService.BRANCH_HEAD)) { return new RepoChangesResult(new ArrayList<>(repoLocalChanges), new ArrayList<>(), repoLocalChanges); diff --git a/backend/src/main/java/service/ViewService.java b/backend/src/main/java/service/ViewService.java index 5a0eb56..4797f81 100644 --- a/backend/src/main/java/service/ViewService.java +++ b/backend/src/main/java/service/ViewService.java @@ -11,6 +11,7 @@ import com.intellij.openapi.vcs.FileStatusManager; import com.intellij.openapi.vcs.VcsApplicationSettings; import com.intellij.openapi.vcs.changes.Change; +import com.intellij.openapi.vcs.changes.VcsDirtyScopeManager; import com.intellij.openapi.vcs.impl.LineStatusTrackerManagerI; import com.intellij.openapi.wm.ToolWindow; import com.intellij.ui.content.Content; @@ -80,6 +81,7 @@ private static class DisposalToken { private Integer savedTabIndex; private final AtomicBoolean tabInitializationInProgress = new AtomicBoolean(false); private final AtomicBoolean initialFileColorsRefreshed = new AtomicBoolean(false); + private final AtomicBoolean initialChangelistRefreshed = new AtomicBoolean(false); private final Map> modelListeners = new HashMap<>(); public ViewService(Project project) { @@ -342,9 +344,29 @@ public void save() { public void eventVcsReady() { this.vcsReady = true; + forceInitialChangelistRefresh(); init(); } + /** + * Forces one full changelist rescan per project, as soon as VCS is usable. + * + *

ChangeListManager restores the changelist persisted in workspace.xml and from then on only + * updates incrementally, over whatever VcsDirtyScopeManager reports as dirty. An entry whose + * file no longer exists can never enter that dirty scope — no VFS event can fire for a file that + * is not there — so it survives every update, is re-persisted on close, and returns on the next + * open. Restarting the IDE does not clear it; only a full rescan does, which is why such an + * entry otherwise lingers until something unrelated (a commit touching .git/index) forces one. + */ + private void forceInitialChangelistRefresh() { + if (project.isDisposed() || !initialChangelistRefreshed.compareAndSet(false, true)) { + // directoryMappingChanged() fires again whenever mappings change; one rescan is enough. + return; + } + LOG.debug("Forcing a full changelist rescan to discard entries restored from workspace.xml"); + VcsDirtyScopeManager.getInstance(project).markEverythingDirty(); + } + public void eventToolWindowReady() { this.toolWindowReady = true; init();