IDEA-165089 refactor code after code review

This commit is contained in:
Sergey Malenkov
2017-05-02 02:02:26 +03:00
parent 5e205485c5
commit bd6db78581
6 changed files with 200 additions and 212 deletions
@@ -190,6 +190,17 @@ public abstract class AbstractSchemesPanel<T extends Scheme, InfoComponent exten
mySchemesCombo.updateSelected();
}
/**
* Returns an indent to calculate a left margin for the scheme name in the combo box.
* By default, all names are aligned to the left.
*
* @param scheme the scheme to calculate its indent
* @return an indent that shows a nesting level for the specified scheme
*/
protected int getIndent(@NotNull T scheme) {
return 0;
}
/**
* @return True if the panel supports project-level schemes along with IDE ones. In this case there will be
* additional "Copy to Project" and "Copy to IDE" actions for IDE and project schemes respectively and Project/IDE schemes
@@ -160,7 +160,7 @@ public class EditableSchemesCombo<T extends Scheme> {
@Override
protected int getIndent(@NotNull T scheme) {
return mySchemesPanel.getModel().getIndent(scheme);
return mySchemesPanel.getIndent(scheme);
}
@NotNull
@@ -53,17 +53,6 @@ public interface SchemesModel<T extends Scheme> {
*/
boolean isProjectScheme(@NotNull T scheme);
/**
* Returns an indent to calculate a left margin for the scheme name in the combo box.
* By default, all names are aligned to the left.
*
* @param scheme The scheme to check.
* @return An indent that shows a nesting level for the specified scheme.
*/
default int getIndent(@NotNull T scheme) {
return 0;
}
/**
* @param scheme The scheme to check.
* @return True if scheme's name can be edited.
@@ -62,10 +62,12 @@ import java.beans.PropertyChangeListener;
import java.util.List;
import java.util.Map;
import static com.intellij.openapi.actionSystem.impl.ActionToolbarImpl.updateAllToolbarsImmediately;
public class KeymapPanel extends JPanel implements SearchableConfigurable, Configurable.NoScroll, KeymapListener, Disposable {
private JCheckBox preferKeyPositionOverCharOption;
private final KeymapSelector myKeymapSelector = new KeymapSelector(this::currentKeymapChanged);
private final KeymapSchemeManager myManager = new KeymapSelector(this::currentKeymapChanged).getManager();
private final ActionsTree myActionsTree = new ActionsTree();
private FilterComponent myFilterComponent;
private TreeExpansionMonitor myTreeExpansionMonitor;
@@ -77,7 +79,7 @@ public class KeymapPanel extends JPanel implements SearchableConfigurable, Confi
public KeymapPanel() {
setLayout(new BorderLayout());
JPanel keymapPanel = new JPanel(new BorderLayout());
keymapPanel.add(myKeymapSelector, BorderLayout.NORTH);
keymapPanel.add(myManager.getSchemesPanel(), BorderLayout.NORTH);
keymapPanel.add(createKeymapSettingsPanel(), BorderLayout.CENTER);
IdeFrame ideFrame = IdeFocusManager.getGlobalInstance().getLastFocusedFrame();
@@ -127,7 +129,7 @@ public class KeymapPanel extends JPanel implements SearchableConfigurable, Confi
@Override
public void quickListRenamed(final QuickList oldQuickList, final QuickList newQuickList) {
myKeymapSelector.visitMutableKeymaps(keymap -> {
myManager.visitMutableKeymaps(keymap -> {
String actionId = oldQuickList.getActionId();
Shortcut[] shortcuts = keymap.getShortcuts(actionId);
if (shortcuts.length != 0) {
@@ -158,7 +160,7 @@ public class KeymapPanel extends JPanel implements SearchableConfigurable, Confi
}
private void currentKeymapChanged() {
currentKeymapChanged(myKeymapSelector.getSelectedKeymap());
currentKeymapChanged(myManager.getSelectedKeymap());
}
private void currentKeymapChanged(Keymap selectedKeymap) {
@@ -422,7 +424,7 @@ public class KeymapPanel extends JPanel implements SearchableConfigurable, Confi
private static Keymap createKeymapCopyIfNeededAndPossible(Component parent, Keymap keymap) {
if (parent instanceof KeymapPanel) {
KeymapPanel panel = (KeymapPanel)parent;
return panel.myKeymapSelector.getMutableKeymap(keymap);
return panel.myManager.getMutableKeymap(keymap);
}
return keymap;
}
@@ -438,18 +440,19 @@ public class KeymapPanel extends JPanel implements SearchableConfigurable, Confi
if (preferKeyPositionOverCharOption != null) {
preferKeyPositionOverCharOption.setSelected(KeyboardSettingsExternalizable.getInstance().isNonEnglishKeyboardSupportEnabled());
}
myKeymapSelector.reset();
myManager.reset();
}
@Override
public void apply() throws ConfigurationException {
String error = myKeymapSelector.apply();
String error = myManager.apply();
if (error != null) throw new ConfigurationException(error);
updateAllToolbarsImmediately();
}
@Override
public boolean isModified() {
return myKeymapSelector.isModified();
return myManager.isModified();
}
public void selectAction(String actionId) {
@@ -488,7 +491,7 @@ public class KeymapPanel extends JPanel implements SearchableConfigurable, Confi
@Nullable
public Shortcut[] getCurrentShortcuts(@NotNull String actionId) {
Keymap keymap = myKeymapSelector.getSelectedKeymap();
Keymap keymap = myManager.getSelectedKeymap();
return keymap == null ? null : keymap.getShortcuts(actionId);
}
@@ -496,7 +499,7 @@ public class KeymapPanel extends JPanel implements SearchableConfigurable, Confi
String actionId = myActionsTree.getSelectedActionId();
if (actionId == null) return;
Keymap selectedKeymap = myKeymapSelector.getSelectedKeymap();
Keymap selectedKeymap = myManager.getSelectedKeymap();
if (selectedKeymap == null) return;
DefaultActionGroup group = createEditActionGroup(actionId, selectedKeymap);
@@ -566,7 +569,7 @@ public class KeymapPanel extends JPanel implements SearchableConfigurable, Confi
group.add(new DumbAwareAction("Remove " + KeymapUtil.getShortcutText(shortcut)) {
@Override
public void actionPerformed(@NotNull AnActionEvent e) {
Keymap keymap = myKeymapSelector.getMutableKeymap(selectedKeymap);
Keymap keymap = myManager.getMutableKeymap(selectedKeymap);
keymap.removeShortcut(actionId, shortcut);
if (StringUtil.startsWithChar(actionId, '$')) {
keymap.removeShortcut(KeyMapBundle.message("editor.shortcut", actionId.substring(1)), shortcut);
@@ -587,12 +590,12 @@ public class KeymapPanel extends JPanel implements SearchableConfigurable, Confi
});
}
}
if (myKeymapSelector.canResetActionInKeymap(selectedKeymap, actionId)) {
if (myManager.canResetActionInKeymap(selectedKeymap, actionId)) {
group.add(new Separator());
group.add(new DumbAwareAction("Reset Shortcuts") {
@Override
public void actionPerformed(@NotNull AnActionEvent event) {
myKeymapSelector.resetActionInKeymap(selectedKeymap, actionId);
myManager.resetActionInKeymap(selectedKeymap, actionId);
repaintLists();
}
});
@@ -15,20 +15,27 @@
*/
package com.intellij.openapi.keymap.impl.ui;
import com.intellij.application.options.schemes.AbstractSchemeActions;
import com.intellij.application.options.schemes.SchemesModel;
import com.intellij.openapi.keymap.Keymap;
import com.intellij.openapi.keymap.KeymapManager;
import com.intellij.openapi.keymap.impl.KeymapManagerImpl;
import com.intellij.openapi.util.Condition;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
import java.util.function.Consumer;
import java.util.function.Predicate;
import static com.intellij.openapi.keymap.KeyMapBundle.message;
import static com.intellij.openapi.util.SystemInfo.isMac;
import static com.intellij.openapi.util.text.StringUtil.isEmptyOrSpaces;
import static com.intellij.openapi.util.text.StringUtil.naturalCompare;
import static java.util.stream.Collectors.toList;
@@ -37,62 +44,180 @@ import static java.util.stream.Collectors.toList;
*
* @author Sergey.Malenkov
*/
final class KeymapSchemeManager {
final class KeymapSchemeManager extends AbstractSchemeActions<KeymapScheme> implements SchemesModel<KeymapScheme> {
private static final Condition<Keymap> FILTER = keymap -> !isMac || !KeymapManager.DEFAULT_IDEA_KEYMAP.equals(keymap.getName());
private final ArrayList<KeymapScheme> list = new ArrayList<>();
private final KeymapSelector selector;
KeymapSchemeManager(KeymapSelector selector) {
super(selector);
this.selector = selector;
}
Keymap getSelectedKeymap() {
KeymapScheme scheme = selector.getSelectedScheme();
return scheme == null ? null : scheme.getCurrent();
}
Keymap getMutableKeymap(Keymap keymap) {
KeymapScheme scheme = find(keymap);
if (scheme == null) return null;
if (scheme.isMutable()) return scheme.getMutable();
String name = message("new.keymap.name", keymap.getPresentableName());
//noinspection ForLoopThatDoesntUseLoopVariable
for (int i = 1; containsScheme(name, false); i++) {
name = message("new.indexed.keymap.name", keymap.getPresentableName(), i);
}
return copyScheme(scheme, name).getMutable();
}
void visitMutableKeymaps(Consumer<Keymap> consumer) {
for (KeymapScheme scheme : list) {
if (scheme.isMutable()) {
consumer.accept(scheme.getMutable());
}
}
}
@Override
protected Class<KeymapScheme> getSchemeType() {
return KeymapScheme.class;
}
@Override
protected void onSchemeChanged(@Nullable KeymapScheme scheme) {
selector.notifyConsumer(scheme);
}
@Override
public boolean isProjectScheme(@NotNull KeymapScheme scheme) {
return false;
}
@Override
public boolean canDuplicateScheme(@NotNull KeymapScheme scheme) {
return true;
}
@Override
protected void duplicateScheme(@NotNull KeymapScheme parent, @NotNull String name) {
copyScheme(parent, name);
}
@NotNull
private KeymapScheme copyScheme(@NotNull KeymapScheme parent, @NotNull String name) {
KeymapScheme scheme = parent.copy(name);
list.add(scheme);
selector.selectKeymap(scheme, true);
return scheme;
}
@Override
public boolean canDeleteScheme(@NotNull KeymapScheme scheme) {
return scheme.isMutable();
}
@Override
public void removeScheme(@NotNull KeymapScheme scheme) {
list.remove(scheme);
selector.selectKeymap(getSchemeToSelect(scheme.getParent()), true);
}
@Override
public boolean canRenameScheme(@NotNull KeymapScheme scheme) {
return scheme.isMutable();
}
@Override
protected void renameScheme(@NotNull KeymapScheme scheme, @NotNull String name) {
scheme.setName(name);
selector.selectKeymap(scheme, true);
}
@Override
public boolean containsScheme(@NotNull String name, boolean projectScheme) {
return null != find(scheme -> scheme.contains(name));
}
@Override
public boolean differsFromDefault(@NotNull KeymapScheme scheme) {
return scheme.canReset();
}
@Override
public boolean canResetScheme(@NotNull KeymapScheme scheme) {
return scheme.isMutable();
}
@Override
protected void resetScheme(@NotNull KeymapScheme scheme) {
scheme.reset();
selector.selectKeymap(scheme, true);
}
boolean canResetActionInKeymap(Keymap mutable, String actionId) {
KeymapScheme scheme = find(mutable);
return scheme != null && scheme.canReset(actionId);
}
void resetActionInKeymap(Keymap mutable, String actionId) {
KeymapScheme scheme = find(mutable);
if (scheme == null) return;
scheme.reset(actionId);
selector.selectKeymap(scheme, false);
}
private KeymapScheme find(Keymap keymap) {
return keymap == null ? null : find(scheme -> scheme.contains(keymap));
}
/**
* @param predicate a predicate to test a scheme
* @return a first scheme that belongs to the specified predicate, or {@code null}
*/
KeymapScheme find(@NotNull Predicate<KeymapScheme> predicate) {
private KeymapScheme find(@NotNull Predicate<KeymapScheme> predicate) {
for (KeymapScheme scheme : list) {
if (predicate.test(scheme)) return scheme;
}
return null;
}
/**
* @param scheme a scheme to add into the list of schemes
* @return the same scheme to select
*/
KeymapScheme add(@NotNull KeymapScheme scheme) {
list.add(scheme);
return scheme;
}
/**
* @param scheme a scheme to remove from the list of schemes
* @return a scheme to select
*/
KeymapScheme remove(@NotNull KeymapScheme scheme) {
list.remove(scheme);
return getSchemeToSelect(scheme.getParent());
}
/**
* Initializes a list of schemes from loaded keymaps.
*
* @return a scheme to select
* @see KeymapSelector#reset()
* @see KeymapPanel#reset()
*/
KeymapScheme reset() {
void reset() {
list.clear();
getKeymaps().forEach(keymap -> list.add(new KeymapScheme(keymap)));
return getSchemeToSelect(null);
selector.selectKeymap(getSchemeToSelect(null), true);
}
/**
* @param selected a selected scheme to activate corresponding keymap
* @return the same scheme to select
* @see KeymapSelector#apply()
* Applies a changes in the internal list of schemes.
*
* @return the error message if changes cannot be applied
* @see KeymapPanel#apply()
*/
KeymapScheme apply(KeymapScheme selected) {
String apply() {
HashSet<String> set = new HashSet<>();
for (KeymapScheme scheme : list) {
String name = scheme.getName();
if (isEmptyOrSpaces(name)) {
return message("configuration.all.keymaps.should.have.non.empty.names.error.message");
}
if (!set.add(name)) {
return message("configuration.all.keymaps.should.have.unique.names.error.message");
}
}
KeymapScheme selected = selector.getSelectedScheme();
Keymap active = selected == null ? null : selected.getOriginal();
List<Keymap> keymaps = list.stream().map(scheme -> scheme.apply()).collect(toList());
KeymapManagerImpl manager = (KeymapManagerImpl)KeymapManager.getInstance();
manager.setKeymaps(keymaps, active, FILTER);
return selected;
selector.notifyConsumer(selected);
return null;
}
/**
@@ -124,11 +249,11 @@ final class KeymapSchemeManager {
}
/**
* @param selected a selected scheme to test
* @return {@code true} if the current list of schemes differs from the list of loaded keymaps
* @see KeymapSelector#isModified()
* @see KeymapPanel#isModified()
*/
boolean isModified(KeymapScheme selected) {
boolean isModified() {
KeymapScheme selected = selector.getSelectedScheme();
Keymap active = selected == null ? null : selected.getOriginal();
if (!Objects.equals(active, KeymapManager.getInstance().getActiveKeymap())) return true;
@@ -140,8 +265,8 @@ final class KeymapSchemeManager {
return keymaps.hasNext() || schemes.hasNext();
}
List<KeymapScheme> getSchemes(boolean sorted) {
if (sorted) list.sort(COMPARATOR);
List<KeymapScheme> getSchemes() {
list.sort(COMPARATOR);
return list;
}
@@ -23,18 +23,15 @@ import com.intellij.openapi.ui.MessageType;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.HashSet;
import java.util.function.Consumer;
import static com.intellij.openapi.actionSystem.impl.ActionToolbarImpl.updateAllToolbarsImmediately;
import static com.intellij.openapi.keymap.KeyMapBundle.message;
import static com.intellij.openapi.util.text.StringUtil.isEmptyOrSpaces;
/**
* @author Sergey.Malenkov
*/
final class KeymapSelector extends SimpleSchemesPanel<KeymapScheme> implements SchemesModel<KeymapScheme> {
private final KeymapSchemeManager manager = new KeymapSchemeManager();
final class KeymapSelector extends SimpleSchemesPanel<KeymapScheme> {
private KeymapSchemeManager manager;
private final Consumer<Keymap> consumer;
private String messageReplacement;
private boolean messageShown;
@@ -44,87 +41,15 @@ final class KeymapSelector extends SimpleSchemesPanel<KeymapScheme> implements S
this.consumer = consumer;
}
/**
* @see KeymapPanel#reset()
*/
void reset() {
selectKeymap(manager.reset(), true);
}
/**
* @see KeymapPanel#apply()
*/
String apply() {
HashSet<String> set = new HashSet<>();
for (KeymapScheme scheme : manager.getSchemes(false)) {
String name = scheme.getName();
if (isEmptyOrSpaces(name)) {
return message("configuration.all.keymaps.should.have.non.empty.names.error.message");
}
if (!set.add(name)) {
return message("configuration.all.keymaps.should.have.unique.names.error.message");
}
}
notifyConsumer(manager.apply(getSelectedScheme()));
updateAllToolbarsImmediately();
return null;
}
/**
* @see KeymapPanel#isModified()
*/
boolean isModified() {
return manager.isModified(getSelectedScheme());
}
void visitMutableKeymaps(Consumer<Keymap> consumer) {
for (KeymapScheme scheme : manager.getSchemes(false)) {
if (scheme.isMutable()) {
consumer.accept(scheme.getMutable());
}
}
}
Keymap getSelectedKeymap() {
KeymapScheme scheme = getSelectedScheme();
return scheme == null ? null : scheme.getCurrent();
}
Keymap getMutableKeymap(Keymap keymap) {
KeymapScheme scheme = find(keymap);
if (scheme == null) return null;
if (scheme.isMutable()) return scheme.getMutable();
String name = message("new.keymap.name", keymap.getPresentableName());
//noinspection ForLoopThatDoesntUseLoopVariable
for (int i = 1; containsScheme(name, false); i++) {
name = message("new.indexed.keymap.name", keymap.getPresentableName(), i);
}
KeymapScheme copy = manager.add(scheme.copy(name));
selectKeymap(copy, true);
return copy.getMutable();
}
boolean canResetActionInKeymap(Keymap mutable, String actionId) {
KeymapScheme scheme = find(mutable);
return scheme != null && scheme.canReset(actionId);
}
void resetActionInKeymap(Keymap mutable, String actionId) {
KeymapScheme scheme = find(mutable);
if (scheme == null) return;
scheme.reset(actionId);
selectKeymap(scheme, false);
}
private KeymapScheme find(Keymap keymap) {
return keymap == null ? null : manager.find(scheme -> scheme.contains(keymap));
public KeymapSchemeManager getManager() {
if (manager == null) manager = new KeymapSchemeManager(this);
return manager;
}
@NotNull
@Override
public SchemesModel<KeymapScheme> getModel() {
return this;
return getManager();
}
@Override
@@ -134,73 +59,11 @@ final class KeymapSelector extends SimpleSchemesPanel<KeymapScheme> implements S
@Override
protected AbstractSchemeActions<KeymapScheme> createSchemeActions() {
return new AbstractSchemeActions<KeymapScheme>(this) {
@Override
protected Class<KeymapScheme> getSchemeType() {
return KeymapScheme.class;
}
@Override
protected void onSchemeChanged(@Nullable KeymapScheme scheme) {
if (!internal) notifyConsumer(scheme);
}
@Override
protected void resetScheme(@NotNull KeymapScheme scheme) {
scheme.reset();
selectKeymap(scheme, true);
}
@Override
protected void renameScheme(@NotNull KeymapScheme scheme, @NotNull String name) {
scheme.setName(name);
selectKeymap(scheme, true);
}
@Override
protected void duplicateScheme(@NotNull KeymapScheme parent, @NotNull String name) {
selectKeymap(manager.add(parent.copy(name)), true);
}
};
return getManager();
}
@Override
public void removeScheme(@NotNull KeymapScheme scheme) {
selectKeymap(manager.remove(scheme), true);
}
@Override
public boolean canResetScheme(@NotNull KeymapScheme scheme) {
return scheme.isMutable();
}
@Override
public boolean canRenameScheme(@NotNull KeymapScheme scheme) {
return scheme.isMutable();
}
@Override
public boolean canDuplicateScheme(@NotNull KeymapScheme scheme) {
return true;
}
@Override
public boolean canDeleteScheme(@NotNull KeymapScheme scheme) {
return scheme.isMutable();
}
@Override
public boolean containsScheme(@NotNull String name, boolean isProjectScheme) {
return null != manager.find(scheme -> scheme.contains(name));
}
@Override
public boolean isProjectScheme(@NotNull KeymapScheme scheme) {
return false;
}
@Override
public int getIndent(@NotNull KeymapScheme scheme) {
protected int getIndent(@NotNull KeymapScheme scheme) {
return scheme.isMutable() ? 1 : 0;
}
@@ -219,11 +82,6 @@ final class KeymapSelector extends SimpleSchemesPanel<KeymapScheme> implements S
return true;
}
@Override
public boolean differsFromDefault(@NotNull KeymapScheme scheme) {
return scheme.canReset();
}
@Override
public void showMessage(@Nullable String message, @NotNull MessageType messageType) {
messageShown = true;
@@ -236,7 +94,9 @@ final class KeymapSelector extends SimpleSchemesPanel<KeymapScheme> implements S
super.showMessage(messageReplacement, MessageType.INFO);
}
private void notifyConsumer(KeymapScheme scheme) {
void notifyConsumer(KeymapScheme scheme) {
if (internal) return;
Keymap keymap = scheme == null ? null : scheme.getParent();
messageReplacement = keymap == null ? null : message("based.on.keymap.label", keymap.getPresentableName());
if (!messageShown) clearMessage();
@@ -244,10 +104,10 @@ final class KeymapSelector extends SimpleSchemesPanel<KeymapScheme> implements S
consumer.accept(scheme == null ? null : scheme.getCurrent());
}
private void selectKeymap(KeymapScheme scheme, boolean reset) {
void selectKeymap(KeymapScheme scheme, boolean reset) {
try {
internal = true;
if (reset) resetSchemes(manager.getSchemes(true));
if (reset) resetSchemes(getManager().getSchemes());
if (scheme != null) selectScheme(scheme);
}
finally {