From 65adec99512ecf733be0149d7da5199636a24cb9 Mon Sep 17 00:00:00 2001 From: "Gregory.Shrago" Date: Fri, 15 Mar 2024 20:41:52 +0400 Subject: [PATCH] Revert "drop explicit synchronous group traversals in EDT" This reverts commit 1a714ced20105958d8745f2ba65dccb752ef5755. GitOrigin-RevId: 7cf12e069cc0f90fd17ccc73ca0a2bc20925c33a --- .../diff/impl/DiffRequestProcessor.java | 10 +-- .../diff/merge/MergeRequestProcessor.java | 6 +- .../tools/combined/CombinedDiffMainToolbar.kt | 9 ++- .../src/com/intellij/diff/util/DiffUtil.java | 42 ---------- .../com/intellij/dvcs/push/ui/PushLog.java | 2 +- .../vcs/changes/ui/ChangesBrowserBase.java | 77 ++++++++----------- 6 files changed, 46 insertions(+), 100 deletions(-) diff --git a/platform/diff-impl/src/com/intellij/diff/impl/DiffRequestProcessor.java b/platform/diff-impl/src/com/intellij/diff/impl/DiffRequestProcessor.java index 2d4bac803f1a..437f0b45909f 100644 --- a/platform/diff-impl/src/com/intellij/diff/impl/DiffRequestProcessor.java +++ b/platform/diff-impl/src/com/intellij/diff/impl/DiffRequestProcessor.java @@ -81,6 +81,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import static com.intellij.diff.util.DiffUtil.recursiveRegisterShortcutSet; import static com.intellij.util.ObjectUtils.chooseNotNull; /** @@ -187,9 +188,6 @@ public abstract class DiffRequestProcessor implements DiffEditorViewer, CheckedD myRightToolbar.setLayoutStrategy(ToolbarLayoutStrategy.NOWRAP_STRATEGY); myRightToolbar.setTargetComponent(myMainPanel); - DiffUtil.keepToolbarActionsPromoted(myToolbar); - DiffUtil.keepToolbarActionsPromoted(myRightToolbar); - myRightToolbarWrapper = new Wrapper(JBUI.Panels.simplePanel(myRightToolbar.getComponent())); myPanel = JBUI.Panels.simplePanel(myMainPanel); @@ -630,11 +628,13 @@ public abstract class DiffRequestProcessor implements DiffEditorViewer, CheckedD collectToolbarActions(viewerActions); ((ActionToolbarImpl)myToolbar).reset(); // do not leak previous DiffViewer via caches - myToolbar.updateActionsAsync(); + myToolbar.updateActionsImmediately(); + recursiveRegisterShortcutSet(myToolbarGroup, myMainPanel, null); if (myIsNewToolbar) { ((ActionToolbarImpl)myRightToolbar).reset(); - myRightToolbar.updateActionsAsync(); + myRightToolbar.updateActionsImmediately(); + recursiveRegisterShortcutSet(myRightToolbarGroup, myMainPanel, null); } } diff --git a/platform/diff-impl/src/com/intellij/diff/merge/MergeRequestProcessor.java b/platform/diff-impl/src/com/intellij/diff/merge/MergeRequestProcessor.java index 3ce8d316aa43..cef3af630c54 100644 --- a/platform/diff-impl/src/com/intellij/diff/merge/MergeRequestProcessor.java +++ b/platform/diff-impl/src/com/intellij/diff/merge/MergeRequestProcessor.java @@ -50,6 +50,8 @@ import java.awt.*; import java.util.Arrays; import java.util.List; +import static com.intellij.diff.util.DiffUtil.recursiveRegisterShortcutSet; + // TODO: support merge request chains // idea - to keep in memory all viewers that were modified (so binary conflict is not the case and OOM shouldn't be too often) // suspend() / resume() methods for viewers? To not interfere with MergeRequest lifecycle: single request -> single viewer -> single applyResult() @@ -259,10 +261,10 @@ public abstract class MergeRequestProcessor implements Disposable { toolbar.setShowSeparatorTitles(true); DataManager.registerDataProvider(toolbar.getComponent(), myMainPanel); - toolbar.setTargetComponent(myMainPanel); + toolbar.setTargetComponent(toolbar.getComponent()); myToolbarPanel.setContent(toolbar.getComponent()); - DiffUtil.keepToolbarActionsPromoted(toolbar); + recursiveRegisterShortcutSet(group, myMainPanel, null); } @NotNull diff --git a/platform/diff-impl/src/com/intellij/diff/tools/combined/CombinedDiffMainToolbar.kt b/platform/diff-impl/src/com/intellij/diff/tools/combined/CombinedDiffMainToolbar.kt index 554ae3aa96a2..00b07062a613 100644 --- a/platform/diff-impl/src/com/intellij/diff/tools/combined/CombinedDiffMainToolbar.kt +++ b/platform/diff-impl/src/com/intellij/diff/tools/combined/CombinedDiffMainToolbar.kt @@ -68,8 +68,6 @@ internal class CombinedDiffMainToolbar( rightToolbar.component.border = JBUI.Borders.empty() rightToolbarPanel = Centerizer(rightToolbar.component, Centerizer.TYPE.VERTICAL) - DiffUtil.keepToolbarActionsPromoted(leftToolbar) - DiffUtil.keepToolbarActionsPromoted(rightToolbar) GuiUtils.installVisibilityReferent(topPanel, leftToolbar.component) GuiUtils.installVisibilityReferent(topPanel, rightToolbar.component) configureTopPanelForActionsMode() @@ -145,9 +143,12 @@ internal class CombinedDiffMainToolbar( fun updateToolbar(blockState: BlockState, toolbarActions: List?) { collectToolbarActions(blockState, toolbarActions) (leftToolbar as ActionToolbarImpl).reset() - leftToolbar.updateActionsAsync() + leftToolbar.updateActionsImmediately() + DiffUtil.recursiveRegisterShortcutSet(leftToolbarGroup, targetComponent, null) (rightToolbar as ActionToolbarImpl).reset() - rightToolbar.updateActionsAsync() + rightToolbar.updateActionsImmediately() + + DiffUtil.recursiveRegisterShortcutSet(rightToolbarGroup, targetComponent, null) } private fun collectToolbarActions(blockState: BlockState, viewerActions: List?) { diff --git a/platform/diff-impl/src/com/intellij/diff/util/DiffUtil.java b/platform/diff-impl/src/com/intellij/diff/util/DiffUtil.java index 40ad5f4e9b8a..dc48208d3966 100644 --- a/platform/diff-impl/src/com/intellij/diff/util/DiffUtil.java +++ b/platform/diff-impl/src/com/intellij/diff/util/DiffUtil.java @@ -30,8 +30,6 @@ import com.intellij.ide.GeneralSettings; import com.intellij.lang.Language; import com.intellij.openapi.Disposable; import com.intellij.openapi.actionSystem.*; -import com.intellij.openapi.actionSystem.ex.CustomComponentAction; -import com.intellij.openapi.actionSystem.impl.ActionButton; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.application.ModalityState; import com.intellij.openapi.application.ReadAction; @@ -114,8 +112,6 @@ import javax.swing.border.Border; import javax.swing.event.HyperlinkEvent; import javax.swing.event.HyperlinkListener; import java.awt.*; -import java.awt.event.ContainerEvent; -import java.awt.event.ContainerListener; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; @@ -464,44 +460,6 @@ public final class DiffUtil { action.registerCustomShortcutSet(action.getShortcutSet(), component); } - public static void keepToolbarActionsPromoted(@NotNull ActionToolbar toolbar) { - JComponent toolbarTargetComponent = toolbar.getTargetComponent(); - if (toolbarTargetComponent == null) { - throw new AssertionError("Toolbar target component is not set"); - } - ContainerListener listener = new ContainerListener() { - @Nullable - AnAction getAction(@NotNull ContainerEvent e) { - Component child = e.getChild(); - return child instanceof ActionButton ab ? ab.getAction() : - ClientProperty.get(child, CustomComponentAction.ACTION_KEY); - } - - @Override - public void componentAdded(@NotNull ContainerEvent e) { - AnAction action = getAction(e); - if (action != null) { - action.registerCustomShortcutSet(toolbarTargetComponent, null); - } - } - - @Override - public void componentRemoved(@NotNull ContainerEvent e) { - AnAction action = getAction(e); - if (action != null) { - action.unregisterCustomShortcutSet(toolbarTargetComponent); - } - } - }; - JComponent toolbarComponent = toolbar.getComponent(); - for (Component child : toolbarComponent.getComponents()) { - listener.componentAdded(new ContainerEvent(toolbarComponent, ContainerEvent.COMPONENT_ADDED, child)); - } - toolbarComponent.addContainerListener(listener); - } - - /** @deprecated Use {@link #keepToolbarActionsPromoted(ActionToolbar)} instead */ - @Deprecated(forRemoval = true) public static void recursiveRegisterShortcutSet(@NotNull ActionGroup group, @NotNull JComponent component, @Nullable Disposable parentDisposable) { diff --git a/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/PushLog.java b/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/PushLog.java index 9f38a228cc2c..132cf900c15e 100644 --- a/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/PushLog.java +++ b/platform/dvcs-impl/src/com/intellij/dvcs/push/ui/PushLog.java @@ -259,7 +259,7 @@ public final class PushLog extends JPanel implements Disposable, DataProvider { detailsSplitter.setSecondComponent(state ? detailsContentPanel : null); }); myShowDetailsAction.setEnabled(false); - myChangesBrowser.addToolbarAction(Separator.getInstance()); + myChangesBrowser.addToolbarSeparator(); myChangesBrowser.addToolbarAction(myShowDetailsAction); JBSplitter splitter = new OnePixelSplitter(TREE_SPLITTER_PROPORTION, 0.5f); diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ui/ChangesBrowserBase.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ui/ChangesBrowserBase.java index 2c465f0942dc..420bfcc41c96 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ui/ChangesBrowserBase.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ui/ChangesBrowserBase.java @@ -6,7 +6,7 @@ import com.intellij.diff.DiffManager; import com.intellij.diff.chains.DiffRequestChain; import com.intellij.diff.util.DiffUserDataKeys; import com.intellij.diff.util.DiffUtil; -import com.intellij.ide.actions.CollapseAllAction; +import com.intellij.ide.actions.NewActionGroup; import com.intellij.openapi.ListSelection; import com.intellij.openapi.actionSystem.*; import com.intellij.openapi.actionSystem.ex.ActionUtil; @@ -35,6 +35,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.function.Predicate; /** * Consider using {@link AsyncChangesBrowserBase} to avoid potentially-expensive tree building operations on EDT. @@ -47,8 +48,8 @@ public abstract class ChangesBrowserBase extends JPanel implements DataProvider protected final ChangesTree myViewer; - private final List myAdditionalToolbarActions = new ArrayList<>(); - + private final DefaultActionGroup myToolBarGroup = new DefaultActionGroup(); + private final DefaultActionGroup myPopupMenuGroup = new DefaultActionGroup(); private final ActionToolbar myToolbar; private final int myToolbarAnchor; private final JScrollPane myViewerScrollPane; @@ -64,11 +65,13 @@ public abstract class ChangesBrowserBase extends JPanel implements DataProvider myProject = project; myViewer = createTreeList(project, showCheckboxes, highlightProblems); - myToolbar = ActionManager.getInstance().createActionToolbar("ChangesBrowser", new DefaultActionGroup(), true); + myToolbar = ActionManager.getInstance().createActionToolbar("ChangesBrowser", myToolBarGroup, true); myToolbar.setTargetComponent(this); myToolbarAnchor = getToolbarAnchor(); myToolbar.setOrientation(isVerticalToolbar() ? SwingConstants.VERTICAL : SwingConstants.HORIZONTAL); + myViewer.installPopupHandler(myPopupMenuGroup); + myViewerScrollPane = ScrollPaneFactory.createScrollPane(myViewer, true); setViewerBorder(createViewerBorder()); @@ -103,51 +106,27 @@ public abstract class ChangesBrowserBase extends JPanel implements DataProvider add(topPanel, BorderLayout.NORTH); add(createCenterPanel(), BorderLayout.CENTER); - ActionGroup toolbarGroup = new ActionGroup() { - final List actions = createToolbarActions(); - final AnAction groupBy = ActionManager.getInstance().getAction(ChangesTree.GROUP_BY_ACTION_GROUP); + myToolBarGroup.addAll(createToolbarActions()); + myPopupMenuGroup.addAll(createPopupMenuActions()); - @Override - public AnAction @NotNull [] getChildren(@Nullable AnActionEvent e) { - if (e == null) return AnAction.EMPTY_ARRAY; - e.getUpdateSession().presentation(groupBy); - return ContainerUtil.concat(actions, myAdditionalToolbarActions).toArray(AnAction.EMPTY_ARRAY); + AnAction groupByAction = ActionManager.getInstance().getAction(ChangesTree.GROUP_BY_ACTION_GROUP); + if (!NewActionGroup.anyActionFromGroupMatches(myToolBarGroup, true, Predicate.isEqual(groupByAction))) { + myToolBarGroup.addSeparator(); + myToolBarGroup.add(groupByAction); + } + + if (isVerticalToolbar()) { + List treeActions = TreeActionsToolbarPanel.createTreeActions(myViewer); + boolean hasTreeActions = ContainerUtil.exists( + treeActions, action -> NewActionGroup.anyActionFromGroupMatches(myToolBarGroup, true, Predicate.isEqual(action))); + if (!hasTreeActions) { + myToolBarGroup.addSeparator(); + myToolBarGroup.addAll(treeActions); } + } - @Override - public @NotNull List postProcessVisibleChildren(@NotNull List visibleChildren, - @NotNull UpdateSession updateSession) { - if (visibleChildren.contains(groupBy)) return Collections.unmodifiableList(visibleChildren); - return ContainerUtil.concat(visibleChildren, List.of(Separator.getInstance()), List.of(groupBy)); - } - }; - ((DefaultActionGroup)myToolbar.getActionGroup()).add(toolbarGroup); - - ActionGroup popupGroup = new ActionGroup() { - final List actions = createPopupMenuActions(); - final List treeActions = isVerticalToolbar() ? TreeActionsToolbarPanel.createTreeActions(myViewer) : Collections.emptyList(); - - @Override - public AnAction @NotNull [] getChildren(@Nullable AnActionEvent e) { - if (e == null) return AnAction.EMPTY_ARRAY; - for (AnAction action : treeActions) { - e.getUpdateSession().presentation(action); - } - return actions.toArray(AnAction.EMPTY_ARRAY); - } - - @Override - public @NotNull List postProcessVisibleChildren(@NotNull List visibleChildren, - @NotNull UpdateSession updateSession) { - if (treeActions.isEmpty()) return Collections.unmodifiableList(visibleChildren); - if (ContainerUtil.findInstance(visibleChildren, CollapseAllAction.class) != null) return Collections.unmodifiableList(visibleChildren); - return ContainerUtil.concat(visibleChildren, List.of(Separator.getInstance()), treeActions); - } - }; - - DiffUtil.keepToolbarActionsPromoted(myToolbar); - myViewer.installPopupHandler(popupGroup); myShowDiffAction.registerCustomShortcutSet(this, null); + DiffUtil.recursiveRegisterShortcutSet(myToolBarGroup, this, null); } @NotNull @@ -245,9 +224,15 @@ public abstract class ChangesBrowserBase extends JPanel implements DataProvider } public void addToolbarAction(@NotNull AnAction action) { - myAdditionalToolbarActions.add(action); + myToolBarGroup.add(action); + action.registerCustomShortcutSet(this, null); } + public void addToolbarSeparator() { + myToolBarGroup.addSeparator(); + } + + @NotNull public JComponent getPreferredFocusedComponent() { return myViewer.getPreferredFocusedComponent();