Skip to content

Update RSyntaxTextArea to 2.6.1 and remove need for Arduino-specific patches #5990

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/.classpath
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
<classpathentry kind="lib" path="lib/jmdns-3.4.1.jar"/>
<classpathentry kind="lib" path="lib/jsch-0.1.50.jar"/>
<classpathentry kind="lib" path="lib/jssc-2.8.0.jar"/>
<classpathentry kind="lib" path="lib/rsyntaxtextarea-2.5.8.1+arduino.jar"/>
<classpathentry kind="lib" path="lib/rsyntaxtextarea-2.6.1.jar"/>
<classpathentry kind="lib" path="lib/xml-apis-1.3.04.jar"/>
<classpathentry kind="lib" path="lib/xml-apis-ext-1.3.04.jar"/>
<classpathentry kind="lib" path="lib/xmlgraphics-commons-2.0.jar"/>
Expand Down
Binary file removed app/lib/rsyntaxtextarea-2.5.8.1+arduino.jar
Binary file not shown.
Binary file added app/lib/rsyntaxtextarea-2.6.1.jar
Binary file not shown.
77 changes: 0 additions & 77 deletions app/src/processing/app/CaretAwareUndoableEdit.java

This file was deleted.

176 changes: 45 additions & 131 deletions app/src/processing/app/Editor.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import jssc.SerialPortException;
import processing.app.debug.RunnerException;
import processing.app.forms.PasswordAuthorizationDialog;
import processing.app.helpers.ActionWrapper;
import processing.app.helpers.Keys;
import processing.app.helpers.OSUtils;
import processing.app.helpers.PreferencesMapException;
Expand All @@ -44,9 +45,9 @@
import javax.swing.*;
import javax.swing.event.*;
import javax.swing.text.BadLocationException;
import javax.swing.undo.CannotRedoException;
import javax.swing.undo.CannotUndoException;
import javax.swing.undo.UndoManager;
import org.fife.ui.rtextarea.RTextArea;
import org.fife.ui.rtextarea.RecordableTextAction;

