From d5def5048e0ebac39a822db57bf34d85989fb656 Mon Sep 17 00:00:00 2001 From: "Gregory.Shrago" Date: Thu, 11 Apr 2024 01:32:51 +0400 Subject: [PATCH] do not unstub actions in `ActionUrl.readExternal` 1. Introduce componentId and componentType 2. Move UI methods to `CustomizableActionsPanel` 3. Drop `Pair` references, component is now never a `Pair` IJPL-936 T: LoadingState.logStateError from ActionManagerImplKt.instantiate IJPL-937 ActionMacroManager and CustomActionsSchema load action classes on application start GitOrigin-RevId: f0d16653aea55c53e14843cdafc4dc71fedc9439 --- .../ide/actionMacro/ActionMacroManager.kt | 2 +- .../ide/ui/customization/ActionUrl.java | 327 ++++++++---------- .../CustomizableActionsPanel.java | 81 +++++ .../actionSystem/impl/ActionManagerImpl.kt | 2 +- 4 files changed, 232 insertions(+), 180 deletions(-) diff --git a/platform/platform-impl/src/com/intellij/ide/actionMacro/ActionMacroManager.kt b/platform/platform-impl/src/com/intellij/ide/actionMacro/ActionMacroManager.kt index f5f7e03bf607..2c665ae83e96 100644 --- a/platform/platform-impl/src/com/intellij/ide/actionMacro/ActionMacroManager.kt +++ b/platform/platform-impl/src/com/intellij/ide/actionMacro/ActionMacroManager.kt @@ -411,7 +411,7 @@ class ActionMacroManager internal constructor(private val coroutineScope: Corout // fix references to and icons of renamed macros in the custom actions schema val customActionsSchema = CustomActionsSchema.getInstance() for (actionUrl in customActionsSchema.getActions()) { - val newId = renamingMap.get(actionUrl.component) + val newId = renamingMap.get(actionUrl.componentId) if (newId != null) { actionUrl.component = newId } diff --git a/platform/platform-impl/src/com/intellij/ide/ui/customization/ActionUrl.java b/platform/platform-impl/src/com/intellij/ide/ui/customization/ActionUrl.java index 9fa38ebab5a7..4ac50e6d64f9 100644 --- a/platform/platform-impl/src/com/intellij/ide/ui/customization/ActionUrl.java +++ b/platform/platform-impl/src/com/intellij/ide/ui/customization/ActionUrl.java @@ -7,31 +7,25 @@ import com.intellij.openapi.actionSystem.AnAction; import com.intellij.openapi.actionSystem.Separator; import com.intellij.openapi.keymap.impl.ui.ActionsTreeUtil; import com.intellij.openapi.keymap.impl.ui.Group; -import com.intellij.openapi.util.*; +import com.intellij.openapi.util.DefaultJDOMExternalizer; +import com.intellij.openapi.util.InvalidDataException; +import com.intellij.openapi.util.JDOMExternalizable; +import com.intellij.openapi.util.WriteExternalException; +import com.intellij.openapi.util.text.StringUtil; import org.intellij.lang.annotations.MagicConstant; import org.jdom.Element; import org.jetbrains.annotations.NonNls; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import javax.swing.*; -import javax.swing.tree.DefaultMutableTreeNode; -import javax.swing.tree.TreePath; import java.util.ArrayList; +import java.util.Objects; public final class ActionUrl implements JDOMExternalizable { public static final int ADDED = 1; public static final int DELETED = -1; - - //temp action only public static final int MOVE = 2; - private ArrayList myGroupPath; - private Object myComponent; - private int myActionType; - private int myAbsolutePosition; - - public int myInitialPosition = -1; - private static final @NonNls String IS_GROUP = "is_group"; private static final @NonNls String SEPARATOR = "seperator"; private static final @NonNls String IS_ACTION = "is_action"; @@ -41,48 +35,117 @@ public final class ActionUrl implements JDOMExternalizable { private static final @NonNls String POSITION = "position"; private static final @NonNls String FORCE_POPUP = "forse_popup"; + private static final int TYPE_NONE = 0; + private static final int TYPE_SEPARATOR = 1; + private static final int TYPE_ACTION = 2; + private static final int TYPE_GROUP = 3; + private static final int TYPE_POPUP_GROUP = 4; + + private @NotNull ArrayList myGroupPath; + private @Nullable String myComponentId; + @MagicConstant(intValues = {TYPE_NONE, TYPE_SEPARATOR, TYPE_ACTION, TYPE_GROUP, TYPE_POPUP_GROUP}) + private int myComponentType; + private @Nullable Object myComponent; + private int myActionType; + private int myAbsolutePosition; + private int myInitialPosition = -1; + public ActionUrl() { myGroupPath = new ArrayList<>(); } - public ActionUrl(final ArrayList groupPath, - final Object component, + public ActionUrl(@NotNull ArrayList groupPath, + @Nullable Object component, @MagicConstant(intValues = {ADDED, DELETED, MOVE}) int actionType, - final int position) { + int position) { myGroupPath = groupPath; - myComponent = component; + setComponent(component); myActionType = actionType; myAbsolutePosition = position; } - public ArrayList getGroupPath() { + private ActionUrl(@NotNull ArrayList groupPath, + @Nullable String componentId, + int componentType, + @Nullable Object component, + int actionType, + int absolutePosition, + int initialPosition) { + myGroupPath = groupPath; + myComponentId = componentId; + myComponentType = componentType; + myComponent = component; + myActionType = actionType; + myAbsolutePosition = absolutePosition; + myInitialPosition = initialPosition; + } + + public @NotNull ArrayList getGroupPath() { return myGroupPath; } + public void setGroupPath(@NotNull ArrayList groupPath) { + myGroupPath = groupPath; + } + public String getParentGroup() { return myGroupPath.get(myGroupPath.size() - 1); } public String getRootGroup() { - return myGroupPath.size() >= 1 ? myGroupPath.get(1) : ""; + return !myGroupPath.isEmpty() ? myGroupPath.get(1) : ""; } - public Object getComponent() { + public @Nullable String getComponentId() { + return myComponentId; + } + + // action or string, separator, group + public @Nullable Object getComponent() { + if (myComponent != null || myComponentType == TYPE_NONE) return myComponent; + switch (myComponentType) { + case TYPE_SEPARATOR -> myComponent = Separator.getInstance(); + case TYPE_ACTION -> myComponent = myComponentId; + case TYPE_GROUP, TYPE_POPUP_GROUP -> { + Objects.requireNonNull(myComponentId); + AnAction action = ActionManager.getInstance().getActionOrStub(myComponentId); + //noinspection HardCodedStringLiteral + Group group = action instanceof ActionGroup o + ? ActionsTreeUtil.createGroup(o, true, null) + : new Group(myComponentId, myComponentId); + group.setForceShowAsPopup(myComponentType == TYPE_POPUP_GROUP); + myComponent = group; + } + } return myComponent; } + public void setComponent(@Nullable Object c) { + myComponent = c; + myComponentType = + c instanceof Separator ? TYPE_SEPARATOR : + c instanceof String || c instanceof AnAction ? TYPE_ACTION : + c instanceof Group o ? (o.isForceShowAsPopup() ? TYPE_POPUP_GROUP : TYPE_GROUP) : + TYPE_NONE; + myComponentId = + c instanceof String o ? o : + c instanceof Group o ? (StringUtil.isEmpty(o.getId()) ? o.getName() : o.getId()) : + null; + } + public @Nullable AnAction getComponentAction() { - if (myComponent instanceof Separator) { - return Separator.getInstance(); + Object component = getComponent(); + if (component instanceof Separator o) { + return o; } - if (myComponent instanceof String) { - return ActionManager.getInstance().getAction((String)myComponent); + if (component instanceof String o) { + return ActionManager.getInstance().getAction(o); } - if (myComponent instanceof Group) { - final String id = ((Group)myComponent).getId(); - if (id == null || id.length() == 0) { - return ((Group)myComponent).constructActionGroup(true); + if (component instanceof Group o) { + String id = o.getId(); + if (StringUtil.isEmpty(id)) { + return o.constructActionGroup(true); } return ActionManager.getInstance().getAction(id); } @@ -120,20 +183,12 @@ public final class ActionUrl implements JDOMExternalizable { for (Element o : element.getChildren(PATH)) { myGroupPath.add(o.getAttributeValue(VALUE)); } - final @NlsSafe String attributeValue = element.getAttributeValue(VALUE); - if (element.getAttributeValue(IS_ACTION) != null) { - myComponent = attributeValue; - } - else if (element.getAttributeValue(SEPARATOR) != null) { - myComponent = Separator.getInstance(); - } - else if (element.getAttributeValue(IS_GROUP) != null) { - final AnAction action = ActionManager.getInstance().getAction(attributeValue); - Group group = action instanceof ActionGroup - ? ActionsTreeUtil.createGroup((ActionGroup)action, true, null) - : new Group(attributeValue, attributeValue); - group.setForceShowAsPopup(Boolean.parseBoolean(element.getAttributeValue(FORCE_POPUP))); - myComponent = group; + myComponentId = element.getAttributeValue(VALUE); + myComponentType = element.getAttributeValue(IS_ACTION) != null ? TYPE_ACTION : + element.getAttributeValue(SEPARATOR) != null ? TYPE_SEPARATOR : + element.getAttributeValue(IS_GROUP) != null ? TYPE_GROUP : TYPE_NONE; + if (myComponentType == TYPE_GROUP && Boolean.parseBoolean(element.getAttributeValue(FORCE_POPUP))) { + myComponentType = TYPE_POPUP_GROUP; } String actionTypeString = element.getAttributeValue(ACTION_TYPE); myActionType = actionTypeString == null ? -1 : Integer.parseInt(actionTypeString); @@ -148,150 +203,30 @@ public final class ActionUrl implements JDOMExternalizable { path.setAttribute(VALUE, s); element.addContent(path); } - if (myComponent instanceof String) { - element.setAttribute(VALUE, (String)myComponent); - element.setAttribute(IS_ACTION, Boolean.TRUE.toString()); - } - else if (myComponent instanceof Separator) { - element.setAttribute(SEPARATOR, Boolean.TRUE.toString()); - } - else if (myComponent instanceof Group group) { - final String groupId = group.getId() != null && !group.getId().isEmpty() - ? group.getId() : group.getName(); - element.setAttribute(VALUE, groupId != null ? groupId : ""); - element.setAttribute(IS_GROUP, Boolean.TRUE.toString()); - element.setAttribute(FORCE_POPUP, Boolean.toString(group.isForceShowAsPopup())); + switch (myComponentType) { + case TYPE_SEPARATOR -> + element.setAttribute(SEPARATOR, Boolean.TRUE.toString()); + case TYPE_ACTION -> { + element.setAttribute(VALUE, StringUtil.notNullize(myComponentId)); + element.setAttribute(IS_ACTION, Boolean.TRUE.toString()); + } + case TYPE_GROUP, TYPE_POPUP_GROUP -> { + element.setAttribute(VALUE, StringUtil.notNullize(myComponentId)); + element.setAttribute(IS_GROUP, Boolean.TRUE.toString()); + element.setAttribute(FORCE_POPUP, Boolean.toString(myComponentType == TYPE_POPUP_GROUP)); + } } element.setAttribute(ACTION_TYPE, Integer.toString(myActionType)); element.setAttribute(POSITION, Integer.toString(myAbsolutePosition)); DefaultJDOMExternalizer.writeExternal(this, element); } - public static void changePathInActionsTree(JTree tree, ActionUrl url) { - if (url.myActionType == ADDED) { - addPathToActionsTree(tree, url); - } - else if (url.myActionType == DELETED) { - removePathFromActionsTree(tree, url); - } - else if (url.myActionType == MOVE) { - movePathInActionsTree(tree, url); - } + public @NotNull ActionUrl copy() { + return new ActionUrl(new ArrayList<>(myGroupPath), myComponentId, myComponentType, myComponent, + myActionType, myAbsolutePosition, myInitialPosition); } - public static @Nullable DefaultMutableTreeNode addPathToActionsTree(JTree tree, ActionUrl url) { - final TreePath treePath = CustomizationUtil.getTreePath(tree, url); - if (treePath == null) return null; - DefaultMutableTreeNode node = (DefaultMutableTreeNode)treePath.getLastPathComponent(); - final int absolutePosition = url.getAbsolutePosition(); - if (node.getChildCount() >= absolutePosition && absolutePosition >= 0) { - DefaultMutableTreeNode newNode; - if (url.getComponent() instanceof Group) { - newNode = ActionsTreeUtil.createNode((Group)url.getComponent()); - } - else { - newNode = new DefaultMutableTreeNode(url.getComponent()); - } - node.insert(newNode, absolutePosition); - return newNode; - } - return null; - } - - private static void removePathFromActionsTree(JTree tree, ActionUrl url) { - if (url.myComponent == null) return; - final TreePath treePath = CustomizationUtil.getTreePath(tree, url); - if (treePath == null) return; - DefaultMutableTreeNode node = (DefaultMutableTreeNode)treePath.getLastPathComponent(); - final int absolutePosition = url.getAbsolutePosition(); - if (node.getChildCount() > absolutePosition && absolutePosition >= 0) { - DefaultMutableTreeNode child = (DefaultMutableTreeNode)node.getChildAt(absolutePosition); - Object userObj = child.getUserObject(); - if (url.getComponent().equals(userObj instanceof Pair pair ? pair.first : userObj)) { - node.remove(child); - } - } - } - - private static void movePathInActionsTree(JTree tree, ActionUrl url) { - final TreePath treePath = CustomizationUtil.getTreePath(tree, url); - if (treePath != null) { - if (treePath.getLastPathComponent() != null) { - final DefaultMutableTreeNode parent = ((DefaultMutableTreeNode)treePath.getLastPathComponent()); - final int absolutePosition = url.getAbsolutePosition(); - final int initialPosition = url.getInitialPosition(); - if (parent.getChildCount() > absolutePosition && absolutePosition >= 0) { - if (parent.getChildCount() > initialPosition && initialPosition >= 0) { - final DefaultMutableTreeNode child = (DefaultMutableTreeNode)parent.getChildAt(initialPosition); - Object userObj = child.getUserObject(); - if (url.getComponent().equals(userObj instanceof Pair pair ? pair.first : userObj)) { - parent.remove(child); - parent.insert(child, absolutePosition); - } - } - } - } - } - } - - public static ArrayList getGroupPath(final TreePath treePath, boolean includeSelf) { - final ArrayList result = new ArrayList<>(); - int length = treePath.getPath().length - (includeSelf ? 0 : 1); - for (int i = 0; i < length; i++) { - Object o = ((DefaultMutableTreeNode)treePath.getPath()[i]).getUserObject(); - if (o instanceof Group) { - result.add(((Group)o).getName()); - } - } - return result; - } - - @Override - public boolean equals(Object object) { - if (!(object instanceof ActionUrl url)) { - return false; - } - Object comp = myComponent instanceof Pair ? ((Pair)myComponent).first : myComponent; - Object thatComp = url.myComponent instanceof Pair ? ((Pair)url.myComponent).first : url.myComponent; - return Comparing.equal(comp, thatComp) - && myGroupPath.equals(url.myGroupPath) - && myAbsolutePosition == url.myAbsolutePosition - && myActionType == url.myActionType; - } - - @Override - public int hashCode() { - int result = myComponent != null ? myComponent.hashCode() : 0; - result += 29 * myGroupPath.hashCode(); - return result; - } - - public void setComponent(final Object object) { - myComponent = object; - } - - public void setGroupPath(final ArrayList groupPath) { - myGroupPath = groupPath; - } - - @Override - public @NonNls String toString() { - return "ActionUrl{" + - "myGroupPath=" + myGroupPath + - ", myComponent=" + myComponent + - ", myActionType=" + myActionType + - ", myAbsolutePosition=" + myAbsolutePosition + - ", myInitialPosition=" + myInitialPosition + - '}'; - } - - public ActionUrl copy() { - ActionUrl url = new ActionUrl(new ArrayList<>(getGroupPath()), getComponent(), getActionType(), getAbsolutePosition()); - url.setInitialPosition(getInitialPosition()); - return url; - } - - public ActionUrl getInverted() { + public @NotNull ActionUrl getInverted() { ActionUrl copy = copy(); if (myActionType == ADDED || myActionType == DELETED) { copy.setActionType(-myActionType); @@ -302,4 +237,40 @@ public final class ActionUrl implements JDOMExternalizable { } return copy; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof ActionUrl url)) return false; + + return myComponentType == url.myComponentType && + myActionType == url.myActionType && + myAbsolutePosition == url.myAbsolutePosition && + myInitialPosition == url.myInitialPosition && + myGroupPath.equals(url.myGroupPath) && + Objects.equals(myComponentId, url.myComponentId); + } + + @Override + public int hashCode() { + int result = myGroupPath.hashCode(); + result = 31 * result + Objects.hashCode(myComponentId); + result = 31 * result + myComponentType; + result = 31 * result + myActionType; + result = 31 * result + myAbsolutePosition; + result = 31 * result + myInitialPosition; + return result; + } + + @Override + public String toString() { + return "ActionUrl{" + "myGroupPath=" + myGroupPath + + ", myComponentId='" + myComponentId + '\'' + + ", myComponentType=" + myComponentType + + ", myComponent=" + myComponent + + ", myActionType=" + myActionType + + ", myAbsolutePosition=" + myAbsolutePosition + + ", myInitialPosition=" + myInitialPosition + + '}'; + } } diff --git a/platform/platform-impl/src/com/intellij/ide/ui/customization/CustomizableActionsPanel.java b/platform/platform-impl/src/com/intellij/ide/ui/customization/CustomizableActionsPanel.java index a0347ac38850..150b22137a1c 100644 --- a/platform/platform-impl/src/com/intellij/ide/ui/customization/CustomizableActionsPanel.java +++ b/platform/platform-impl/src/com/intellij/ide/ui/customization/CustomizableActionsPanel.java @@ -10,6 +10,7 @@ import com.intellij.openapi.actionSystem.impl.ActionToolbarImpl; import com.intellij.openapi.actionSystem.toolbarLayout.ToolbarLayoutStrategy; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.keymap.KeymapUtil; +import com.intellij.openapi.keymap.impl.ui.ActionsTreeUtil; import com.intellij.openapi.keymap.impl.ui.Group; import com.intellij.openapi.options.ConfigurationException; import com.intellij.openapi.project.DumbAwareAction; @@ -415,6 +416,86 @@ public class CustomizableActionsPanel { return true; } + private static void changePathInActionsTree(@NotNull JTree tree, @NotNull ActionUrl url) { + int actionType = url.getActionType(); + if (actionType == ADDED) { + addPathToActionsTree(tree, url); + } + else if (actionType == DELETED) { + removePathFromActionsTree(tree, url); + } + else if (actionType == MOVE) { + movePathInActionsTree(tree, url); + } + } + + private static @Nullable DefaultMutableTreeNode addPathToActionsTree(@NotNull JTree tree, @NotNull ActionUrl url) { + TreePath treePath = CustomizationUtil.getTreePath(tree, url); + if (treePath == null) return null; + DefaultMutableTreeNode node = (DefaultMutableTreeNode)treePath.getLastPathComponent(); + int absolutePosition = url.getAbsolutePosition(); + if (node.getChildCount() >= absolutePosition && absolutePosition >= 0) { + DefaultMutableTreeNode newNode; + if (url.getComponent() instanceof Group o) { + newNode = ActionsTreeUtil.createNode(o); + } + else { + newNode = new DefaultMutableTreeNode(url.getComponent()); + } + node.insert(newNode, absolutePosition); + return newNode; + } + return null; + } + + private static void removePathFromActionsTree(@NotNull JTree tree, @NotNull ActionUrl url) { + Object component = url.getComponent(); + if (component == null) return; + TreePath treePath = CustomizationUtil.getTreePath(tree, url); + if (treePath == null) return; + DefaultMutableTreeNode node = (DefaultMutableTreeNode)treePath.getLastPathComponent(); + int absolutePosition = url.getAbsolutePosition(); + if (node.getChildCount() > absolutePosition && absolutePosition >= 0) { + DefaultMutableTreeNode child = (DefaultMutableTreeNode)node.getChildAt(absolutePosition); + Object userObj = child.getUserObject(); + if (component.equals(userObj instanceof Pair pair ? pair.first : userObj)) { + node.remove(child); + } + } + } + + private static void movePathInActionsTree(@NotNull JTree tree, @NotNull ActionUrl url) { + TreePath treePath = CustomizationUtil.getTreePath(tree, url); + Object pathComponent = treePath == null ? null : treePath.getLastPathComponent(); + DefaultMutableTreeNode parent = pathComponent instanceof DefaultMutableTreeNode o ? o : null; + if (parent == null) return; + int absolutePosition = url.getAbsolutePosition(); + int initialPosition = url.getInitialPosition(); + Object component = url.getComponent(); + if (parent.getChildCount() > absolutePosition && absolutePosition >= 0) { + if (parent.getChildCount() > initialPosition && initialPosition >= 0) { + final DefaultMutableTreeNode child = (DefaultMutableTreeNode)parent.getChildAt(initialPosition); + Object userObj = child.getUserObject(); + if (component != null && component.equals(userObj instanceof Pair pair ? pair.first : userObj)) { + parent.remove(child); + parent.insert(child, absolutePosition); + } + } + } + } + + private static @NotNull ArrayList getGroupPath(final TreePath treePath, boolean includeSelf) { + ArrayList result = new ArrayList<>(); + int length = treePath.getPath().length - (includeSelf ? 0 : 1); + for (int i = 0; i < length; i++) { + Object o = ((DefaultMutableTreeNode)treePath.getPath()[i]).getUserObject(); + if (o instanceof Group) { + result.add(((Group)o).getName()); + } + } + return result; + } + private final class EditIconDialog extends DialogWrapper { private final DefaultMutableTreeNode myNode; private final boolean isNodeInsideMenu; diff --git a/platform/platform-impl/src/com/intellij/openapi/actionSystem/impl/ActionManagerImpl.kt b/platform/platform-impl/src/com/intellij/openapi/actionSystem/impl/ActionManagerImpl.kt index 587cd19c532c..82d31c1f39f8 100644 --- a/platform/platform-impl/src/com/intellij/openapi/actionSystem/impl/ActionManagerImpl.kt +++ b/platform/platform-impl/src/com/intellij/openapi/actionSystem/impl/ActionManagerImpl.kt @@ -1821,7 +1821,7 @@ private class PostInitActionRegistrar( override fun actionRegistered(actionId: String, action: AnAction) { val schema = ApplicationManager.getApplication().serviceIfCreated() ?: return for (url in schema.getActions()) { - if (url.component == actionId) { + if (url.componentId == actionId) { schema.incrementModificationStamp() break }