This is an automated email from the ASF dual-hosted git repository.

bamaer pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git


The following commit(s) were added to refs/heads/main by this push:
     new 81bf2d6b1e Issue #4940 : Add Ctrl+Z and Ctrl+Y undo and redo in script 
editors (#8695)
81bf2d6b1e is described below

commit 81bf2d6b1ed1e79972e43f9b62c7f791407ca29c
Author: Matt Casters <[email protected]>
AuthorDate: Thu Oct 1 09:15:54 2026 +0200

    Issue #4940 : Add Ctrl+Z and Ctrl+Y undo and redo in script editors (#8695)
---
 .../widget/StyledTextVarHistoryShortcutTest.java   | 101 +++++++++++++++++++++
 .../apache/hop/ui/core/widget/StyledTextVar.java   |  28 +++++-
 .../apache/hop/ui/core/widget/TextComposite.java   |  29 ++++--
 .../org/apache/hop/ui/hopgui/HopGuiKeyHandler.java |  47 +++++++++-
 .../core/widget/messages/messages_en_US.properties |   2 +-
 .../core/widget/messages/messages_pt_BR.properties |   2 +-
 .../core/widget/messages/messages_zh_CN.properties |   2 +-
 .../apache/hop/ui/hopgui/HopGuiKeyHandlerTest.java |  57 ++++++++++++
 8 files changed, 252 insertions(+), 16 deletions(-)

diff --git 
a/rcp/src/test/java/org/apache/hop/ui/core/widget/StyledTextVarHistoryShortcutTest.java
 
b/rcp/src/test/java/org/apache/hop/ui/core/widget/StyledTextVarHistoryShortcutTest.java
new file mode 100644
index 0000000000..56e9769ff4
--- /dev/null
+++ 
b/rcp/src/test/java/org/apache/hop/ui/core/widget/StyledTextVarHistoryShortcutTest.java
@@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.hop.ui.core.widget;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.apache.hop.core.variables.Variables;
+import org.apache.hop.ui.testing.SwtBotTestBase;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.custom.StyledText;
+import org.eclipse.swt.layout.FillLayout;
+import org.eclipse.swt.widgets.Event;
+import org.eclipse.swt.widgets.Shell;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+/** Ctrl+Z / Ctrl+Y in the script and Java editors (StyledTextVar), issue 
#4940. */
+@Tag("uitest")
+class StyledTextVarHistoryShortcutTest extends SwtBotTestBase {
+
+  @Test
+  void ctrlZAndCtrlYWalkTheEditHistory() {
+    Shell shell = new Shell(display);
+    shell.setLayout(new FillLayout());
+    try {
+      StyledTextVar text =
+          new StyledTextVar(
+              new Variables(), shell, SWT.MULTI | SWT.V_SCROLL | SWT.H_SCROLL, 
false, false, false);
+      shell.setSize(400, 300);
+      shell.open();
+
+      text.setText("base");
+      assertFalse(text.canUndo(), "Loading the script must not be an undoable 
edit");
+
+      type(text, "X");
+      type(text, "Y");
+      assertEquals("baseXY", text.getText());
+      assertTrue(text.canUndo());
+      assertFalse(text.canRedo());
+
+      press(text.getTextWidget(), 'z', SWT.MOD1);
+      assertEquals("baseX", text.getText());
+      press(text.getTextWidget(), 'z', SWT.MOD1);
+      assertEquals("base", text.getText());
+      // Applying undo used to record itself, so the next Ctrl+Z put the edit 
back.
+      press(text.getTextWidget(), 'z', SWT.MOD1);
+      assertEquals("base", text.getText());
+      assertFalse(text.canUndo());
+      assertTrue(text.canRedo());
+
+      press(text.getTextWidget(), 'y', SWT.MOD1);
+      assertEquals("baseX", text.getText());
+      press(text.getTextWidget(), 'Z', SWT.MOD1 | SWT.MOD2);
+      assertEquals("baseXY", text.getText(), "Ctrl+Shift+Z still redoes");
+      assertFalse(text.canRedo());
+
+      press(text.getTextWidget(), 'z', SWT.MOD1);
+      press(text.getTextWidget(), 'z', SWT.MOD1);
+      assertEquals("base", text.getText());
+      type(text, "Q");
+      assertEquals("baseQ", text.getText());
+      press(text.getTextWidget(), 'y', SWT.MOD1);
+      assertEquals("baseQ", text.getText(), "A new edit clears redo");
+      assertFalse(text.canRedo());
+    } finally {
+      shell.dispose();
+    }
+  }
+
+  private static void type(StyledTextVar text, String value) {
+    text.setCaretPosition(text.getCharCount());
+    text.insert(value);
+  }
+
+  private static void press(StyledText widget, char key, int stateMask) {
+    Event event = new Event();
+    event.type = SWT.KeyDown;
+    event.keyCode = key;
+    event.stateMask = stateMask;
+    event.doit = true;
+    widget.notifyListeners(SWT.KeyDown, event);
+    assertFalse(event.doit, "The editor shortcut must be consumed");
+  }
+}
diff --git a/ui/src/main/java/org/apache/hop/ui/core/widget/StyledTextVar.java 
b/ui/src/main/java/org/apache/hop/ui/core/widget/StyledTextVar.java
index f871f39f1b..2b2a67aaf1 100644
--- a/ui/src/main/java/org/apache/hop/ui/core/widget/StyledTextVar.java
+++ b/ui/src/main/java/org/apache/hop/ui/core/widget/StyledTextVar.java
@@ -26,6 +26,7 @@ import org.apache.hop.ui.core.FormDataBuilder;
 import org.apache.hop.ui.core.PropsUi;
 import org.apache.hop.ui.core.gui.GuiResource;
 import org.apache.hop.ui.core.widget.highlight.JavaHighlight;
+import org.apache.hop.ui.hopgui.HopGuiKeyHandler;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.custom.LineStyleListener;
 import org.eclipse.swt.custom.StyleRange;
@@ -58,6 +59,9 @@ public class StyledTextVar extends TextComposite {
 
   private boolean fullSelection = false;
 
+  /** True while undo or redo writes the text back, so that write is not 
stored as a new edit. */
+  private boolean applyingHistory;
+
   public StyledTextVar(IVariables variables, Composite parent, int style) {
     this(variables, parent, style, true, false, true, STYLE_TYPE_GENERIC);
   }
@@ -128,6 +132,8 @@ public class StyledTextVar extends TextComposite {
     redoStack = new LinkedList<>();
 
     wText = new StyledText(this, style);
+    // This control handles Ctrl/Cmd+Z and Ctrl/Cmd+Y. The graph must not take 
those chords.
+    wText.setData(HopGuiKeyHandler.HOP_TEXT_EDITOR_HISTORY, Boolean.TRUE);
     wPopupMenu = new Menu(parent.getShell(), SWT.POP_UP);
 
     buildingStyledTextMenu(wPopupMenu);
@@ -397,6 +403,10 @@ public class StyledTextVar extends TextComposite {
 
     wText.addExtendedModifyListener(
         event -> {
+          if (applyingHistory) {
+            fullSelection = false;
+            return;
+          }
           int eventLength = event.length;
           int eventStartPostition = event.start;
 
@@ -430,6 +440,8 @@ public class StyledTextVar extends TextComposite {
               if (undoStack.size() == MAX_STACK_SIZE) {
                 undoStack.remove(undoStack.size() - 1);
               }
+              // A new edit invalidates anything that could be redone.
+              redoStack.clear();
               undoStack.add(0, urs);
             }
           }
@@ -439,7 +451,11 @@ public class StyledTextVar extends TextComposite {
 
   @Override
   protected void undo() {
-    if (!undoStack.isEmpty()) {
+    if (undoStack.isEmpty()) {
+      return;
+    }
+    applyingHistory = true;
+    try {
       UndoRedoStack undo = undoStack.remove(0);
       if (redoStack.size() == MAX_STACK_SIZE) {
         redoStack.remove(redoStack.size() - 1);
@@ -463,12 +479,18 @@ public class StyledTextVar extends TextComposite {
         }
       }
       redoStack.add(0, redo);
+    } finally {
+      applyingHistory = false;
     }
   }
 
   @Override
   protected void redo() {
-    if (!redoStack.isEmpty()) {
+    if (redoStack.isEmpty()) {
+      return;
+    }
+    applyingHistory = true;
+    try {
       UndoRedoStack redo = redoStack.remove(0);
       if (undoStack.size() == MAX_STACK_SIZE) {
         undoStack.remove(undoStack.size() - 1);
@@ -492,6 +514,8 @@ public class StyledTextVar extends TextComposite {
         }
       }
       undoStack.add(0, undo);
+    } finally {
+      applyingHistory = false;
     }
   }
 }
diff --git a/ui/src/main/java/org/apache/hop/ui/core/widget/TextComposite.java 
b/ui/src/main/java/org/apache/hop/ui/core/widget/TextComposite.java
index 5e95f883b4..243d856198 100644
--- a/ui/src/main/java/org/apache/hop/ui/core/widget/TextComposite.java
+++ b/ui/src/main/java/org/apache/hop/ui/core/widget/TextComposite.java
@@ -718,24 +718,33 @@ public abstract class TextComposite extends Composite 
implements IFindReplaceTar
     addListener(
         SWT.KeyDown,
         event -> {
-          if (isSupportUnoRedo()
-              && event.keyCode == 'z'
-              && (event.stateMask & SWT.MOD1) != 0
-              && (event.stateMask & SWT.MOD2) != 0) {
+          if ((event.stateMask & SWT.MOD1) == 0) {
+            return;
+          }
+          // Letters stay lowercase with Shift held on some platforms and not 
on others.
+          char key = Character.toLowerCase((char) (event.keyCode & 
SWT.KEY_MASK));
+          boolean shift = (event.stateMask & SWT.MOD2) != 0;
+          // Consume undo/redo. Otherwise the same chord also undoes the 
pipeline or workflow
+          // and moves focus back to the graph.
+          if (isSupportUnoRedo() && key == 'y' && !shift) {
+            redo();
+            updateToolbar();
+            event.doit = false;
+          } else if (isSupportUnoRedo() && key == 'z' && shift) {
             redo();
             updateToolbar();
-          } else if (isSupportUnoRedo()
-              && event.keyCode == 'z'
-              && (event.stateMask & SWT.MOD1) != 0) {
+            event.doit = false;
+          } else if (isSupportUnoRedo() && key == 'z') {
             undo();
             updateToolbar();
-          } else if (event.keyCode == 'a' && (event.stateMask & SWT.MOD1) != 
0) {
+            event.doit = false;
+          } else if (key == 'a') {
             selectAll();
             updateToolbar();
-          } else if (event.keyCode == 'f' && (event.stateMask & SWT.MOD1) != 
0) {
+          } else if (key == 'f') {
             find();
             event.doit = false;
-          } else if (event.keyCode == 'h' && (event.stateMask & SWT.MOD1) != 
0) {
+          } else if (key == 'h') {
             findAndReplace();
             event.doit = false;
           }
diff --git a/ui/src/main/java/org/apache/hop/ui/hopgui/HopGuiKeyHandler.java 
b/ui/src/main/java/org/apache/hop/ui/hopgui/HopGuiKeyHandler.java
index 40d375cf80..bba0e97f6c 100644
--- a/ui/src/main/java/org/apache/hop/ui/hopgui/HopGuiKeyHandler.java
+++ b/ui/src/main/java/org/apache/hop/ui/hopgui/HopGuiKeyHandler.java
@@ -60,6 +60,12 @@ public class HopGuiKeyHandler extends KeyAdapter {
   /** Data key marking the terminal widget, which handles all keys itself. */
   public static final String HOP_TERMINAL_WIDGET = "HOP_TERMINAL_WIDGET";
 
+  /**
+   * Data key on a text control that handles Ctrl/Cmd+Z, Ctrl/Cmd+Shift+Z and 
Ctrl/Cmd+Y itself.
+   * Those chords also undo and redo the pipeline or workflow.
+   */
+  public static final String HOP_TEXT_EDITOR_HISTORY = 
"HOP_TEXT_EDITOR_HISTORY";
+
   /** Widget classes that pass their key listeners on to a widget inside them. 
*/
   private static final Map<Class<?>, Boolean> DELEGATING_KEY_LISTENERS = new 
ConcurrentHashMap<>();
 
@@ -544,6 +550,10 @@ public class HopGuiKeyHandler extends KeyAdapter {
       // cancel the key.
       return TextEditing.STOP;
     }
+    if (textLike && isTextEditorHistoryKey(widget, keyCode, stateMask)) {
+      // The editor's own key listener performs undo/redo. Do not also undo 
the graph.
+      return TextEditing.STOP;
+    }
     if (!textLike || !isNativeTextEditingKey(keyCode, stateMask, character)) {
       return TextEditing.PASS;
     }
@@ -606,6 +616,40 @@ public class HopGuiKeyHandler extends KeyAdapter {
     return alt || control || command;
   }
 
+  /**
+   * Ctrl/Cmd+Z, Ctrl/Cmd+Shift+Z and Ctrl/Cmd+Y on a text control marked with 
{@link
+   * #HOP_TEXT_EDITOR_HISTORY}.
+   */
+  private static boolean isTextEditorHistoryKey(Widget widget, int keyCode, 
int stateMask) {
+    if (!(widget instanceof Control control) || 
!editorOwnsHistoryKeys(control)) {
+      return false;
+    }
+    if ((stateMask & SWT.MOD1) == 0) {
+      return false;
+    }
+    char key = Character.toLowerCase((char) (keyCode & SWT.KEY_MASK));
+    boolean shift = (stateMask & SWT.SHIFT) != 0;
+    if (key == 'y') {
+      return !shift;
+    }
+    return key == 'z';
+  }
+
+  private static boolean editorOwnsHistoryKeys(Control control) {
+    Control current = control;
+    while (current != null) {
+      try {
+        if (current.getData(HOP_TEXT_EDITOR_HISTORY) == Boolean.TRUE) {
+          return true;
+        }
+        current = current.getParent();
+      } catch (SWTException e) {
+        return false;
+      }
+    }
+    return false;
+  }
+
   /** Ctrl/Cmd+A with no Alt and no Shift. */
   private static boolean isSelectAllKey(int keyCode, int stateMask) {
     if ((stateMask & (SWT.ALT | SWT.SHIFT)) != 0) {
@@ -640,7 +684,8 @@ public class HopGuiKeyHandler extends KeyAdapter {
    *
    * <p>Graph shortcuts such as Space (output fields), {@code z} (open 
referenced object) and {@code
    * x} (open execution) must not steal those keys from filter and search 
fields. App shortcuts with
-   * CTRL/CMD/ALT (e.g. Ctrl+S) still run, except the horizontal word-movement 
keys handled above.
+   * CTRL/CMD/ALT (e.g. Ctrl+S) still run, except the horizontal word-movement 
keys handled above
+   * and the undo/redo chords of an editor that keeps its own history.
    */
   private static boolean isNativeTextEditingKey(int keyCode, int stateMask, 
char character) {
     if ((stateMask & (SWT.CONTROL | SWT.COMMAND)) != 0) {
diff --git 
a/ui/src/main/resources/org/apache/hop/ui/core/widget/messages/messages_en_US.properties
 
b/ui/src/main/resources/org/apache/hop/ui/core/widget/messages/messages_en_US.properties
index 4d85e4faf3..181973ac92 100644
--- 
a/ui/src/main/resources/org/apache/hop/ui/core/widget/messages/messages_en_US.properties
+++ 
b/ui/src/main/resources/org/apache/hop/ui/core/widget/messages/messages_en_US.properties
@@ -129,7 +129,7 @@ WidgetDialog.Styled.Cut=Cut\tCtrl+X
 WidgetDialog.Styled.Find=Find\tCtrl+F
 WidgetDialog.Styled.FindReplace=Find/Replace\tCtrl+H
 WidgetDialog.Styled.Paste=Paste\tCtrl+V
-WidgetDialog.Styled.Redo=Redo\tCtrl+Shift+Z
+WidgetDialog.Styled.Redo=Redo\tCtrl+Y
 WidgetDialog.Styled.SelectAll=Select &All\tCtrl+A
 WidgetDialog.Styled.Undo=Undo\tCtrl+Z
 FileTreeWidget.IncludeDependencies.Label=Include workflows/pipelines that 
depend on the selection
diff --git 
a/ui/src/main/resources/org/apache/hop/ui/core/widget/messages/messages_pt_BR.properties
 
b/ui/src/main/resources/org/apache/hop/ui/core/widget/messages/messages_pt_BR.properties
index bfcddb593f..d6720ea676 100644
--- 
a/ui/src/main/resources/org/apache/hop/ui/core/widget/messages/messages_pt_BR.properties
+++ 
b/ui/src/main/resources/org/apache/hop/ui/core/widget/messages/messages_pt_BR.properties
@@ -96,7 +96,7 @@ TableView.ToolBarWidget.SetFilter.ToolTip=Selecione linhas 
usando um filtro
 TableView.ToolBarWidget.UndoAction.ToolTip=Desfaça a última ação
 TextVar.InternalVariable.Message=Esta é uma variável interna.
 TextVar.VariableValue.Message=O valor da variável é \: \n\n\n\t\n\n\n\n
-WidgetDialog.Styled.Redo=Redo\tCtrl+Shift+Z
+WidgetDialog.Styled.Redo=Redo\tCtrl+Y
 WidgetDialog.Styled.SelectAll=Selecione &All\tCtrl+A
 WidgetDialog.Styled.Undo=Desfazer\tCtrl+Z
 FileTreeWidget.IncludeDependencies.Label=Inclua fluxos de trabalho/pipelines 
que dependem da seleção
diff --git 
a/ui/src/main/resources/org/apache/hop/ui/core/widget/messages/messages_zh_CN.properties
 
b/ui/src/main/resources/org/apache/hop/ui/core/widget/messages/messages_zh_CN.properties
index 5e7ec68880..8b84127122 100644
--- 
a/ui/src/main/resources/org/apache/hop/ui/core/widget/messages/messages_zh_CN.properties
+++ 
b/ui/src/main/resources/org/apache/hop/ui/core/widget/messages/messages_zh_CN.properties
@@ -78,5 +78,5 @@ WidgetDialog.Styled.Paste=\u7C98\u8D34\tCtrl+V
 WidgetDialog.Styled.SelectAll=\u5168\u9009\tCtrl+A
 WidgetDialog.Styled.Find=\u641C\u7D22\tCtrl+F
 WidgetDialog.Styled.FindReplace=\u641C\u7D22/\u66FF\u6362\tCtrl+H
-WidgetDialog.Styled.Redo=\u91CD\u505A\tCtrl+Shift+Z
+WidgetDialog.Styled.Redo=\u91CD\u505A\tCtrl+Y
 WidgetDialog.Styled.Undo=\u64A4\u9500\tCtrl+Z
diff --git 
a/ui/src/test/java/org/apache/hop/ui/hopgui/HopGuiKeyHandlerTest.java 
b/ui/src/test/java/org/apache/hop/ui/hopgui/HopGuiKeyHandlerTest.java
index ee5d903b32..2d253d009e 100644
--- a/ui/src/test/java/org/apache/hop/ui/hopgui/HopGuiKeyHandlerTest.java
+++ b/ui/src/test/java/org/apache/hop/ui/hopgui/HopGuiKeyHandlerTest.java
@@ -339,6 +339,63 @@ class HopGuiKeyHandlerTest {
     }
   }
 
+  /** Stands in for the Edit / Undo shortcut on the graph and the main menu. */
+  public static class HistoryGraph {
+    public int undos;
+    public int redos;
+
+    @GuiKeyboardShortcut(control = true, key = 'z')
+    @GuiOsxKeyboardShortcut(command = true, key = 'z')
+    public void undo() {
+      undos++;
+    }
+
+    @GuiKeyboardShortcut(control = true, shift = true, key = 'z')
+    @GuiOsxKeyboardShortcut(command = true, shift = true, key = 'z')
+    public void redo() {
+      redos++;
+    }
+  }
+
+  @Test
+  void undoRedoStayInEditorsThatKeepTheirOwnHistory() {
+    HistoryGraph graph = new HistoryGraph();
+    registerShortcutsLikeHopGuiEnvironment(HistoryGraph.class);
+
+    HopGuiKeyHandler keyHandler = HopGuiKeyHandler.getInstance();
+    keyHandler.addParentObjectToHandle(graph);
+    try {
+      StyledText editor = mock(StyledText.class);
+      
when(editor.getData(HopGuiKeyHandler.HOP_TEXT_EDITOR_HISTORY)).thenReturn(Boolean.TRUE);
+
+      KeyEvent undo = keyEvent(editor, 'z', SWT.CONTROL);
+      keyHandler.keyPressed(undo);
+      assertEquals(0, graph.undos, "Ctrl+Z in a script editor must not undo 
the graph");
+      assertTrue(undo.doit, "The editor performs undo; this handler must not 
consume the key");
+
+      KeyEvent upper = keyEvent(editor, 'Z', SWT.CONTROL);
+      keyHandler.keyPressed(upper);
+      assertEquals(0, graph.undos, "Ctrl+Z must match regardless of key-code 
case");
+
+      KeyEvent redo = keyEvent(editor, 'y', SWT.CONTROL);
+      keyHandler.keyPressed(redo);
+      assertEquals(0, graph.redos);
+      assertTrue(redo.doit, "Ctrl+Y stays with the editor");
+
+      KeyEvent shiftRedo = keyEvent(editor, 'z', SWT.CONTROL | SWT.SHIFT);
+      keyHandler.keyPressed(shiftRedo);
+      assertEquals(0, graph.redos, "Ctrl+Shift+Z in a script editor must not 
redo the graph");
+      assertTrue(shiftRedo.doit);
+
+      KeyEvent outside = keyEvent(mock(StyledText.class), 'z', SWT.CONTROL);
+      keyHandler.keyPressed(outside);
+      assertEquals(1, graph.undos, "Ctrl+Z outside that editor still undoes 
the graph");
+      assertFalse(outside.doit);
+    } finally {
+      keyHandler.removeParentObjectToHandle(graph);
+    }
+  }
+
   @Test
   void arrowKeysAreLeftToTablesAndTrees() {
     NavigationGraph graph = new NavigationGraph();

Reply via email to