From f9e1dbd484ccb7790611986156eb3e63f979e208 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 10 Dec 2015 10:51:07 +0100 Subject: [PATCH 01/10] Remove unused method --- app/src/processing/app/EditorHeader.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/app/src/processing/app/EditorHeader.java b/app/src/processing/app/EditorHeader.java index a1e100aea43..56e6ae2cecb 100644 --- a/app/src/processing/app/EditorHeader.java +++ b/app/src/processing/app/EditorHeader.java @@ -376,11 +376,6 @@ public void actionPerformed(ActionEvent e) { } - public void deselectMenu() { - repaint(); - } - - public Dimension getPreferredSize() { return getMinimumSize(); } From 8c176e74291e79cdfa4e12f98e9d5d710c163a2a Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 10 Dec 2015 12:06:10 +0100 Subject: [PATCH 02/10] Remove some old, commented out code --- app/src/processing/app/EditorHeader.java | 56 ------------------------ 1 file changed, 56 deletions(-) diff --git a/app/src/processing/app/EditorHeader.java b/app/src/processing/app/EditorHeader.java index 56e6ae2cecb..e4c93ab7cd7 100644 --- a/app/src/processing/app/EditorHeader.java +++ b/app/src/processing/app/EditorHeader.java @@ -72,8 +72,6 @@ public class EditorHeader extends JComponent { static Image[][] pieces; - // - Image offscreen; int sizeW, sizeH; int imageW, imageH; @@ -236,7 +234,6 @@ public void rebuild() { public void rebuildMenu() { - //System.out.println("rebuilding"); if (menu != null) { menu.removeAll(); @@ -246,54 +243,9 @@ public void rebuildMenu() { popup = menu.getPopupMenu(); add(popup); popup.setLightWeightPopupEnabled(true); - - /* - popup.addPopupMenuListener(new PopupMenuListener() { - public void popupMenuCanceled(PopupMenuEvent e) { - // on redraw, the isVisible() will get checked. - // actually, a repaint may be fired anyway, so this - // may be redundant. - repaint(); - } - - public void popupMenuWillBecomeInvisible(PopupMenuEvent e) { } - public void popupMenuWillBecomeVisible(PopupMenuEvent e) { } - }); - */ } JMenuItem item; - // maybe this shouldn't have a command key anyways.. - // since we're not trying to make this a full ide.. - //item = Editor.newJMenuItem("New", 'T'); - - /* - item = Editor.newJMenuItem("Previous", KeyEvent.VK_PAGE_UP); - item.addActionListener(new ActionListener() { - public void actionPerformed(ActionEvent e) { - System.out.println("prev"); - } - }); - if (editor.sketch != null) { - item.setEnabled(editor.sketch.codeCount > 1); - } - menu.add(item); - - item = Editor.newJMenuItem("Next", KeyEvent.VK_PAGE_DOWN); - item.addActionListener(new ActionListener() { - public void actionPerformed(ActionEvent e) { - System.out.println("ext"); - } - }); - if (editor.sketch != null) { - item.setEnabled(editor.sketch.codeCount > 1); - } - menu.add(item); - - menu.addSeparator(); - */ - - //item = new JMenuItem("New Tab"); item = Editor.newJMenuItemShift(tr("New Tab"), 'N'); item.addActionListener(new ActionListener() { public void actionPerformed(ActionEvent e) { @@ -306,12 +258,6 @@ public void actionPerformed(ActionEvent e) { item.addActionListener(new ActionListener() { public void actionPerformed(ActionEvent e) { editor.getSketch().handleRenameCode(); - /* - // this is already being called by nameCode(), the second stage of rename - if (editor.sketch.current == editor.sketch.code[0]) { - editor.sketchbook.rebuildMenus(); - } - */ } }); menu.add(item); @@ -330,8 +276,6 @@ public void actionPerformed(ActionEvent event) { menu.addSeparator(); - // KeyEvent.VK_LEFT and VK_RIGHT will make Windows beep - item = new JMenuItem(tr("Previous Tab")); KeyStroke ctrlAltLeft = KeyStroke .getKeyStroke(KeyEvent.VK_LEFT, Editor.SHORTCUT_ALT_KEY_MASK); From 957331299b729a4e21b1fe994159e6177bd60f34 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 10 Dec 2015 14:51:46 +0100 Subject: [PATCH 03/10] Use Actions to simplify the EditorHeader popup menu building Instead of defining JMenuItems, setting accelerator keys, and attaching an ActionListener inline, this first defines a list of actions (with a name, optional accelerator key and using a lambda as the action listener). Then menu items are added, that simply refer to the previously defined actions. The actions are defined in a inner class named Actions, of which one instance is created. This allows grouping the actions together inside the `actions` attribute, and allows external access to the actions (in case the same action is present in multiple menus, or otherwise performed from other places). This might not be the best way to expose these actions, and perhaps they should be moved elsewhere, but for now this seems like a good start. This adds new helper classes SimpleAction, to allow more consisely defining Action instances, and a new class Keys, to allow consisely defining keyboard shortcuts. --- app/src/processing/app/EditorHeader.java | 85 ++++++------- app/src/processing/app/helpers/Keys.java | 83 +++++++++++++ .../processing/app/helpers/SimpleAction.java | 112 ++++++++++++++++++ 3 files changed, 228 insertions(+), 52 deletions(-) create mode 100644 app/src/processing/app/helpers/Keys.java create mode 100644 app/src/processing/app/helpers/SimpleAction.java diff --git a/app/src/processing/app/EditorHeader.java b/app/src/processing/app/EditorHeader.java index e4c93ab7cd7..97836d672c5 100644 --- a/app/src/processing/app/EditorHeader.java +++ b/app/src/processing/app/EditorHeader.java @@ -22,7 +22,10 @@ */ package processing.app; + +import processing.app.helpers.Keys; import processing.app.helpers.OSUtils; +import processing.app.helpers.SimpleAction; import processing.app.tools.MenuScroller; import static processing.app.I18n.tr; @@ -76,6 +79,31 @@ public class EditorHeader extends JComponent { int sizeW, sizeH; int imageW, imageH; + public class Actions { + public final Action newTab = new SimpleAction(tr("New Tab"), + Keys.ctrlShift(KeyEvent.VK_N), + () -> editor.getSketch().handleNewCode()); + + public final Action renameTab = new SimpleAction(tr("Rename"), + () -> editor.getSketch().handleRenameCode()); + + public final Action deleteTab = new SimpleAction(tr("Delete"), () -> { + try { + editor.getSketch().handleDeleteCode(); + } catch (IOException e) { + e.printStackTrace(); + } + }); + + public final Action prevTab = new SimpleAction(tr("Previous Tab"), + Keys.ctrlAlt(KeyEvent.VK_LEFT), + () -> editor.sketch.handlePrevCode()); + + public final Action nextTab = new SimpleAction(tr("Next Tab"), + Keys.ctrlAlt(KeyEvent.VK_RIGHT), + () -> editor.sketch.handleNextCode()); + } + public Actions actions = new Actions(); public EditorHeader(Editor eddie) { this.editor = eddie; // weird name for listener @@ -246,59 +274,12 @@ public void rebuildMenu() { } JMenuItem item; - item = Editor.newJMenuItemShift(tr("New Tab"), 'N'); - item.addActionListener(new ActionListener() { - public void actionPerformed(ActionEvent e) { - editor.getSketch().handleNewCode(); - } - }); - menu.add(item); - - item = new JMenuItem(tr("Rename")); - item.addActionListener(new ActionListener() { - public void actionPerformed(ActionEvent e) { - editor.getSketch().handleRenameCode(); - } - }); - menu.add(item); - - item = new JMenuItem(tr("Delete")); - item.addActionListener(new ActionListener() { - public void actionPerformed(ActionEvent event) { - try { - editor.getSketch().handleDeleteCode(); - } catch (IOException e) { - e.printStackTrace(); - } - } - }); - menu.add(item); - + menu.add(new JMenuItem(actions.newTab)); + menu.add(new JMenuItem(actions.renameTab)); + menu.add(new JMenuItem(actions.deleteTab)); menu.addSeparator(); - - item = new JMenuItem(tr("Previous Tab")); - KeyStroke ctrlAltLeft = KeyStroke - .getKeyStroke(KeyEvent.VK_LEFT, Editor.SHORTCUT_ALT_KEY_MASK); - item.setAccelerator(ctrlAltLeft); - item.addActionListener(new ActionListener() { - @Override - public void actionPerformed(ActionEvent e) { - editor.sketch.handlePrevCode(); - } - }); - menu.add(item); - - item = new JMenuItem(tr("Next Tab")); - KeyStroke ctrlAltRight = KeyStroke - .getKeyStroke(KeyEvent.VK_RIGHT, Editor.SHORTCUT_ALT_KEY_MASK); - item.setAccelerator(ctrlAltRight); - item.addActionListener(new ActionListener() { - @Override - public void actionPerformed(ActionEvent e) { - editor.sketch.handleNextCode(); - } - }); - menu.add(item); + menu.add(new JMenuItem(actions.prevTab)); + menu.add(new JMenuItem(actions.nextTab)); Sketch sketch = editor.getSketch(); if (sketch != null) { diff --git a/app/src/processing/app/helpers/Keys.java b/app/src/processing/app/helpers/Keys.java new file mode 100644 index 00000000000..ff6702c1168 --- /dev/null +++ b/app/src/processing/app/helpers/Keys.java @@ -0,0 +1,83 @@ +/* + * This file is part of Arduino. + * + * Copyright 2015 Matthijs Kooijman + * + * Arduino is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + * As a special exception, you may use this file as part of a free software + * library without restriction. Specifically, if other files instantiate + * templates or use macros or inline functions from this file, or you compile + * this file and link it with other files to produce an executable, this + * file does not by itself cause the resulting executable to be covered by + * the GNU General Public License. This exception does not however + * invalidate any other reasons why the executable file might be covered by + * the GNU General Public License. + */ + +package processing.app.helpers; + +import java.awt.Toolkit; +import java.awt.event.InputEvent; +import java.beans.PropertyChangeEvent; + +import javax.swing.Action; +import javax.swing.JComponent; +import javax.swing.KeyStroke; + +/** + * This class contains some keybinding-related helper methods. + */ +public class Keys { + + private static final int CTRL = Toolkit.getDefaultToolkit() + .getMenuShortcutKeyMask(); + + /** + * Creates a KeyCode for the "menu shortcut" + the key passed in. By default, + * the menu shortcut is the ctrl key (hence the method name), but platforms + * might use a different key (like the Apple key on OSX). + * + * keyCode should be a KeyEvent.VK_* constant (it can also be a char constant, + * but this does not work for all characters, so is not recommended). + */ + public static KeyStroke ctrl(int keyCode) { + return KeyStroke.getKeyStroke(keyCode, CTRL); + } + + /** + * Creates a KeyCode for the "menu shortcut" + shift + the key passed in. By + * default, the menu shortcut is the ctrl key (hence the method name), but + * platforms might use a different key (like the Apple key on OSX). + * + * keyCode should be a KeyEvent.VK_* constant (it can also be a char constant, + * but this does not work for all characters, so is not recommended). + */ + public static KeyStroke ctrlShift(int keyCode) { + return KeyStroke.getKeyStroke(keyCode, CTRL | InputEvent.SHIFT_MASK); + } + + /** + * Creates a KeyCode for the "menu shortcut" + shift + the key passed in. By + * default, the menu shortcut is the ctrl key (hence the method name), but + * platforms might use a different key (like the Apple key on OSX). + * + * keyCode should be a KeyEvent.VK_* constant (it can also be a char constant, + * but this does not work for all characters, so is not recommended). + */ + public static KeyStroke ctrlAlt(int keyCode) { + return KeyStroke.getKeyStroke(keyCode, CTRL | InputEvent.ALT_MASK); + } +} diff --git a/app/src/processing/app/helpers/SimpleAction.java b/app/src/processing/app/helpers/SimpleAction.java new file mode 100644 index 00000000000..85c096392a2 --- /dev/null +++ b/app/src/processing/app/helpers/SimpleAction.java @@ -0,0 +1,112 @@ +/* + * This file is part of Arduino. + * + * Copyright 2015 Matthijs Kooijman + * + * Arduino is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + * As a special exception, you may use this file as part of a free software + * library without restriction. Specifically, if other files instantiate + * templates or use macros or inline functions from this file, or you compile + * this file and link it with other files to produce an executable, this + * file does not by itself cause the resulting executable to be covered by + * the GNU General Public License. This exception does not however + * invalidate any other reasons why the executable file might be covered by + * the GNU General Public License. + */ + +package processing.app.helpers; + +import java.awt.event.ActionEvent; +import java.awt.event.ActionListener; + +import javax.swing.AbstractAction; +import javax.swing.KeyStroke; + +/** + * Class to easily define instances of the Swing Action interface. + * + * When using AbstractAction, you have to create a subclass that implements the + * actionPerformed() method, and sets attributes in the constructor, which gets + * verbose quickly. This class implements actionPerformed for you, and forwards + * it to the ActionListener passed to the constructor (intended to be a lambda + * expression). Additional Action attributes can be set by passing constructor + * arguments. + * + * The name of this class refers to the fact that it's simple to create an + * action using this class, but perhaps a better name can be found for it. + * + * @see javax.swing.Action + */ +public class SimpleAction extends AbstractAction { + private ActionListener listener; + + /** + * Version of ActionListener that does not take an ActionEvent as an argument + * This can be used when you do not care about the event itself, just that it + * happened, typically for passing a argumentless lambda or method reference + * to the SimpleAction constructor. + */ + public interface AnonymousActionListener { + public void actionPerformed(); + } + + public SimpleAction(String name, ActionListener listener) { + this(name, null, null, listener); + } + + public SimpleAction(String name, AnonymousActionListener listener) { + this(name, null, null, listener); + } + + public SimpleAction(String name, KeyStroke accelerator, + ActionListener listener) { + this(name, null, accelerator, listener); + } + + public SimpleAction(String name, KeyStroke accelerator, + AnonymousActionListener listener) { + this(name, null, accelerator, listener); + } + + public SimpleAction(String name, String description, + ActionListener listener) { + this(name, description, null, listener); + } + + public SimpleAction(String name, String description, + AnonymousActionListener listener) { + this(name, description, null, listener); + } + + public SimpleAction(String name, String description, KeyStroke accelerator, + AnonymousActionListener listener) { + this(name, description, accelerator, + (ActionEvent) -> listener.actionPerformed()); + } + + public SimpleAction(String name, String description, KeyStroke accelerator, + ActionListener listener) { + this.putValue(NAME, name); + this.putValue(SHORT_DESCRIPTION, description); + this.putValue(ACCELERATOR_KEY, accelerator); + this.listener = listener; + } + + @Override + public void actionPerformed(ActionEvent e) { + listener.actionPerformed(e); + } +} From 2f5375d52383fdbe76b523ae81542aba9087d615 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 10 Dec 2015 15:47:19 +0100 Subject: [PATCH 04/10] Fix accelerator keybindings for the tab menu Some items in this menu had accelerator keys (shortcuts) defined. Normally, this automatically takes care of registering such keybindings when the menu item is added to a menu. However, this requires adding the item (indirectly) to a menubar, which is again added to a window. Since the tab menu is just a separate popup menu, this did not work. It seems an attempt was made to fix this by adding the popup menu to the EditorHeader JComponent, which indeed made the keybindings work. However, this is a hack at best, and as soon as the popup menu was opened, it would be moved to another container and again detached when the menu was closed, breaking the keyboard shortcuts again (re-adding the popup menu turned out not to work either, then the menu would actually be drawn on top of the tab bar). To properly fix this, keybindings for the menu items are added to the EditorHeader itself. By looking at the existing accelerator keystroke property of the actions, there is no need to duplicate the keystrokes themselves, and the displayed value will always match the actually bound value. To simplify this, some methods are added to the Keys helper class, which will likely come in handy in other places as well. --- app/src/processing/app/EditorHeader.java | 10 ++- app/src/processing/app/helpers/Keys.java | 85 ++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/app/src/processing/app/EditorHeader.java b/app/src/processing/app/EditorHeader.java index 97836d672c5..ca0a70774e7 100644 --- a/app/src/processing/app/EditorHeader.java +++ b/app/src/processing/app/EditorHeader.java @@ -102,6 +102,15 @@ public class Actions { public final Action nextTab = new SimpleAction(tr("Next Tab"), Keys.ctrlAlt(KeyEvent.VK_RIGHT), () -> editor.sketch.handleNextCode()); + + Actions() { + // Explicitly bind keybindings for the actions with accelerators above + // Normally, this happens automatically for any actions bound to menu + // items, but only for menus attached to a window, not for popup menus. + Keys.bind(EditorHeader.this, newTab); + Keys.bind(EditorHeader.this, prevTab); + Keys.bind(EditorHeader.this, nextTab); + } } public Actions actions = new Actions(); @@ -269,7 +278,6 @@ public void rebuildMenu() { menu = new JMenu(); MenuScroller.setScrollerFor(menu); popup = menu.getPopupMenu(); - add(popup); popup.setLightWeightPopupEnabled(true); } JMenuItem item; diff --git a/app/src/processing/app/helpers/Keys.java b/app/src/processing/app/helpers/Keys.java index ff6702c1168..e4c797ed2e8 100644 --- a/app/src/processing/app/helpers/Keys.java +++ b/app/src/processing/app/helpers/Keys.java @@ -42,6 +42,91 @@ */ public class Keys { + /** + * Register a keybinding in the given components WHEN_IN_FOCUSED_WINDOW input + * map, runing the given action when the action's accelerator key is pressed. + * + * Note that this is typically automatically handled when the action is + * assigned to a JMenuItem, but it can still be needed for other actions, or + * actions mapped to JMenuItems in a popup menu that is not added to any + * window normally (and thus does not fire when the popup is closed). + * + * When the action is disabled, the keybinding is unregistered, and when it is + * enabled, it is registered again. + */ + public static void bind(final JComponent component, final Action action) { + bind(component, action, + (KeyStroke) action.getValue(Action.ACCELERATOR_KEY)); + } + + /** + * Register a keybinding, running the given action when the given keystroke is + * pressed when the given component is in the focused window. + * + * This is typically used to bind an additional keystroke to a menu item, in + * addition to the primary accelerator key. + * + * When the action is disabled, the keybinding is unregistered, and when it is + * enabled, it is registered again. + */ + public static void bind(final JComponent component, final Action action, + KeyStroke keystroke) { + bind(component, action, keystroke, JComponent.WHEN_IN_FOCUSED_WINDOW); + } + + /** + * Register a keybinding to be handled in given condition, running the given + * action when the given keystroke is pressed. + * + * When the action is disabled, the keybinding is unregistered, and when it is + * enabled, it is registered again. + * + * @param component + * The component to register the keybinding on. + * @param action + * The action to run when the keystroke is pressed + * @param action + * The keystroke to bind + * @param condition + * The condition under which to run the keystroke. Should be one of + * JComponent.WHEN_FOCUSED, + * JComponent.WHEN_ANCESTOR_OF_FOCUSED_COMPONENT or + * JComponent.WHEN_IN_FOCUSED_WINDOW. + */ + public static void bind(final JComponent component, final Action action, + KeyStroke keystroke, int condition) { + // The input map maps keystrokes to arbitrary objects (originally strings + // that described the option, we just use the Action object itself). + if (action.isEnabled()) + enableBind(component, action, keystroke, condition); + + // The action map maps the arbitrary option to an Action to execute. These + // be kept in the component even when the action is disabled. + component.getActionMap().put(action, action); + + // Enable and disable the binding when the action is enabled / disabled. + action.addPropertyChangeListener((PropertyChangeEvent e) -> { + if (e.getPropertyName().equals("enabled")) { + if (e.getNewValue().equals(Boolean.TRUE)) + enableBind(component, action, keystroke, condition); + else + disableBind(component, action, keystroke, condition); + } + }); + } + + private static void enableBind(final JComponent component, + final Action action, final KeyStroke keystroke, + int condition) { + component.getInputMap(condition).put(keystroke, action); + } + + private static void disableBind(final JComponent component, + final Action action, + final KeyStroke keystroke, int condition) { + component.getInputMap(condition).put(keystroke, action); + } + private static final int CTRL = Toolkit.getDefaultToolkit() .getMenuShortcutKeyMask(); From a3ba935a573640b2be6ca73927ce853d1322b4d4 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 10 Dec 2015 15:13:46 +0100 Subject: [PATCH 05/10] Slightly simplify EditorHeader tab selection menu items Previously, this would use a single ActionListener object, and pass the filename of the file to switch to in the action command. This means that whenever switching the filename needs to be looked up. This commit instead uses a lambda to capture the index of the tab to switch to for every tab (so it uses a different ActionListener for each tab). --- app/src/processing/app/EditorHeader.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/app/src/processing/app/EditorHeader.java b/app/src/processing/app/EditorHeader.java index ca0a70774e7..ee25b6a2f7d 100644 --- a/app/src/processing/app/EditorHeader.java +++ b/app/src/processing/app/EditorHeader.java @@ -292,17 +292,14 @@ public void rebuildMenu() { Sketch sketch = editor.getSketch(); if (sketch != null) { menu.addSeparator(); - - ActionListener jumpListener = new ActionListener() { - public void actionPerformed(ActionEvent e) { - editor.getSketch().setCurrentCode(e.getActionCommand()); - } - }; + int i = 0; for (SketchCode code : sketch.getCodes()) { + final int index = i++; item = new JMenuItem(code.isExtension(sketch.getDefaultExtension()) ? code.getPrettyName() : code.getFileName()); - item.setActionCommand(code.getFileName()); - item.addActionListener(jumpListener); + item.addActionListener((ActionEvent e) -> { + editor.getSketch().setCurrentCode(index); + }); menu.add(item); } } From e98285f900ec4e36b4c1f551caad7e3cc1849e01 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Wed, 9 Dec 2015 11:32:57 +0100 Subject: [PATCH 06/10] Remove duplicate ctrl+alt+left/right handling These key combinations were registered as accelerator keystrokes in the tab bar popup menu, but also handled by EditorListener. This was probably added in an attempt to work around the broken accelerator keys on the tab bar popup menus, but in practice this only meant that the shortcut would sometimes (and now that the accelerator keys are fixed, always) switch tabs *twice*. Removing the handling from EditorListener helps to fix this. References: #4228 --- app/src/processing/app/EditorListener.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/app/src/processing/app/EditorListener.java b/app/src/processing/app/EditorListener.java index cbd082cfcad..51157881295 100644 --- a/app/src/processing/app/EditorListener.java +++ b/app/src/processing/app/EditorListener.java @@ -54,15 +54,6 @@ public void keyPressed(KeyEvent event) { sketch.handlePrevCode(); } - // Navigation.. - if ((event.getModifiers() & CTRL_ALT) == CTRL_ALT) { - if (code == KeyEvent.VK_LEFT) { - sketch.handlePrevCode(); - } else if (code == KeyEvent.VK_RIGHT) { - sketch.handleNextCode(); - } - } - // if (event.isAltDown() && code == KeyEvent.VK_T) { // int line = textarea.getCaretLineNumber(); // textarea.setActiveLineRange(line, line + 3); From fc4b2028fabfe9e252c0e60813e8743c7facd853 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 10 Dec 2015 18:03:11 +0100 Subject: [PATCH 07/10] Move ctrl-tab and ctrl-shift-tab handling into EditorHeader Previously, EditorListener handled these keys, by registering a handler in the text area. This meant that they would only work while the text area was focused. By registering the keys as WHEN_IN_FOCUSED_WINDOW bindings, they can always be handled, and EditorHeader seems like a more appropriate place. Note that this does not currently work (so this commit breaks these keys), since these keys are also handled in a few different places as well, preventing these newly added keybindings to take effect. This will be fixed in the next commit. One additional change is that previously, these keybindings did not work when the text area was readonly. This was probably a remnant from when EditorListener took care of a lot of other editing keybindings, but this does not make much sense anymore now. Finally, with the old bindings, ctrl-shift-tab did not (seem to) work. What happened is that the binding for ctrl-tab did not check the shift state, so both bindings would fire on ctrl-shift-tab, switching forward and back again, making it seem the keys did not work. The Swing keybinding mechanism that is now used for these bindings checks the complete keystroke, including all modifier keys, so this problem is fixed by this change. References #195 --- app/src/processing/app/EditorHeader.java | 4 ++++ app/src/processing/app/EditorListener.java | 25 ---------------------- 2 files changed, 4 insertions(+), 25 deletions(-) diff --git a/app/src/processing/app/EditorHeader.java b/app/src/processing/app/EditorHeader.java index ee25b6a2f7d..32597b2c92c 100644 --- a/app/src/processing/app/EditorHeader.java +++ b/app/src/processing/app/EditorHeader.java @@ -110,6 +110,10 @@ public class Actions { Keys.bind(EditorHeader.this, newTab); Keys.bind(EditorHeader.this, prevTab); Keys.bind(EditorHeader.this, nextTab); + + // Add alternative keybindings to switch tabs + Keys.bind(EditorHeader.this, prevTab, Keys.ctrlShift(KeyEvent.VK_TAB)); + Keys.bind(EditorHeader.this, nextTab, Keys.ctrl(KeyEvent.VK_TAB)); } } public Actions actions = new Actions(); diff --git a/app/src/processing/app/EditorListener.java b/app/src/processing/app/EditorListener.java index 51157881295..e5203b4681b 100644 --- a/app/src/processing/app/EditorListener.java +++ b/app/src/processing/app/EditorListener.java @@ -34,31 +34,6 @@ public void keyTyped(KeyEvent event) { @Override public void keyPressed(KeyEvent event) { - - SketchTextArea textarea = editor.getTextArea(); - - if (!textarea.isEditable()) return; - - Sketch sketch = editor.getSketch(); - - int code = event.getKeyCode(); - - // Navigation.. - if ((event.getModifiers() & CTRL) == CTRL && code == KeyEvent.VK_TAB) { - sketch.handleNextCode(); - } - - // Navigation.. - // FIXME: not working on LINUX !!! - if ((event.getModifiers() & CTRL_SHIFT) == CTRL_SHIFT && code == KeyEvent.VK_TAB) { - sketch.handlePrevCode(); - } - -// if (event.isAltDown() && code == KeyEvent.VK_T) { -// int line = textarea.getCaretLineNumber(); -// textarea.setActiveLineRange(line, line + 3); -// } - } @Override From f06820713e7464d1bfd6e57187f28dc451e005a8 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 11 Dec 2015 11:37:17 +0100 Subject: [PATCH 08/10] Make ctrl-tab and ctrl-shift-tab work again In the previous commit, these bindings were moved to EditorTab and registered in a cleaner way, but this move also allows more components to hijack these keystrokes and prevent them from reaching EditorTab. This commit makes the keybindings work again, by preventing other components from handling the keys. In particular: - JSplitPane had a binding to switch between its two panes, which is now removed after creating the JSplitPane. - The default focus traversal manager in Swing uses these keys to traverse focus (in addition to the the normal tab and shift-tab keys). By removing these keys from the set of "focus traversal keys" defined for the window, this should be prevented when the focus is on any component inside the window. - JTextPane didn't respond to the previous modification of the window-default focus traversal keys, since it defines its own set (to only contain ctrl-tab and ctrl-shift-tab, but not tab and shift-tab, for undocumented reasons). To fix this, focus traversal is simply disabled on the JTextPane, since this wasn't really being used anyway. There was some code in SketchTextArea that tried to modify the focus traversal keys for just the text area, which is now removed. This code wasn't really useful, since focus traversal is disabled for the text area already. Also, the code contained a bug where it would not actually set the new set of keys for the backward focus traversal. Closes #195 --- app/src/processing/app/Editor.java | 7 ++ app/src/processing/app/EditorConsole.java | 1 + app/src/processing/app/EditorHeader.java | 21 ++++++ app/src/processing/app/helpers/Keys.java | 65 +++++++++++++++++++ .../processing/app/syntax/SketchTextArea.java | 27 -------- 5 files changed, 94 insertions(+), 27 deletions(-) diff --git a/app/src/processing/app/Editor.java b/app/src/processing/app/Editor.java index 9a30f5250b8..c922a9c557d 100644 --- a/app/src/processing/app/Editor.java +++ b/app/src/processing/app/Editor.java @@ -38,6 +38,7 @@ import org.fife.ui.rtextarea.RTextScrollPane; import processing.app.debug.RunnerException; import processing.app.forms.PasswordAuthorizationDialog; +import processing.app.helpers.Keys; import processing.app.helpers.OSUtils; import processing.app.helpers.PreferencesMapException; import processing.app.legacy.PApplet; @@ -312,6 +313,12 @@ public void windowDeactivated(WindowEvent e) { // to fix ugliness.. normally macosx java 1.3 puts an // ugly white border around this object, so turn it off. splitPane.setBorder(null); + // By default, the split pane binds Ctrl-Tab and Ctrl-Shift-Tab for changing + // focus. Since we do not use that, but want to use these shortcuts for + // switching tabs, remove the bindings from the split pane. This allows the + // events to bubble up and be handled by the EditorHeader. + Keys.killBinding(splitPane, Keys.ctrl(KeyEvent.VK_TAB)); + Keys.killBinding(splitPane, Keys.ctrlShift(KeyEvent.VK_TAB)); // the default size on windows is too small and kinda ugly int dividerSize = PreferencesData.getInteger("editor.divider.size"); diff --git a/app/src/processing/app/EditorConsole.java b/app/src/processing/app/EditorConsole.java index f39f98cb84a..f31a01b00f9 100644 --- a/app/src/processing/app/EditorConsole.java +++ b/app/src/processing/app/EditorConsole.java @@ -63,6 +63,7 @@ public EditorConsole() { consoleTextPane.setEditable(false); DefaultCaret caret = (DefaultCaret) consoleTextPane.getCaret(); caret.setUpdatePolicy(DefaultCaret.NEVER_UPDATE); + consoleTextPane.setFocusTraversalKeysEnabled(false); Color backgroundColour = Theme.getColor("console.color"); consoleTextPane.setBackground(backgroundColour); diff --git a/app/src/processing/app/EditorHeader.java b/app/src/processing/app/EditorHeader.java index 32597b2c92c..84dc49df4c1 100644 --- a/app/src/processing/app/EditorHeader.java +++ b/app/src/processing/app/EditorHeader.java @@ -118,6 +118,27 @@ public class Actions { } public Actions actions = new Actions(); + /** + * Called whenever we, or any of our ancestors, is added to a container. + */ + public void addNotify() { + super.addNotify(); + /* + * Once we get added to a window, remove Ctrl-Tab and Ctrl-Shift-Tab from + * the keys used for focus traversal (so our bindings for these keys will + * work). All components inherit from the window eventually, so this should + * work whenever the focus is inside our window. Some components (notably + * JTextPane / JEditorPane) keep their own focus traversal keys, though, and + * have to be treated individually (either the same as below, or by + * disabling focus traversal entirely). + */ + Window window = SwingUtilities.getWindowAncestor(this); + if (window != null) { + Keys.killFocusTraversalBinding(window, Keys.ctrl(KeyEvent.VK_TAB)); + Keys.killFocusTraversalBinding(window, Keys.ctrlShift(KeyEvent.VK_TAB)); + } + } + public EditorHeader(Editor eddie) { this.editor = eddie; // weird name for listener diff --git a/app/src/processing/app/helpers/Keys.java b/app/src/processing/app/helpers/Keys.java index e4c797ed2e8..6c0053e50ef 100644 --- a/app/src/processing/app/helpers/Keys.java +++ b/app/src/processing/app/helpers/Keys.java @@ -29,11 +29,17 @@ package processing.app.helpers; +import java.awt.AWTKeyStroke; +import java.awt.Component; +import java.awt.KeyboardFocusManager; import java.awt.Toolkit; import java.awt.event.InputEvent; import java.beans.PropertyChangeEvent; +import java.util.HashSet; +import java.util.Set; import javax.swing.Action; +import javax.swing.InputMap; import javax.swing.JComponent; import javax.swing.KeyStroke; @@ -115,6 +121,65 @@ public static void bind(final JComponent component, final Action action, }); } + /** + * Kill an existing binding from the given condition. If the binding is + * defined on the given component, it is removed, but if it is defined through + * a parent inputmap (typically shared by multiple components, so best not + * touched), this adds a dummy binding for this component, that will never + * match an action in the component's action map, effectively disabling the + * binding. + * + * This method is not intended to unbind a binding created by bind(), since + * such a binding would get re-enabled when the action is re-enabled. + */ + public static void killBinding(final JComponent component, + final KeyStroke keystroke, int condition) { + InputMap map = component.getInputMap(condition); + // First, try removing it + map.remove(keystroke); + // If the binding is defined in a parent map, defining it will not work, so + // instead add an override that will never appear in the action map. + if (map.get(keystroke) != null) + map.put(keystroke, new Object()); + } + + /** + * Kill an existing binding like above, but from all three conditions + * (WHEN_FOCUSED, WHEN_ANCESTOR_OF_FOCUSED_COMPONENT, WHEN_IN_FOCUSED_WINDOW). + */ + public static void killBinding(final JComponent component, + final KeyStroke key) { + killBinding(component, key, JComponent.WHEN_FOCUSED); + killBinding(component, key, JComponent.WHEN_ANCESTOR_OF_FOCUSED_COMPONENT); + killBinding(component, key, JComponent.WHEN_IN_FOCUSED_WINDOW); + } + + /** + * Remove a keystroke from the keys used to shift focus in or below the given + * component. This modifies all sets of focus traversal keys on the given + * component to remove the given keystroke. These sets are inherited down the + * component hierarchy (until a component that has a custom set itself). + */ + public static void killFocusTraversalBinding(final Component component, + final KeyStroke keystroke) { + int[] sets = { KeyboardFocusManager.FORWARD_TRAVERSAL_KEYS, + KeyboardFocusManager.BACKWARD_TRAVERSAL_KEYS, + KeyboardFocusManager.UP_CYCLE_TRAVERSAL_KEYS, + KeyboardFocusManager.DOWN_CYCLE_TRAVERSAL_KEYS }; + for (int set : sets) { + Set keys = component.getFocusTraversalKeys(set); + // keys is immutable, so create a new set to allow changes + keys = new HashSet<>(keys); + if (set == 0) + keys.add(ctrlAlt('Z')); + + // If the given keystroke was present in the set, replace it with the + // updated set with the keystroke removed. + if (keys.remove(keystroke)) + component.setFocusTraversalKeys(set, keys); + } + } + private static void enableBind(final JComponent component, final Action action, final KeyStroke keystroke, int condition) { diff --git a/app/src/processing/app/syntax/SketchTextArea.java b/app/src/processing/app/syntax/SketchTextArea.java index 50d946a06a5..58e0bb4e63b 100644 --- a/app/src/processing/app/syntax/SketchTextArea.java +++ b/app/src/processing/app/syntax/SketchTextArea.java @@ -41,7 +41,6 @@ import processing.app.EditorListener; import processing.app.PreferencesData; -import javax.swing.*; import javax.swing.event.EventListenerList; import javax.swing.event.HyperlinkEvent; import javax.swing.event.HyperlinkListener; @@ -57,9 +56,7 @@ import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; -import java.util.HashSet; import java.util.Map; -import java.util.Set; import java.util.logging.Logger; /** @@ -91,8 +88,6 @@ private void installFeatures() throws IOException { setLinkGenerator(new DocLinkGenerator(pdeKeywords)); - fixControlTab(); - setSyntaxEditingStyle(SYNTAX_STYLE_CPLUSPLUS); } @@ -153,28 +148,6 @@ private void setSyntaxTheme(int tokenType, String id) { getSyntaxScheme().setStyle(tokenType, style); } - // Removing the default focus traversal keys - // This is because the DefaultKeyboardFocusManager handles the keypress and consumes the event - private void fixControlTab() { - removeCTRLTabFromFocusTraversal(); - - removeCTRLSHIFTTabFromFocusTraversal(); - } - - private void removeCTRLSHIFTTabFromFocusTraversal() { - KeyStroke ctrlShiftTab = KeyStroke.getKeyStroke("ctrl shift TAB"); - Set backwardKeys = new HashSet<>(this.getFocusTraversalKeys(KeyboardFocusManager.BACKWARD_TRAVERSAL_KEYS)); - backwardKeys.remove(ctrlShiftTab); - } - - private void removeCTRLTabFromFocusTraversal() { - KeyStroke ctrlTab = KeyStroke.getKeyStroke("ctrl TAB"); - Set forwardKeys = new HashSet<>(this.getFocusTraversalKeys(KeyboardFocusManager.FORWARD_TRAVERSAL_KEYS)); - forwardKeys.remove(ctrlTab); - this.setFocusTraversalKeys(KeyboardFocusManager.FORWARD_TRAVERSAL_KEYS, forwardKeys); - } - - public boolean isSelectionActive() { return this.getSelectedText() != null; } From ac66a9c64ab2051f0058d237f5e68396862c826b Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 11 Dec 2015 15:23:04 +0100 Subject: [PATCH 09/10] Change workaround for ctrl-slash handling in RSyntaxTextArea Previously, there was a handler on the text area that consumed most KEY_TYPED events with control pressed. This was added a long time ago to fix a problem with ctrl-slash doing both the toggle comment action and inserting a /. Further investigation shows that with RSyntaxTextArea this problem is still present, but is caused by a weird binding on the slash key that Arduino is not even using. Removing that binding is a cleaner workaround for this problem, so this commit switches to that workaround. Ideally this would be fixed in RSyntaxTextArea, see https://github.com/bobbylight/RSyntaxTextArea/issues/157 --- app/src/processing/app/EditorListener.java | 8 -------- .../app/syntax/SketchTextAreaDefaultInputMap.java | 9 +++++++++ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/app/src/processing/app/EditorListener.java b/app/src/processing/app/EditorListener.java index e5203b4681b..f9fa0d5acbe 100644 --- a/app/src/processing/app/EditorListener.java +++ b/app/src/processing/app/EditorListener.java @@ -22,14 +22,6 @@ public EditorListener(Editor editor) { private static final int CTRL_SHIFT = InputEvent.SHIFT_MASK | CTRL; public void keyTyped(KeyEvent event) { - char c = event.getKeyChar(); - - if ((event.getModifiers() & KeyEvent.CTRL_MASK) != 0) { - // The char is not control code when CTRL key pressed? It should be a shortcut. - if (!Character.isISOControl(c)) { - event.consume(); - } - } } @Override diff --git a/app/src/processing/app/syntax/SketchTextAreaDefaultInputMap.java b/app/src/processing/app/syntax/SketchTextAreaDefaultInputMap.java index f713e26d5ed..dfa5df928c4 100644 --- a/app/src/processing/app/syntax/SketchTextAreaDefaultInputMap.java +++ b/app/src/processing/app/syntax/SketchTextAreaDefaultInputMap.java @@ -23,6 +23,15 @@ public SketchTextAreaDefaultInputMap() { remove(KeyStroke.getKeyStroke(KeyEvent.VK_K, defaultModifier)); + // Remove a troublesome binding for the / key. By default, RSyntaxTextArea + // binds the / KEY_TYPED event to insert a / and optionally complete any XML + // tags. However, since this also triggeres on ctrl-slash, this means that + // in addition to toggling comments on ctrl-slash, it also inserts a slash. + // Since we don't need the XML completion feature anyway, just unbind it + // here. A future version of RSyntaxTextArea might fix this, see + // https://github.com/bobbylight/RSyntaxTextArea/issues/157. + remove(KeyStroke.getKeyStroke('/')); + if (PreferencesData.getBoolean("editor.advanced")) { put(KeyStroke.getKeyStroke(KeyEvent.VK_DOWN, alt), RTextAreaEditorKit.rtaLineDownAction); put(KeyStroke.getKeyStroke(KeyEvent.VK_UP, alt), RTextAreaEditorKit.rtaLineUpAction); From 7eea624dfa0d4705dcb26e40356d62770ee7dd99 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 11 Dec 2015 15:28:57 +0100 Subject: [PATCH 10/10] Remove EditorListener class It did not contain any actual code anymore, so it can be removed, along with the infrastructure for setting it up. --- app/src/processing/app/Editor.java | 1 - app/src/processing/app/EditorListener.java | 37 ------------------- .../processing/app/syntax/SketchTextArea.java | 30 --------------- 3 files changed, 68 deletions(-) delete mode 100644 app/src/processing/app/EditorListener.java diff --git a/app/src/processing/app/Editor.java b/app/src/processing/app/Editor.java index c922a9c557d..6960e229edd 100644 --- a/app/src/processing/app/Editor.java +++ b/app/src/processing/app/Editor.java @@ -1040,7 +1040,6 @@ private SketchTextArea createTextArea() throws IOException { textArea.setAntiAliasingEnabled(PreferencesData.getBoolean("editor.antialias")); textArea.setTabsEmulated(PreferencesData.getBoolean("editor.tabs.expand")); textArea.setTabSize(PreferencesData.getInteger("editor.tabs.size")); - textArea.setEditorListener(new EditorListener(this)); textArea.addHyperlinkListener(new HyperlinkListener() { @Override public void hyperlinkUpdate(HyperlinkEvent hyperlinkEvent) { diff --git a/app/src/processing/app/EditorListener.java b/app/src/processing/app/EditorListener.java deleted file mode 100644 index f9fa0d5acbe..00000000000 --- a/app/src/processing/app/EditorListener.java +++ /dev/null @@ -1,37 +0,0 @@ -package processing.app; - -import java.awt.Toolkit; -import java.awt.event.InputEvent; -import java.awt.event.KeyEvent; -import java.awt.event.KeyListener; - -import processing.app.syntax.SketchTextArea; - -public class EditorListener implements KeyListener { - - private Editor editor; - - public EditorListener(Editor editor) { - super(); - this.editor = editor; - } - - /** ctrl-alt on windows and linux, cmd-alt on mac os x */ - private static final int CTRL = Toolkit.getDefaultToolkit().getMenuShortcutKeyMask(); - private static final int CTRL_ALT = InputEvent.ALT_MASK | CTRL; - private static final int CTRL_SHIFT = InputEvent.SHIFT_MASK | CTRL; - - public void keyTyped(KeyEvent event) { - } - - @Override - public void keyPressed(KeyEvent event) { - } - - @Override - public void keyReleased(KeyEvent e) { - // TODO Auto-generated method stub - - } - -} diff --git a/app/src/processing/app/syntax/SketchTextArea.java b/app/src/processing/app/syntax/SketchTextArea.java index 58e0bb4e63b..ac50a2dc2a6 100644 --- a/app/src/processing/app/syntax/SketchTextArea.java +++ b/app/src/processing/app/syntax/SketchTextArea.java @@ -38,7 +38,6 @@ import org.fife.ui.rtextarea.RUndoManager; import processing.app.Base; import processing.app.BaseNoGui; -import processing.app.EditorListener; import processing.app.PreferencesData; import javax.swing.event.EventListenerList; @@ -49,7 +48,6 @@ import javax.swing.text.Segment; import javax.swing.undo.UndoManager; import java.awt.*; -import java.awt.event.KeyEvent; import java.awt.event.MouseEvent; import java.io.File; import java.io.FileInputStream; @@ -69,8 +67,6 @@ public class SketchTextArea extends RSyntaxTextArea { private final static Logger LOG = Logger.getLogger(SketchTextArea.class.getName()); - private EditorListener editorListener; - private PdeKeywords pdeKeywords; public SketchTextArea(PdeKeywords pdeKeywords) throws IOException { @@ -152,27 +148,6 @@ public boolean isSelectionActive() { return this.getSelectedText() != null; } - public void processKeyEvent(KeyEvent evt) { - - // this had to be added because the menu key events weren't making it up to the frame. - - switch (evt.getID()) { - case KeyEvent.KEY_TYPED: - if (editorListener != null) editorListener.keyTyped(evt); - break; - case KeyEvent.KEY_PRESSED: - if (editorListener != null) editorListener.keyPressed(evt); - break; - case KeyEvent.KEY_RELEASED: - // inputHandler.keyReleased(evt); - break; - } - - if (!evt.isConsumed()) { - super.processKeyEvent(evt); - } - } - public void switchDocument(Document document, UndoManager newUndo) { // HACK: Dont discard changes on curret UndoManager. @@ -206,11 +181,6 @@ public void getTextLine(int line, Segment segment) { } } - - public void setEditorListener(EditorListener editorListener) { - this.editorListener = editorListener; - } - private static class DocLinkGenerator implements LinkGenerator { private final PdeKeywords pdeKeywords;