import java.awt.*;
import java.awt.datatransfer.DataFlavor;
import java.awt.datatransfer.Transferable;
Expand Down Expand Up @@ -182,12 +183,6 @@ public boolean test(SketchController sketch) {
//boolean presenting;
private boolean uploading;

// undo fellers
private JMenuItem undoItem;
private JMenuItem redoItem;
protected UndoAction undoAction;
protected RedoAction redoAction;

private FindReplace find;

Runnable runHandler;
Expand Down Expand Up @@ -1265,43 +1260,58 @@ public void actionPerformed(ActionEvent e) {
return menu;
}

/**
* Wrapper around RSyntaxTextArea's RecordableTextActions. Normally,
* these actions use the event source, or the most recently focused
* component to find the text area to work on, but this does not
* always work. This wrapper makes them always work on the currently
* selected tab instead.
*/
class RstaActionWrapper extends ActionWrapper {
RstaActionWrapper(RecordableTextAction action) {
super(action);
}

@Override
public void actionPerformed(ActionEvent e) {
((RecordableTextAction) getWrappedAction()).actionPerformedImpl(e, getCurrentTab().getTextArea());
}
}

private JMenu buildEditMenu() {
JMenu menu = new JMenu(tr("Edit"));
menu.setName("menuEdit");
menu.setMnemonic(KeyEvent.VK_E);

undoItem = newJMenuItem(tr("Undo"), 'Z');
// Create a dummy RTextArea, to force it to create the static list
// of actions accessible through RTextArea.getAction().
new RTextArea();

RecordableTextAction undoAction = RTextArea.getAction(RTextArea.UNDO_ACTION);
JMenuItem undoItem = new JMenuItem(new RstaActionWrapper(undoAction));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dummy object creation to obtain the UNDO_ACTION makes me really uneasy.
Also, we already have a EditorTab.handleUndo() method, why not just call that instead of trying to fit the RSTA action?

Moreover the RSTA action is wrapped inside another ActionWrapper class that is made just to route the message to the correct EditorTab through getCurrentTab(), while it may be much more simple to do something like:

undoItem.addActionListener(() -> getCurrentTab().handleUndo());

Or maybe I'm missing something?

I remember that I've already solved this in another refactoring experiment I did some time ago I'll try to recover this attempt from my old branches.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason to use RSTA's actions, is that they are automatically updated to become enabled/disabled when the undo state changes, and to automatically show "Undo" or "Can't undo". If we do that manually, as is done currently, we need access to the UndoManager which needs a patch (at least for the getUndoPresentationName(), IIRC RSTA does export a canUndo() or similar). Also, we need to keep track of changes within the document with change listeners and other stuff, which is a bit messy. Having said that, this approach isn't 100% elegant either, so I can understand your being hesitant.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the clarification, inedeed the fact that RSTA handle automatically the enable/disable is a point.

Another thing that comes to mind is how the localization is handled? is it something already in the RSTA resources? about getUndoPresentationName() it seems that the menu is always presented as "Undo" or "Redo", IMHO it's something that can be easily dropped in case.

undoItem.setName("menuEditUndo");
undoItem.addActionListener(undoAction = new UndoAction());
menu.add(undoItem);

if (!OSUtils.isMacOS()) {
redoItem = newJMenuItem(tr("Redo"), 'Y');
} else {
redoItem = newJMenuItemShift(tr("Redo"), 'Z');
}
RecordableTextAction redoAction = RTextArea.getAction(RTextArea.REDO_ACTION);
JMenuItem redoItem = new JMenuItem(new RstaActionWrapper(redoAction));
redoItem.setName("menuEditRedo");
redoItem.addActionListener(redoAction = new RedoAction());
menu.add(redoItem);

// RSTA defaults to Ctrl-Y for redo, but convention on OSX is Ctrl-Shift-Z
// This might not be the best place, but it works.
if (OSUtils.isMacOS()) {
int mods = SHORTCUT_KEY_MASK | ActionEvent.SHIFT_MASK;
redoAction.setAccelerator(KeyStroke.getKeyStroke(KeyEvent.VK_Z, mods));
}

menu.addSeparator();

JMenuItem cutItem = newJMenuItem(tr("Cut"), 'X');
cutItem.addActionListener(new ActionListener() {
public void actionPerformed(ActionEvent e) {
getCurrentTab().handleCut();
}
});
menu.add(cutItem);
RecordableTextAction cutAction = RTextArea.getAction(RTextArea.CUT_ACTION);
menu.add(new JMenuItem(new RstaActionWrapper(cutAction)));

RecordableTextAction copyAction = RTextArea.getAction(RTextArea.COPY_ACTION);
menu.add(new JMenuItem(new RstaActionWrapper(copyAction)));

JMenuItem copyItem = newJMenuItem(tr("Copy"), 'C');
copyItem.addActionListener(new ActionListener() {
public void actionPerformed(ActionEvent e) {
getCurrentTab().getTextArea().copy();
}
});
menu.add(copyItem);

JMenuItem copyForumItem = newJMenuItemShift(tr("Copy for Forum"), 'C');
copyForumItem.addActionListener(new ActionListener() {
Expand All @@ -1319,21 +1329,11 @@ public void actionPerformed(ActionEvent e) {
});
menu.add(copyHTMLItem);

JMenuItem pasteItem = newJMenuItem(tr("Paste"), 'V');
pasteItem.addActionListener(new ActionListener() {
public void actionPerformed(ActionEvent e) {
getCurrentTab().handlePaste();
}
});
menu.add(pasteItem);
RecordableTextAction pasteAction = RTextArea.getAction(RTextArea.PASTE_ACTION);
menu.add(new JMenuItem(new RstaActionWrapper(pasteAction)));

JMenuItem selectAllItem = newJMenuItem(tr("Select All"), 'A');
selectAllItem.addActionListener(new ActionListener() {
public void actionPerformed(ActionEvent e) {
getCurrentTab().handleSelectAll();
}
});
menu.add(selectAllItem);
RecordableTextAction selectAllAction = RTextArea.getAction(RTextArea.SELECT_ALL_ACTION);
menu.add(new JMenuItem(new RstaActionWrapper(selectAllAction)));

JMenuItem gotoLine = newJMenuItem(tr("Go to line..."), 'L');
gotoLine.addActionListener(e -> {
Expand Down Expand Up @@ -1422,21 +1422,6 @@ public void actionPerformed(ActionEvent e) {
menu.add(useSelectionForFindItem);
}

menu.addMenuListener(new MenuListener() {
@Override
public void menuSelected(MenuEvent e) {
boolean enabled = getCurrentTab().getSelectedText() != null;
cutItem.setEnabled(enabled);
copyItem.setEnabled(enabled);
}

@Override
public void menuDeselected(MenuEvent e) {}

@Override
public void menuCanceled(MenuEvent e) {}
});

return menu;
}

Expand Down Expand Up @@ -1474,75 +1459,6 @@ private static JMenuItem newJMenuItemAlt(String title, int what) {
return menuItem;
}


// . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .


class UndoAction extends AbstractAction {
public UndoAction() {
super("Undo");
this.setEnabled(false);
}

public void actionPerformed(ActionEvent e) {
try {
getCurrentTab().handleUndo();
} catch (CannotUndoException ex) {
//System.out.println("Unable to undo: " + ex);
//ex.printStackTrace();
}
}

protected void updateUndoState() {
UndoManager undo = getCurrentTab().getUndoManager();

if (undo.canUndo()) {
this.setEnabled(true);
undoItem.setEnabled(true);
undoItem.setText(undo.getUndoPresentationName());
putValue(Action.NAME, undo.getUndoPresentationName());
} else {
this.setEnabled(false);
undoItem.setEnabled(false);
undoItem.setText(tr("Undo"));
putValue(Action.NAME, "Undo");
}
}
}


class RedoAction extends AbstractAction {
public RedoAction() {
super("Redo");
this.setEnabled(false);
}

public void actionPerformed(ActionEvent e) {
try {
getCurrentTab().handleRedo();
} catch (CannotRedoException ex) {
//System.out.println("Unable to redo: " + ex);
//ex.printStackTrace();
}
}

protected void updateRedoState() {
UndoManager undo = getCurrentTab().getUndoManager();

if (undo.canRedo()) {
redoItem.setEnabled(true);
redoItem.setText(undo.getRedoPresentationName());
putValue(Action.NAME, undo.getRedoPresentationName());
} else {
this.setEnabled(false);
redoItem.setEnabled(false);
redoItem.setText(tr("Redo"));
putValue(Action.NAME, "Redo");
}
}
}


// . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .


Expand Down Expand Up @@ -1610,8 +1526,6 @@ public List<EditorTab> getTabs() {
*/
public void selectTab(final int index) {
currentTabIndex = index;
undoAction.updateUndoState();
redoAction.updateRedoState();
updateTitle();
header.rebuild();
getCurrentTab().activated();
Expand Down
14 changes: 2 additions & 12 deletions app/src/processing/app/EditorTab.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,13 @@
import javax.swing.text.BadLocationException;
import javax.swing.text.Element;
import javax.swing.text.PlainDocument;
import javax.swing.undo.UndoManager;
import javax.swing.text.DefaultCaret;

import org.fife.ui.rsyntaxtextarea.RSyntaxDocument;
import org.fife.ui.rsyntaxtextarea.RSyntaxTextAreaEditorKit;
import org.fife.ui.rsyntaxtextarea.RSyntaxUtilities;
import org.fife.ui.rtextarea.Gutter;
import org.fife.ui.rtextarea.RTextScrollPane;
import org.fife.ui.rtextarea.RUndoManager;

import cc.arduino.UpdatableBoardsLibsFakeURLsHandler;
import processing.app.helpers.DocumentTextChangeListener;
Expand Down Expand Up @@ -107,10 +105,6 @@ public EditorTab(Editor editor, SketchFile file, String contents)
file.setStorage(this);
applyPreferences();
add(this.scrollPane, BorderLayout.CENTER);

RUndoManager undo = new LastUndoableEditAwareUndoManager(this.textarea, this.editor);
document.addUndoableEditListener(undo);
textarea.setUndoManager(undo);
}

private RSyntaxDocument createDocument(String contents) {
Expand Down Expand Up @@ -406,14 +400,14 @@ public void setText(String what) {
int oldLength = doc.getLength();
// The undo manager already seems to group the insert and remove together
// automatically, but better be explicit about it.
textarea.getUndoManager().beginInternalAtomicEdit();
textarea.beginAtomicEdit();
try {
doc.insertString(oldLength, what, null);
doc.remove(0, oldLength);
} catch (BadLocationException e) {
System.err.println("Unexpected failure replacing text");
} finally {
textarea.getUndoManager().endInternalAtomicEdit();
textarea.endAtomicEdit();
}
} finally {
caret.setUpdatePolicy(policy);
Expand Down Expand Up @@ -553,10 +547,6 @@ void handleRedo() {
textarea.redoLastAction();
}

public UndoManager getUndoManager() {
return textarea.getUndoManager();
}

public String getCurrentKeyword() {
String text = "";
if (textarea.getSelectedText() != null)
Expand Down
Loading