Skip to content

Commit 6c1c0b8

Browse files
GubaerGubaer
committed
improve, clean up, and unify feedback for scripting errors
1 parent 7061bc0 commit 6c1c0b8

File tree

7 files changed

+190
-324
lines changed

7 files changed

+190
-324
lines changed

‎src/main/java/org/openstreetmap/josm/plugins/scripting/ui/IScriptErrorHandler.java‎

Lines changed: 0 additions & 14 deletions
This file was deleted.

‎src/main/java/org/openstreetmap/josm/plugins/scripting/ui/RunScriptService.java‎

Lines changed: 13 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,9 @@
22

33
import org.openstreetmap.josm.data.Preferences;
44
import org.openstreetmap.josm.gui.HelpAwareOptionPane;
5+
import org.openstreetmap.josm.gui.MainApplication;
56
import org.openstreetmap.josm.gui.help.HelpUtil;
6-
import org.openstreetmap.josm.plugins.scripting.graalvm.GraalVMEvalException;
77
import org.openstreetmap.josm.plugins.scripting.graalvm.GraalVMFacadeFactory;
8-
import org.openstreetmap.josm.plugins.scripting.graalvm.IGraalVMFacade;
98
import org.openstreetmap.josm.plugins.scripting.model.JSR223ScriptEngineProvider;
109
import org.openstreetmap.josm.plugins.scripting.model.ScriptEngineDescriptor;
1110

