From abd4c25a0a849c8ffcfb6dbc29672d630d9b20f5 Mon Sep 17 00:00:00 2001 From: Dmitry Batrak Date: Thu, 20 Feb 2014 13:37:44 +0400 Subject: [PATCH] IDEA-80056 Column selection mode improvement * fix issue with Select/Unselect next occurence actions * fix regression (IDEA-121011) --- .../actionSystem/ex/AnActionListener.java | 5 +- .../EditorLastActionTracker.java} | 14 ++-- .../actions/SelectNextOccurrenceAction.java | 8 +- .../actions/UnselectLastOccurrenceAction.java | 1 + .../impl/EditorLastActionTrackerImpl.java} | 16 ++-- .../src/componentSets/Platform.xml | 5 ++ .../src/componentSets/UICore.xml | 4 - .../SelectUnselectOccurrenceActionsTest.java | 13 +++- .../impl/EditorLastActionTrackerTest.java | 76 +++++++++++++++++++ 9 files changed, 118 insertions(+), 24 deletions(-) rename platform/editor-ui-api/src/com/intellij/openapi/{actionSystem/LastActionTracker.java => editor/EditorLastActionTracker.java} (68%) rename platform/platform-impl/src/com/intellij/openapi/{actionSystem/impl/LastActionTrackerImpl.java => editor/impl/EditorLastActionTrackerImpl.java} (83%) create mode 100644 platform/platform-tests/testSrc/com/intellij/openapi/editor/impl/EditorLastActionTrackerTest.java diff --git a/platform/editor-ui-api/src/com/intellij/openapi/actionSystem/ex/AnActionListener.java b/platform/editor-ui-api/src/com/intellij/openapi/actionSystem/ex/AnActionListener.java index f494fdc710ad..e195cd193218 100644 --- a/platform/editor-ui-api/src/com/intellij/openapi/actionSystem/ex/AnActionListener.java +++ b/platform/editor-ui-api/src/com/intellij/openapi/actionSystem/ex/AnActionListener.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2013 JetBrains s.r.o. + * Copyright 2000-2014 JetBrains s.r.o. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -35,6 +35,9 @@ import com.intellij.openapi.actionSystem.DataContext; public interface AnActionListener { void beforeActionPerformed(AnAction action, DataContext dataContext, AnActionEvent event); + /** + * Note that using dataContext in implementing methods is unsafe - it could have been invalidated by the performed action. + */ void afterActionPerformed(AnAction action, DataContext dataContext, AnActionEvent event); void beforeEditorTyping(char c, DataContext dataContext); diff --git a/platform/editor-ui-api/src/com/intellij/openapi/actionSystem/LastActionTracker.java b/platform/editor-ui-api/src/com/intellij/openapi/editor/EditorLastActionTracker.java similarity index 68% rename from platform/editor-ui-api/src/com/intellij/openapi/actionSystem/LastActionTracker.java rename to platform/editor-ui-api/src/com/intellij/openapi/editor/EditorLastActionTracker.java index b3f18c926e4c..9e580b60feb6 100644 --- a/platform/editor-ui-api/src/com/intellij/openapi/actionSystem/LastActionTracker.java +++ b/platform/editor-ui-api/src/com/intellij/openapi/editor/EditorLastActionTracker.java @@ -13,25 +13,25 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package com.intellij.openapi.actionSystem; +package com.intellij.openapi.editor; import com.intellij.openapi.application.ApplicationManager; import org.jetbrains.annotations.Nullable; /** - * This component provides the notion of last action. Its purpose is to be able to determine whether some action was performed right after - * another specific action. + * This component provides the notion of last editor action. + * Its purpose is to be able to determine whether some action was performed right after another specific action. *

* It's supposed to be used from EDT only. */ -public abstract class LastActionTracker { - public static LastActionTracker getInstance() { - return ApplicationManager.getApplication().getComponent(LastActionTracker.class); +public abstract class EditorLastActionTracker { + public static EditorLastActionTracker getInstance() { + return ApplicationManager.getApplication().getComponent(EditorLastActionTracker.class); } /** * Returns the id of the previously invoked action or null, if no history exists yet, or last user activity was of - * non-action type, like mouse clicking in editor or text typing. + * non-action type, like mouse clicking in editor or text typing, or previous action was invoked for a different editor. */ @Nullable public abstract String getLastActionId(); diff --git a/platform/lang-impl/src/com/intellij/openapi/editor/actions/SelectNextOccurrenceAction.java b/platform/lang-impl/src/com/intellij/openapi/editor/actions/SelectNextOccurrenceAction.java index d8870484c380..35e34a8a1f2e 100644 --- a/platform/lang-impl/src/com/intellij/openapi/editor/actions/SelectNextOccurrenceAction.java +++ b/platform/lang-impl/src/com/intellij/openapi/editor/actions/SelectNextOccurrenceAction.java @@ -23,7 +23,7 @@ import com.intellij.find.FindBundle; import com.intellij.find.FindManager; import com.intellij.find.FindModel; import com.intellij.find.FindResult; -import com.intellij.openapi.actionSystem.LastActionTracker; +import com.intellij.openapi.editor.EditorLastActionTracker; import com.intellij.openapi.actionSystem.DataContext; import com.intellij.openapi.actionSystem.IdeActions; import com.intellij.openapi.editor.Caret; @@ -45,7 +45,7 @@ public class SelectNextOccurrenceAction extends EditorAction { super(new Handler()); } - private static class Handler extends EditorActionHandler { + static class Handler extends EditorActionHandler { @Override public boolean isEnabled(Editor editor, DataContext dataContext) { return super.isEnabled(editor, dataContext) && editor.getProject() != null && editor.getCaretModel().supportsMultipleCarets(); @@ -127,7 +127,7 @@ public class SelectNextOccurrenceAction extends EditorAction { false); } - private static boolean getAndResetNotFoundStatus(Editor editor) { + static boolean getAndResetNotFoundStatus(Editor editor) { boolean status = editor.getUserData(NOT_FOUND) != null; editor.putUserData(NOT_FOUND, null); return status && isRepeatedActionInvocation(); @@ -149,7 +149,7 @@ public class SelectNextOccurrenceAction extends EditorAction { } private static boolean isRepeatedActionInvocation() { - String lastActionId = LastActionTracker.getInstance().getLastActionId(); + String lastActionId = EditorLastActionTracker.getInstance().getLastActionId(); return IdeActions.ACTION_SELECT_NEXT_OCCURENCE.equals(lastActionId) || IdeActions.ACTION_UNSELECT_LAST_OCCURENCE.equals(lastActionId); } } diff --git a/platform/lang-impl/src/com/intellij/openapi/editor/actions/UnselectLastOccurrenceAction.java b/platform/lang-impl/src/com/intellij/openapi/editor/actions/UnselectLastOccurrenceAction.java index 37b6c56a5c4a..192e41cdfdb6 100644 --- a/platform/lang-impl/src/com/intellij/openapi/editor/actions/UnselectLastOccurrenceAction.java +++ b/platform/lang-impl/src/com/intellij/openapi/editor/actions/UnselectLastOccurrenceAction.java @@ -42,6 +42,7 @@ public class UnselectLastOccurrenceAction extends EditorAction { else { editor.getSelectionModel().removeSelection(); } + SelectNextOccurrenceAction.Handler.getAndResetNotFoundStatus(editor); editor.getScrollingModel().scrollToCaret(ScrollType.RELATIVE); } } diff --git a/platform/platform-impl/src/com/intellij/openapi/actionSystem/impl/LastActionTrackerImpl.java b/platform/platform-impl/src/com/intellij/openapi/editor/impl/EditorLastActionTrackerImpl.java similarity index 83% rename from platform/platform-impl/src/com/intellij/openapi/actionSystem/impl/LastActionTrackerImpl.java rename to platform/platform-impl/src/com/intellij/openapi/editor/impl/EditorLastActionTrackerImpl.java index 7f003654d1f2..8e2d4e7429f3 100644 --- a/platform/platform-impl/src/com/intellij/openapi/actionSystem/impl/LastActionTrackerImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/editor/impl/EditorLastActionTrackerImpl.java @@ -13,27 +13,29 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package com.intellij.openapi.actionSystem.impl; +package com.intellij.openapi.editor.impl; import com.intellij.openapi.actionSystem.*; import com.intellij.openapi.actionSystem.ex.AnActionListener; import com.intellij.openapi.components.ApplicationComponent; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.editor.EditorFactory; +import com.intellij.openapi.editor.EditorLastActionTracker; import com.intellij.openapi.editor.event.EditorEventMulticaster; import com.intellij.openapi.editor.event.EditorMouseEvent; import com.intellij.openapi.editor.event.EditorMouseListener; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -public class LastActionTrackerImpl extends LastActionTracker implements ApplicationComponent, AnActionListener, EditorMouseListener { +public class EditorLastActionTrackerImpl extends EditorLastActionTracker implements ApplicationComponent, AnActionListener, EditorMouseListener { private final ActionManager myActionManager; private final EditorEventMulticaster myEditorEventMulticaster; private String myLastActionId; + private Editor myCurrentEditor; private Editor myLastEditor; - LastActionTrackerImpl(ActionManager actionManager, EditorFactory editorFactory) { + EditorLastActionTrackerImpl(ActionManager actionManager, EditorFactory editorFactory) { myActionManager = actionManager; myEditorEventMulticaster = editorFactory.getEventMulticaster(); } @@ -53,7 +55,7 @@ public class LastActionTrackerImpl extends LastActionTracker implements Applicat @NotNull @Override public String getComponentName() { - return "LastActionTracker"; + return "EditorLastActionTracker"; } @Override @@ -64,7 +66,8 @@ public class LastActionTrackerImpl extends LastActionTracker implements Applicat @Override public void beforeActionPerformed(AnAction action, DataContext dataContext, AnActionEvent event) { - if (CommonDataKeys.EDITOR.getData(dataContext) != myLastEditor) { + myCurrentEditor = CommonDataKeys.EDITOR.getData(dataContext); + if (myCurrentEditor != myLastEditor) { resetLastAction(); } } @@ -72,7 +75,8 @@ public class LastActionTrackerImpl extends LastActionTracker implements Applicat @Override public void afterActionPerformed(AnAction action, DataContext dataContext, AnActionEvent event) { myLastActionId = getActionId(action); - myLastEditor = CommonDataKeys.EDITOR.getData(dataContext); + myLastEditor = myCurrentEditor; + myCurrentEditor = null; } @Override diff --git a/platform/platform-resources/src/componentSets/Platform.xml b/platform/platform-resources/src/componentSets/Platform.xml index 77d5050c09d6..af2ede649d0b 100644 --- a/platform/platform-resources/src/componentSets/Platform.xml +++ b/platform/platform-resources/src/componentSets/Platform.xml @@ -145,6 +145,11 @@ com.intellij.diagnostic.DebugLogManager + + + com.intellij.openapi.editor.EditorLastActionTracker + com.intellij.openapi.editor.impl.EditorLastActionTrackerImpl + diff --git a/platform/platform-resources/src/componentSets/UICore.xml b/platform/platform-resources/src/componentSets/UICore.xml index 042b053f68d4..8d7d5d63e055 100644 --- a/platform/platform-resources/src/componentSets/UICore.xml +++ b/platform/platform-resources/src/componentSets/UICore.xml @@ -21,10 +21,6 @@ com.intellij.openapi.actionSystem.ActionManager com.intellij.openapi.actionSystem.impl.ActionManagerImpl - - com.intellij.openapi.actionSystem.LastActionTracker - com.intellij.openapi.actionSystem.impl.LastActionTrackerImpl - com.intellij.openapi.keymap.KeymapManager com.intellij.openapi.keymap.impl.KeymapManagerImpl diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/editor/actions/SelectUnselectOccurrenceActionsTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/editor/actions/SelectUnselectOccurrenceActionsTest.java index 58d36578ee37..50f8ca0e6f3b 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/editor/actions/SelectUnselectOccurrenceActionsTest.java +++ b/platform/platform-tests/testSrc/com/intellij/openapi/editor/actions/SelectUnselectOccurrenceActionsTest.java @@ -16,6 +16,7 @@ package com.intellij.openapi.editor.actions; import com.intellij.codeInsight.hint.EditorHintListener; +import com.intellij.openapi.actionSystem.IdeActions; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.editor.Document; import com.intellij.openapi.editor.FoldRegion; @@ -176,6 +177,14 @@ public class SelectUnselectOccurrenceActionsTest extends LightPlatformCodeInsigh assertTrue(foldRegions[0].isExpanded()); } + public void testSelectAfterNotFoundAndUnselect() throws Exception { + init("text text text"); + executeAction(); + executeReverseAction(); + executeAction(); + checkResult("text text text"); + } + private void init(String text) { myFixture.configureByText(FileTypes.PLAIN_TEXT, text); } @@ -185,10 +194,10 @@ public class SelectUnselectOccurrenceActionsTest extends LightPlatformCodeInsigh } private void executeAction() { - myFixture.performEditorAction("SelectNextOccurrence"); + myFixture.performEditorAction(IdeActions.ACTION_SELECT_NEXT_OCCURENCE); } private void executeReverseAction() { - myFixture.performEditorAction("UnselectLastOccurrence"); + myFixture.performEditorAction(IdeActions.ACTION_UNSELECT_LAST_OCCURENCE); } } diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/editor/impl/EditorLastActionTrackerTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/editor/impl/EditorLastActionTrackerTest.java new file mode 100644 index 000000000000..c972183cd3de --- /dev/null +++ b/platform/platform-tests/testSrc/com/intellij/openapi/editor/impl/EditorLastActionTrackerTest.java @@ -0,0 +1,76 @@ +/* + * Copyright 2000-2014 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.openapi.editor.impl; + +import com.intellij.openapi.actionSystem.DataContext; +import com.intellij.openapi.editor.Caret; +import com.intellij.openapi.editor.Editor; +import com.intellij.openapi.editor.EditorLastActionTracker; +import com.intellij.openapi.editor.actionSystem.EditorActionHandler; +import com.intellij.openapi.editor.actionSystem.EditorActionManager; +import com.intellij.testFramework.fixtures.EditorMouseFixture; +import com.intellij.testFramework.fixtures.LightPlatformCodeInsightFixtureTestCase; +import org.jetbrains.annotations.Nullable; + +public class EditorLastActionTrackerTest extends LightPlatformCodeInsightFixtureTestCase { + public static final String SAMPLE_ACTION = "EditorDelete"; + private final EditorActionHandler myActionHandler = new MyActionHandler(); + + private EditorLastActionTracker myTracker; + private EditorActionHandler mySavedHandler; + + @Override + public void setUp() throws Exception { + super.setUp(); + myTracker = EditorLastActionTracker.getInstance(); + mySavedHandler = EditorActionManager.getInstance().setActionHandler(SAMPLE_ACTION, myActionHandler); + + myFixture.configureByText(getTestName(true) + ".txt", "doesn't matter"); + myFixture.performEditorAction(SAMPLE_ACTION); + } + + @Override + public void tearDown() throws Exception { + EditorActionManager.getInstance().setActionHandler(SAMPLE_ACTION, mySavedHandler); + super.tearDown(); + } + + public void testLastActionIsAvailable() throws Exception { + assertEquals(SAMPLE_ACTION, myTracker.getLastActionId()); + } + + public void testMouseClickClearsLastAction() throws Exception { + new EditorMouseFixture((EditorImpl)myFixture.getEditor()).clickAt(0, 1); + assertNull(myTracker.getLastActionId()); + } + + public void testTypingClearsLastAction() throws Exception { + myFixture.type('A'); + assertNull(myTracker.getLastActionId()); + } + + public void testTwoEditors() throws Exception { + myFixture.configureByText(getTestName(true) + "-other.txt", "doesn't matter as well"); + myFixture.performEditorAction(SAMPLE_ACTION); + } + + private class MyActionHandler extends EditorActionHandler { + @Override + public void execute(Editor editor, @Nullable Caret caret, DataContext dataContext) { + assertNull(myTracker.getLastActionId()); + } + } +}