@@ -16,7 +15,6 @@
1615
import java.io.File;
1716
import java.io.IOException;
1817
import java.io.Reader;
19-
import java.text.MessageFormat;
2018
import java.util.Objects;
2119
import java.util.logging.Level;
2220
import java.util.logging.Logger;
@@ -183,23 +181,7 @@ public boolean canRunScript(String fileName, Component parent) {
183181
* @throws NullPointerException thrown if engine is null
184182
*/
185183
public void runScript(@NotNull String fileName, @NotNull ScriptEngineDescriptor engine) {
186-
runScript(fileName, engine, null);
187-
}
188-
189-
protected void runScriptWithGraalVM(
190-
final File script, final ScriptEngineDescriptor engine)
191-
throws IOException, GraalVMEvalException {
192-
if (logger.isLoggable(Level.FINE)) {
193-
final String message = MessageFormat.format(
194-
"executing script with GraalVM ''{0}''. Script file: ''{1}''",
195-
engine.getEngineId(),
196-
script.getAbsolutePath()
197-
);
198-
logger.log(Level.FINE, message);
199-
}
200-
final IGraalVMFacade facade = GraalVMFacadeFactory
201-
.getOrCreateGraalVMFacade();
202-
facade.eval(engine, script);
184+
runScript(fileName, engine, null /* use default parent */);
203185
}
204186

205187
/**
@@ -209,25 +191,28 @@ protected void runScriptWithGraalVM(
209191
*
210192
* @param fileName the script file name
211193
* @param engine the script engine descriptor
212-
* @param parent the parent component
194+
* @param parent the parent component. Uses {@link MainApplication#getMainFrame()} if null.
195+
* @throws NullPointerException if <code>fileName</code> is null
196+
* @throws NullPointerException if <code>engine</code> is null
213197
*/
214-
public void runScript(@NotNull String fileName,
215-
@NotNull ScriptEngineDescriptor engine,
198+
public void runScript(@NotNull final String fileName,
199+
@NotNull final ScriptEngineDescriptor engine,
216200
@Null Component parent) {
217201
Objects.requireNonNull(fileName);
218202
Objects.requireNonNull(engine);
219203
File f = new File(fileName);
204+
if (parent == null) {
205+
parent = MainApplication.getMainFrame();
206+
}
220207

221-
MostRecentlyRunScriptsModel model = MostRecentlyRunScriptsModel
222-
.getInstance();
208+
final var model = MostRecentlyRunScriptsModel.getInstance();
223209
model.remember(f.getAbsolutePath());
224210
model.saveToPreferences(Preferences.main());
225211

226212
switch(engine.getEngineType()){
227213
case EMBEDDED:
228214
if (logger.isLoggable(Level.FINE)) {
229-
logger.log(Level.FINE,
230-
"executing script with embedded engine ...");
215+
logger.log(Level.FINE, "executing script with embedded engine ...");
231216
}
232217
new ScriptExecutor(parent).runScriptWithEmbeddedEngine(f);
233218
break;
@@ -237,11 +222,7 @@ public void runScript(@NotNull String fileName,
237222
break;
238223

239224
case GRAALVM:
240-
try {
241-
runScriptWithGraalVM(f, engine);
242-
} catch(IOException |GraalVMEvalException e) {
243-
logger.log(Level.SEVERE, e.getMessage(), e);
244-
}
225+
new ScriptExecutor(parent).runScriptWithGraalVM(f, engine);
245226
break;
246227
}
247228
}

‎src/main/java/org/openstreetmap/josm/plugins/scripting/ui/ScriptErrorDialog.java‎

Lines changed: 46 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,86 +1,87 @@
11
package org.openstreetmap.josm.plugins.scripting.ui;
22

3+
import org.openstreetmap.josm.gui.MainApplication;
4+
import org.openstreetmap.josm.gui.util.WindowGeometry;
5+
36
import javax.swing.*;
47
import javax.validation.constraints.NotNull;
58
import java.awt.*;
6-
import java.io.PrintWriter;
7-
import java.io.StringWriter;
9+
import java.util.Objects;
810

911
import static org.openstreetmap.josm.tools.I18n.tr;
1012

1113
public class ScriptErrorDialog {
1214

13-
private static JPanel buildTopPanel() {
14-
final JPanel pnl = new JPanel(new BorderLayout());
15-
final JEditorPane pane = EditorPaneBuilder.buildInfoEditorPane();
16-
final String infoTxt = "<html>" + tr(
17-
"An error occurred when executing a script. This is most likely " +
18-
"a bug in the script, and not in JOSM.<p>" +
19-
"Please get in touch with the script author before you file " +
20-
"a bug in JOSMs bug tracker."
21-
) + "</html>";
22-
pane.setText(infoTxt);
23-
pnl.add(pane, BorderLayout.CENTER);
24-
return pnl;
25-
}
15+
private static class ContentPane extends JPanel {
2616

27-
private static String dumpStackTrace(Throwable t) {
28-
final StringWriter sw = new StringWriter();
29-
t.printStackTrace(new PrintWriter(sw));
30-
return sw.toString();
31-
}
17+
private ScriptErrorViewer viewer;
3218

33-
private static Component buildStacktracePane(Throwable forException) {
34-
// build text pane
35-
final JTextPane tp = new JTextPane();
36-
tp.setText(dumpStackTrace(forException));
37-
tp.setEditable(false);
38-
tp.setCaretPosition(0); // scroll to top
19+
private JPanel buildTopPanel() {
20+
final JPanel pnl = new JPanel(new BorderLayout());
21+
final JEditorPane pane = EditorPaneBuilder.buildInfoEditorPane();
22+
final String infoTxt = "<html>" + tr(
23+
"An error occurred when executing a script. This is most likely " +
24+
"a bug in the script, and not in JOSM.<p>" +
25+
"Please get in touch with the script author before you file " +
26+
"a bug in JOSMs bug tracker."
27+
) + "</html>";
28+
pane.setText(infoTxt);
29+
pnl.add(pane, BorderLayout.CENTER);
30+
return pnl;
31+
}
3932

40-
// build scroll pane
41-
final JScrollPane sp = new JScrollPane();
42-
sp.setHorizontalScrollBarPolicy(
43-
JScrollPane.HORIZONTAL_SCROLLBAR_AS_NEEDED);
44-
sp.setVerticalScrollBarPolicy(
45-
JScrollPane.VERTICAL_SCROLLBAR_AS_NEEDED);
46-
sp.getViewport().add(tp);
47-
return sp;
48-
}
33+
private void build() {
34+
setLayout(new BorderLayout());
35+
add(buildTopPanel(), BorderLayout.NORTH);
36+
add(viewer = new ScriptErrorViewer(), BorderLayout.CENTER);
37+
}
4938

50-
private static JPanel buildContentPanel(Throwable forException) {
51-
final JPanel pnl = new JPanel(new BorderLayout());
52-
pnl.add(buildTopPanel(), BorderLayout.NORTH);
53-
pnl.add(buildStacktracePane(forException), BorderLayout.CENTER);
54-
return pnl;
39+
public ContentPane() {
40+
build();
41+
}
42+
43+
public ScriptErrorViewerModel getModel() {
44+
return viewer.getModel();
45+
}
5546
}
5647

5748
/**
58-
* Display an error dialog with the stack trace for the exception.
49+
* Display an error dialog with the error information
5950
*
60-
* @param forException the exception whose stack trace is displayed
51+
* @param forException the exception
52+
* @throws NullPointerException thrown if <code>forException</code> is null
6153
*/
6254
@SuppressWarnings("unused")
6355
public static void showErrorDialog(@NotNull Throwable forException) {
6456
showErrorDialog(tr("Script Error - Stack Trace"), forException);
6557
}
6658

6759
/**
68-
* Display an error dialog with the stack trace for the exception and
60+
* Display an error dialog with the error information and
6961
* a custom title.
7062
*
7163
* @param title the dialog title
7264
* @param forException the exception whose stack trace is displayed
65+
* @throws NullPointerException thrown if <code>title</code> is null
66+
* @throws NullPointerException thrown if <code>forException</code> is null
7367
*/
7468
@SuppressWarnings("WeakerAccess") // part of the public API
7569
public static void showErrorDialog(@NotNull String title,
7670
@NotNull Throwable forException) {
71+
Objects.requireNonNull(title);
72+
Objects.requireNonNull(forException);
73+
final var contentPane = new ContentPane();
7774
final JOptionPane pane = new JOptionPane(
78-
buildContentPanel(forException),
75+
contentPane,
7976
JOptionPane.ERROR_MESSAGE
8077
);
78+
contentPane.getModel().setError(forException);
8179
final JDialog dialog = pane.createDialog(title);
82-
dialog.setSize(600,400);
8380
dialog.setResizable(true);
81+
WindowGeometry.centerInWindow(
82+
MainApplication.getMainFrame(),
83+
new Dimension(600, 400)
84+
).applySafe(dialog);
8485
dialog.setVisible(true);
8586
}
8687
}

src/main/java/org/openstreetmap/josm/plugins/scripting/ui/console/ErrorOutputPanel.java renamed to src/main/java/org/openstreetmap/josm/plugins/scripting/ui/ScriptErrorViewer.java

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package org.openstreetmap.josm.plugins.scripting.ui.console;
1+
package org.openstreetmap.josm.plugins.scripting.ui;
22

33

44
import org.mozilla.javascript.EcmaError;
@@ -9,23 +9,54 @@
99
import javax.validation.constraints.NotNull;
1010
import javax.validation.constraints.Null;
1111
import java.awt.*;
12+
import java.beans.PropertyChangeEvent;
13+
import java.beans.PropertyChangeListener;
1214
import java.io.PrintWriter;
1315
import java.io.StringWriter;
1416
import java.text.MessageFormat;
1517
import java.util.Arrays;
18+
import java.util.Objects;
1619
import java.util.stream.Collectors;
1720

1821
/**
19-
* Displays errors when the execution of a script fails
22+
* A {@link JPanel} which displays an error thrown during the
23+
* execution of a script.
2024
*/
21-
public class ErrorOutputPanel extends JPanel {
25+
public class ScriptErrorViewer extends JPanel {
2226

2327
private JTextPane paneOutput;
28+
private final ScriptErrorViewerModel model;
2429

25-
public ErrorOutputPanel() {
30+
/**
31+
* Creates a new viewer with a new view model.
32+
*
33+
* @see #ScriptErrorViewer(ScriptErrorViewerModel)
34+
*/
35+
public ScriptErrorViewer() {
36+
this(new ScriptErrorViewerModel());
37+
}
38+
39+
/**
40+
* Creates a new viewer with a supplied model.
41+
*
42+
* @param model the model
43+
* @throws NullPointerException thrown if <code>model</code> is null
44+
*/
45+
public ScriptErrorViewer(@NotNull final ScriptErrorViewerModel model) {
46+
Objects.requireNonNull(model);
47+
this.model = model;
2648
build();
2749
}
2850

51+
/**
52+
* Replies the view model
53+
*
54+
* @return the model
55+
*/
56+
public @NotNull ScriptErrorViewerModel getModel() {
57+
return model;
58+
}
59+
2960
protected void build() {
3061
setLayout(new BorderLayout());
3162
paneOutput = new JTextPane();
@@ -36,6 +67,7 @@ protected void build() {
3667
editorScrollPane.setHorizontalScrollBarPolicy(
3768
JScrollPane.HORIZONTAL_SCROLLBAR_AS_NEEDED);
3869
add(editorScrollPane, BorderLayout.CENTER);
70+
model.addPropertyChangeListener(new ErrorModelChangeListener());
3971
}
4072

4173
protected void displayPolyglotException(Throwable exception) {
@@ -125,4 +157,18 @@ protected String formatPolyglotException(@NotNull final Throwable exception) {
125157
))
126158
.collect(Collectors.joining("\n"));
127159
}
160+
161+
class ErrorModelChangeListener implements PropertyChangeListener {
162+
@Override
163+
public void propertyChange(PropertyChangeEvent event) {
164+
if (! ScriptErrorViewerModel.PROP_ERROR.equals(event.getPropertyName())) {
165+
return;
166+
}
167+
if (event.getNewValue() == null) {
168+
displayException(null);
169+
} else if (event.getNewValue() instanceof Throwable) {
170+
displayException((Throwable) event.getNewValue());
171+
}
172+
}
173+
}
128174
}

src/main/java/org/openstreetmap/josm/plugins/scripting/ui/console/ErrorModel.java renamed to src/main/java/org/openstreetmap/josm/plugins/scripting/ui/ScriptErrorViewerModel.java

Lines changed: 4 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,18 @@
1-
package org.openstreetmap.josm.plugins.scripting.ui.console;
1+
package org.openstreetmap.josm.plugins.scripting.ui;
22

3-
import org.openstreetmap.josm.plugins.scripting.ui.IScriptErrorHandler;
4-
5-
import javax.validation.constraints.NotNull;
63
import javax.validation.constraints.Null;
74
import java.beans.PropertyChangeEvent;
85
import java.beans.PropertyChangeListener;
96
import java.beans.PropertyChangeSupport;
107

11-
public class ErrorModel implements IScriptErrorHandler {
8+
public class ScriptErrorViewerModel {
129
/**
1310
* The name of the error property
1411
*/
15-
public static final String PROP_ERROR = ErrorModel.class.getName() + ".errorChanged";
16-
17-
private static ErrorModel instance;
12+
public static final String PROP_ERROR = ScriptErrorViewerModel.class.getName() + ".errorChanged";
1813

1914
private Throwable error = null;
2015

21-
/**
22-
* Replies the unique instance of the error model
23-
*
24-
* @return the error model
25-
*/
26-
public static @NotNull ErrorModel getInstance() {
27-
if (instance == null) {
28-
instance = new ErrorModel();
29-
}
30-
return instance;
31-
}
32-
3316
final private PropertyChangeSupport support = new PropertyChangeSupport(this);
3417

3518
/**
@@ -75,7 +58,7 @@ protected void fireErrorChanged(Throwable oldError, Throwable newError) {
7558
support.firePropertyChange(event);
7659
}
7760

78-
private ErrorModel() {
61+
public ScriptErrorViewerModel() {
7962
}
8063

8164
/**
@@ -95,12 +78,4 @@ public void setError(@Null Throwable error) {
9578
public void clearError() {
9679
setError(null);
9780
}
98-
99-
/* -------------------------------------------------------------------- */
100-
/* IScriptErrorHandler */
101-
/* -------------------------------------------------------------------- */
102-
@Override
103-
public void handleScriptExecutionError(Throwable exception) {
104-
setError(exception);
105-
}
10681
}

0 commit comments

Comments
 (0)