From 2af2c7601d02142affeb371a1e388935563c0007 Mon Sep 17 00:00:00 2001 From: Sam Carlberg Date: Tue, 26 Jul 2016 13:44:27 -0400 Subject: [PATCH 1/8] Add barebones UI support for custom Python operations --- .../wpi/grip/core/OperationDescription.java | 1 + .../wpi/grip/core/operations/Operations.java | 19 ++- .../python/PythonOperationUtils.java | 52 ++++++++ .../{ => python}/PythonScriptFile.java | 16 ++- .../{ => python}/PythonScriptOperation.java | 66 ++++------- .../java/edu/wpi/grip/core/PythonTest.java | 2 +- .../grip/core/serialization/ProjectTest.java | 2 +- .../ui/CustomOperationsListController.java | 112 ++++++++++++++++++ .../wpi/grip/ui/OperationListController.java | 12 +- .../edu/wpi/grip/ui/PaletteController.java | 20 ++-- .../edu/wpi/grip/ui/CustomOperationsList.fxml | 16 +++ .../edu/wpi/grip/ui/OperationList.fxml | 12 +- .../resources/edu/wpi/grip/ui/Palette.fxml | 30 +++-- 13 files changed, 280 insertions(+), 80 deletions(-) create mode 100644 core/src/main/java/edu/wpi/grip/core/operations/python/PythonOperationUtils.java rename core/src/main/java/edu/wpi/grip/core/operations/{ => python}/PythonScriptFile.java (83%) rename core/src/main/java/edu/wpi/grip/core/operations/{ => python}/PythonScriptOperation.java (73%) create mode 100644 ui/src/main/java/edu/wpi/grip/ui/CustomOperationsListController.java create mode 100644 ui/src/main/resources/edu/wpi/grip/ui/CustomOperationsList.fxml diff --git a/core/src/main/java/edu/wpi/grip/core/OperationDescription.java b/core/src/main/java/edu/wpi/grip/core/OperationDescription.java index d1872567a4..90cbcddef8 100644 --- a/core/src/main/java/edu/wpi/grip/core/OperationDescription.java +++ b/core/src/main/java/edu/wpi/grip/core/OperationDescription.java @@ -137,6 +137,7 @@ public enum Category { LOGICAL, OPENCV, MISCELLANEOUS, + CUSTOM, } /** diff --git a/core/src/main/java/edu/wpi/grip/core/operations/Operations.java b/core/src/main/java/edu/wpi/grip/core/operations/Operations.java index 7056f7e5fe..30203f8cb0 100644 --- a/core/src/main/java/edu/wpi/grip/core/operations/Operations.java +++ b/core/src/main/java/edu/wpi/grip/core/operations/Operations.java @@ -40,6 +40,9 @@ import edu.wpi.grip.core.operations.opencv.MinMaxLoc; import edu.wpi.grip.core.operations.opencv.NewPointOperation; import edu.wpi.grip.core.operations.opencv.NewSizeOperation; +import edu.wpi.grip.core.operations.python.PythonOperationUtils; +import edu.wpi.grip.core.operations.python.PythonScriptFile; +import edu.wpi.grip.core.operations.python.PythonScriptOperation; import edu.wpi.grip.core.sockets.InputSocket; import edu.wpi.grip.core.sockets.OutputSocket; @@ -53,6 +56,10 @@ import org.bytedeco.javacpp.opencv_core.Point; import org.bytedeco.javacpp.opencv_core.Size; +import java.util.Arrays; +import java.util.stream.Collectors; +import java.util.stream.Stream; + import static com.google.common.base.Preconditions.checkNotNull; @Singleton @@ -75,7 +82,8 @@ public class Operations { checkNotNull(httpPublishFactory, "httpPublisherFactory cannot be null"); checkNotNull(rosPublishFactory, "rosPublishFactory cannot be null"); checkNotNull(fileManager, "fileManager cannot be null"); - this.operations = ImmutableList.of( + PythonOperationUtils.checkDirExists(); + this.operations = new ImmutableList.Builder().addAll(Arrays.asList( // Composite operations new OperationMetaData(BlurOperation.DESCRIPTION, () -> new BlurOperation(isf, osf)), @@ -186,7 +194,14 @@ public class Operations { new OperationMetaData(HttpPublishOperation.descriptionFor(Boolean.class), () -> new HttpPublishOperation<>(isf, Boolean.class, BooleanPublishable.class, BooleanPublishable::new, httpPublishFactory)) - ); + )).addAll( + Stream.of(PythonOperationUtils.DIRECTORY.listFiles((dir, name) -> name.endsWith(".py"))) + .map(PythonOperationUtils::read) + .map(PythonScriptFile::create) + .map(psf -> new OperationMetaData(PythonScriptOperation.descriptionFor(psf), + () -> new PythonScriptOperation(isf, osf, psf))) + .collect(Collectors.toList()) + ).build(); } @VisibleForTesting diff --git a/core/src/main/java/edu/wpi/grip/core/operations/python/PythonOperationUtils.java b/core/src/main/java/edu/wpi/grip/core/operations/python/PythonOperationUtils.java new file mode 100644 index 0000000000..1ceec6065c --- /dev/null +++ b/core/src/main/java/edu/wpi/grip/core/operations/python/PythonOperationUtils.java @@ -0,0 +1,52 @@ +package edu.wpi.grip.core.operations.python; + +import edu.wpi.grip.core.GripFileManager; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.Charset; +import java.nio.file.Files; + +/** + * Utility class handling functionality for custom python operation files on disk. + */ +public final class PythonOperationUtils { + + /** + * The directory where custom python operation files are stored. + */ + public static final File DIRECTORY = new File(GripFileManager.GRIP_DIRECTORY, "operations"); + + private PythonOperationUtils() { + + } + + /** + * Reads the contents of the given file. Assumes it's encoded as UTF-8. + * + * @param file the file to read + * @return the String contents of the file, in UTF-8 encoding + */ + public static String read(File file) { + if (!file.getParentFile().equals(DIRECTORY) || !file.getName().endsWith(".py")) { + throw new IllegalArgumentException( + "Not a custom python operation: " + file.getAbsolutePath()); + } + try { + return new String(Files.readAllBytes(file.toPath()), Charset.forName("UTF-8")); + } catch (IOException e) { + throw new RuntimeException("Could not read " + file.getAbsolutePath(), e); + } + } + + /** + * Ensures that {@link #DIRECTORY} exists. + */ + public static void checkDirExists() { + if (DIRECTORY.exists()) { + return; + } + DIRECTORY.mkdirs(); + } + +} diff --git a/core/src/main/java/edu/wpi/grip/core/operations/PythonScriptFile.java b/core/src/main/java/edu/wpi/grip/core/operations/python/PythonScriptFile.java similarity index 83% rename from core/src/main/java/edu/wpi/grip/core/operations/PythonScriptFile.java rename to core/src/main/java/edu/wpi/grip/core/operations/python/PythonScriptFile.java index c29536bfa4..8f504db9f7 100644 --- a/core/src/main/java/edu/wpi/grip/core/operations/PythonScriptFile.java +++ b/core/src/main/java/edu/wpi/grip/core/operations/python/PythonScriptFile.java @@ -1,4 +1,4 @@ -package edu.wpi.grip.core.operations; +package edu.wpi.grip.core.operations.python; import edu.wpi.grip.core.OperationMetaData; @@ -26,6 +26,20 @@ @AutoValue public abstract class PythonScriptFile { + public static final String TEMPLATE = + "import edu.wpi.grip.core.sockets as grip\n\n" + + "name = \"Addition Sample\"\n" + + "summary = \"The sample python operation to add two numbers\"\n\n" + + "inputs = [\n" + + " grip.SocketHints.createNumberSocketHint(\"a\", 0.0),\n" + + " grip.SocketHints.createNumberSocketHint(\"b\", 0.0),\n" + + "]\n" + + "outputs = [\n" + + " grip.SocketHints.createNumberSocketHint(\"sum\", 0.0),\n" + + "]\n\n" + + "def perform(a, b):\n" + + " return a + b\n"; + static { Properties pythonProperties = new Properties(); pythonProperties.setProperty("python.import.site", "false"); diff --git a/core/src/main/java/edu/wpi/grip/core/operations/PythonScriptOperation.java b/core/src/main/java/edu/wpi/grip/core/operations/python/PythonScriptOperation.java similarity index 73% rename from core/src/main/java/edu/wpi/grip/core/operations/PythonScriptOperation.java rename to core/src/main/java/edu/wpi/grip/core/operations/python/PythonScriptOperation.java index 378bbcee6e..e7cd996e40 100644 --- a/core/src/main/java/edu/wpi/grip/core/operations/PythonScriptOperation.java +++ b/core/src/main/java/edu/wpi/grip/core/operations/python/PythonScriptOperation.java @@ -1,4 +1,4 @@ -package edu.wpi.grip.core.operations; +package edu.wpi.grip.core.operations.python; import edu.wpi.grip.core.Operation; import edu.wpi.grip.core.OperationDescription; @@ -13,7 +13,6 @@ import org.python.core.PySequence; import java.util.List; -import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -81,7 +80,7 @@ public static OperationDescription descriptionFor(PythonScriptFile pythonScriptF .name(pythonScriptFile.name()) .summary(pythonScriptFile.summary()) .icon(Icon.iconStream("python")) - .category(OperationDescription.Category.MISCELLANEOUS) + .category(OperationDescription.Category.CUSTOM) .build(); } @@ -121,44 +120,31 @@ public void perform() { pyInputs[i] = Py.java2py(inputSockets.get(i).getValue().get()); } - try { - PyObject pyOutput = this.scriptFile.performFunction().__call__(pyInputs); - - if (pyOutput.isSequenceType()) { - /* - * If the Python function returned a sequence type, there must be multiple outputs for - * this step. - * Each element in the sequence is assigned to one output socket. - */ - PySequence pySequence = (PySequence) pyOutput; - Object[] javaOutputs = Py.tojava(pySequence, Object[].class); - - if (outputSockets.size() != javaOutputs.length) { - throw new IllegalArgumentException(wrongNumberOfArgumentsMsg(outputSockets.size(), - javaOutputs.length)); - } - - for (int i = 0; i < javaOutputs.length; i++) { - outputSockets.get(i).setValue(javaOutputs[i]); - } - } else { - /* If the Python script did not return a sequence, there should only be one - output socket. */ - if (outputSockets.size() != 1) { - throw new IllegalArgumentException(wrongNumberOfArgumentsMsg(outputSockets.size(), 1)); - } - - Object javaOutput = Py.tojava(pyOutput, outputSockets.get(0).getSocketHint().getType()); - outputSockets.get(0).setValue(javaOutput); + PyObject pyOutput = this.scriptFile.performFunction().__call__(pyInputs); + + if (pyOutput.isSequenceType()) { + // If the Python function returned a sequence type, + // there must be multiple outputs for this step. + // Each element in the sequence is assigned to one output socket. + PySequence pySequence = (PySequence) pyOutput; + Object[] javaOutputs = Py.tojava(pySequence, Object[].class); + + if (outputSockets.size() != javaOutputs.length) { + throw new IllegalArgumentException(wrongNumberOfArgumentsMsg(outputSockets.size(), + javaOutputs.length)); + } + + for (int i = 0; i < javaOutputs.length; i++) { + outputSockets.get(i).setValue(javaOutputs[i]); + } + } else { + // If the Python script did not return a sequence, there should only be one output socket. + if (outputSockets.size() != 1) { + throw new IllegalArgumentException(wrongNumberOfArgumentsMsg(outputSockets.size(), 1)); } - } catch (RuntimeException e) { - /* Exceptions can happen if there's a mistake in a Python script, so just print a - stack trace and leave the - * current state of the output sockets alone. - * - * TODO: communicate the error to the GUI. - */ - logger.log(Level.WARNING, e.getMessage(), e); + + Object javaOutput = Py.tojava(pyOutput, outputSockets.get(0).getSocketHint().getType()); + outputSockets.get(0).setValue(javaOutput); } } } diff --git a/core/src/test/java/edu/wpi/grip/core/PythonTest.java b/core/src/test/java/edu/wpi/grip/core/PythonTest.java index ed37e68a5a..b313007134 100644 --- a/core/src/test/java/edu/wpi/grip/core/PythonTest.java +++ b/core/src/test/java/edu/wpi/grip/core/PythonTest.java @@ -1,6 +1,6 @@ package edu.wpi.grip.core; -import edu.wpi.grip.core.operations.PythonScriptFile; +import edu.wpi.grip.core.operations.python.PythonScriptFile; import edu.wpi.grip.core.sockets.InputSocket; import edu.wpi.grip.core.sockets.OutputSocket; import edu.wpi.grip.core.sockets.Socket; diff --git a/core/src/test/java/edu/wpi/grip/core/serialization/ProjectTest.java b/core/src/test/java/edu/wpi/grip/core/serialization/ProjectTest.java index e62021d756..881db95fc8 100644 --- a/core/src/test/java/edu/wpi/grip/core/serialization/ProjectTest.java +++ b/core/src/test/java/edu/wpi/grip/core/serialization/ProjectTest.java @@ -12,7 +12,7 @@ import edu.wpi.grip.core.events.OperationAddedEvent; import edu.wpi.grip.core.events.ProjectSettingsChangedEvent; import edu.wpi.grip.core.events.SourceAddedEvent; -import edu.wpi.grip.core.operations.PythonScriptFile; +import edu.wpi.grip.core.operations.python.PythonScriptFile; import edu.wpi.grip.core.settings.ProjectSettings; import edu.wpi.grip.core.sockets.InputSocket; import edu.wpi.grip.core.sockets.OutputSocket; diff --git a/ui/src/main/java/edu/wpi/grip/ui/CustomOperationsListController.java b/ui/src/main/java/edu/wpi/grip/ui/CustomOperationsListController.java new file mode 100644 index 0000000000..6387ad468e --- /dev/null +++ b/ui/src/main/java/edu/wpi/grip/ui/CustomOperationsListController.java @@ -0,0 +1,112 @@ +package edu.wpi.grip.ui; + +import edu.wpi.grip.core.GripFileManager; +import edu.wpi.grip.core.OperationMetaData; +import edu.wpi.grip.core.events.OperationAddedEvent; +import edu.wpi.grip.core.operations.python.PythonScriptFile; +import edu.wpi.grip.core.operations.python.PythonScriptOperation; +import edu.wpi.grip.core.sockets.InputSocket; +import edu.wpi.grip.core.sockets.OutputSocket; +import edu.wpi.grip.ui.annotations.ParametrizedController; + +import com.google.common.eventbus.EventBus; +import com.google.common.io.Files; +import com.google.inject.Inject; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.Charset; +import java.util.Optional; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import javafx.event.ActionEvent; +import javafx.fxml.FXML; +import javafx.scene.control.Alert; +import javafx.scene.control.ButtonType; +import javafx.scene.control.Dialog; +import javafx.scene.control.TextArea; +import javafx.scene.control.TextInputDialog; + +/** + * Controller for the custom operations list. + */ +@ParametrizedController(url = "CustomOperationsList.fxml") +public class CustomOperationsListController extends OperationListController { + + @Inject private EventBus eventBus; + @Inject private InputSocket.Factory isf; + @Inject private OutputSocket.Factory osf; + + @Override + protected void initialize() { + super.initialize(); + } + + @FXML + private void createNewPythonOperation(ActionEvent actionEvent) { + Dialog dialog = new TextInputDialog(); + dialog.getDialogPane().setContent(new TextArea(PythonScriptFile.TEMPLATE)); + dialog.setResultConverter(bt -> { + if (bt == ButtonType.OK) { + return ((TextArea) dialog.getDialogPane().getContent()).getText(); + } + return null; + }); + Optional result = dialog.showAndWait(); + if (result.isPresent()) { + String code = result.get(); + String[] lines = code.split("\n"); + String name = null; + // Find the name in the user code + final Pattern p = Pattern.compile("name *= *\"(.*)\" *"); + for (String line : lines) { + Matcher m = p.matcher(line); + if (m.matches()) { + name = m.group(1); + break; + } + } + if (name == null) { + Alert a = new Alert(Alert.AlertType.ERROR); + a.setContentText("A name must be specified"); + a.showAndWait(); + return; + } else if (name.isEmpty() || name.matches("[ \t]+")) { + // Empty names are not allowed + Alert a = new Alert(Alert.AlertType.ERROR); + a.setContentText("Name cannot be empty"); + a.showAndWait(); + return; + } else if (!name.matches("[a-zA-Z0-9_\\- ]+")) { + // Name can only contain (English) letters, numbers, underscores, dashes, and spaces + Alert a = new Alert(Alert.AlertType.ERROR); + a.setContentText("Name contains illegal characters"); + a.showAndWait(); + return; + } + File file = new File(GripFileManager.GRIP_DIRECTORY + File.separator + "operations", + name.replaceAll("[\\s]", "_") + ".py"); + if (file.exists()) { + Alert a = new Alert(Alert.AlertType.ERROR); + a.setContentText("A file for the custom operation \"" + name + "\" already exists"); + a.showAndWait(); + return; + } + try { + Files.write(code, file, Charset.defaultCharset()); + } catch (IOException e) { + Alert a = new Alert(Alert.AlertType.ERROR); + a.setContentText("Could not save custom operation to " + file.getAbsolutePath()); + a.showAndWait(); + return; + } + PythonScriptFile pcs = PythonScriptFile.create(code); + eventBus.post(new OperationAddedEvent(new OperationMetaData( + PythonScriptOperation.descriptionFor(pcs), + () -> new PythonScriptOperation(isf, osf, pcs) + ))); + } + } + +} diff --git a/ui/src/main/java/edu/wpi/grip/ui/OperationListController.java b/ui/src/main/java/edu/wpi/grip/ui/OperationListController.java index cb0135087c..6e56d531fa 100644 --- a/ui/src/main/java/edu/wpi/grip/ui/OperationListController.java +++ b/ui/src/main/java/edu/wpi/grip/ui/OperationListController.java @@ -17,7 +17,7 @@ import javafx.collections.MapChangeListener; import javafx.fxml.FXML; import javafx.scene.Node; -import javafx.scene.control.Tab; +import javafx.scene.control.TitledPane; import javafx.scene.layout.VBox; /** @@ -31,7 +31,7 @@ public class OperationListController { protected static final String FILTER_TEXT = "filterText"; private final StringProperty filterText = new SimpleStringProperty(this, FILTER_TEXT, ""); - @FXML private Tab root; + @FXML private TitledPane root; @FXML private VBox operations; @Inject private OperationController.Factory operationControllerFactory; @SuppressWarnings("PMD.SingularField") private String baseText = null; @@ -43,7 +43,7 @@ protected void initialize() { baseText = root.getText(); InvalidationListener filterOperations = observable -> { - if (baseText == null) { + if (baseText == null || baseText.isEmpty()) { baseText = root.getText(); } @@ -60,10 +60,8 @@ protected void initialize() { if (!filter.isEmpty() && numMatches > 0) { // If we're filtering some operations and there's at least one match, set the title to - // bold and show the - // number of matches. This lets the user quickly see which tabs have matching operations - // when - // searching. + // bold and show the umber of matches. + // This lets the user quickly see which tabs have matching operations when searching. root.setText(baseText + " (" + numMatches + ")"); root.styleProperty().setValue("-fx-font-weight: bold"); } else { diff --git a/ui/src/main/java/edu/wpi/grip/ui/PaletteController.java b/ui/src/main/java/edu/wpi/grip/ui/PaletteController.java index 4fd3b9013c..dc4eb689c7 100644 --- a/ui/src/main/java/edu/wpi/grip/ui/PaletteController.java +++ b/ui/src/main/java/edu/wpi/grip/ui/PaletteController.java @@ -10,8 +10,8 @@ import javafx.beans.property.ObjectProperty; import javafx.fxml.FXML; -import javafx.scene.control.Tab; import javafx.scene.control.TextField; +import javafx.scene.control.TitledPane; import javafx.scene.layout.VBox; import javax.inject.Singleton; @@ -24,13 +24,14 @@ public class PaletteController { @FXML private VBox root; @FXML private CustomTextField operationSearch; - @FXML private Tab allOperations; - @FXML private Tab imgprocOperations; - @FXML private Tab featureOperations; - @FXML private Tab networkOperations; - @FXML private Tab logicalOperations; - @FXML private Tab opencvOperations; - @FXML private Tab miscellaneousOperations; + @FXML private TitledPane allOperations; + @FXML private TitledPane imgprocOperations; + @FXML private TitledPane featureOperations; + @FXML private TitledPane networkOperations; + @FXML private TitledPane logicalOperations; + @FXML private TitledPane opencvOperations; + @FXML private TitledPane miscellaneousOperations; + @FXML private TitledPane customOperations; @FXML protected void initialize() { @@ -51,6 +52,7 @@ protected void initialize() { logicalOperations.setUserData(OperationDescription.Category.LOGICAL); opencvOperations.setUserData(OperationDescription.Category.OPENCV); miscellaneousOperations.setUserData(OperationDescription.Category.MISCELLANEOUS); + customOperations.setUserData(OperationDescription.Category.CUSTOM); // Bind the filterText of all of the individual tabs to the search field operationSearch.textProperty().addListener(observable -> { @@ -68,6 +70,8 @@ protected void initialize() { .getText()); miscellaneousOperations.getProperties().put(OperationListController.FILTER_TEXT, operationSearch.getText()); + customOperations.getProperties().put(OperationListController.FILTER_TEXT, + operationSearch.getText()); }); // The palette should have a lower priority for resizing than other elements diff --git a/ui/src/main/resources/edu/wpi/grip/ui/CustomOperationsList.fxml b/ui/src/main/resources/edu/wpi/grip/ui/CustomOperationsList.fxml new file mode 100644 index 0000000000..8b14d481f6 --- /dev/null +++ b/ui/src/main/resources/edu/wpi/grip/ui/CustomOperationsList.fxml @@ -0,0 +1,16 @@ + + + + + + + + + + + + + From 92bfaa437c4fc8c9fd1042de50b0e58a362cfdba Mon Sep 17 00:00:00 2001 From: Sam Carlberg Date: Wed, 27 Jul 2016 16:33:27 -0400 Subject: [PATCH 6/8] Use RichTextFX for python editor --- build.gradle | 1 + .../main/java/edu/wpi/grip/core/Palette.java | 20 +- .../core/events/OperationRemovedEvent.java | 21 ++ .../ui/CustomOperationsListController.java | 105 +++----- .../wpi/grip/ui/OperationListController.java | 14 + .../ui/components/PreviousNextButtons.java | 1 + .../ui/preview/LinesSocketPreviewView.java | 1 + .../ui/python/PythonEditorController.java | 242 ++++++++++++++++++ .../edu/wpi/grip/ui/CustomOperationsList.fxml | 2 +- .../edu/wpi/grip/ui/python/PythonEditor.fxml | 62 +++++ .../wpi/grip/ui/python/python-keywords.css | 31 +++ 11 files changed, 421 insertions(+), 79 deletions(-) create mode 100644 core/src/main/java/edu/wpi/grip/core/events/OperationRemovedEvent.java create mode 100644 ui/src/main/java/edu/wpi/grip/ui/python/PythonEditorController.java create mode 100644 ui/src/main/resources/edu/wpi/grip/ui/python/PythonEditor.fxml create mode 100644 ui/src/main/resources/edu/wpi/grip/ui/python/python-keywords.css diff --git a/build.gradle b/build.gradle index 2548a22a1e..f33b9a9ec1 100644 --- a/build.gradle +++ b/build.gradle @@ -321,6 +321,7 @@ project(":ui") { testCompile group: 'org.testfx', name: 'testfx-core', version: '4.0.+' testCompile group: 'org.testfx', name: 'testfx-junit', version: '4.0.+' testRuntime group: 'org.testfx', name: 'openjfx-monocle', version: '1.8.0_20' + compile group: 'org.fxmisc.richtext', name: 'richtextfx', version: '0.7-M2' } evaluationDependsOn(':core') diff --git a/core/src/main/java/edu/wpi/grip/core/Palette.java b/core/src/main/java/edu/wpi/grip/core/Palette.java index 1768fc5e6a..ffc44a714f 100644 --- a/core/src/main/java/edu/wpi/grip/core/Palette.java +++ b/core/src/main/java/edu/wpi/grip/core/Palette.java @@ -1,8 +1,11 @@ package edu.wpi.grip.core; import edu.wpi.grip.core.events.OperationAddedEvent; +import edu.wpi.grip.core.events.OperationRemovedEvent; +import com.google.common.eventbus.EventBus; import com.google.common.eventbus.Subscribe; +import com.google.inject.Inject; import java.util.Collection; import java.util.LinkedHashMap; @@ -11,7 +14,6 @@ import javax.inject.Singleton; -import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; /** @@ -20,6 +22,7 @@ @Singleton public class Palette { + @Inject private EventBus eventBus; private final Map operations = new LinkedHashMap<>(); @Subscribe @@ -39,7 +42,20 @@ public void onOperationAdded(OperationAddedEvent event) { * @throws IllegalArgumentException if the key is already in the {@link #operations} map. */ private void map(String key, OperationMetaData operation) { - checkArgument(!operations.containsKey(key), "Operation name or alias already exists: " + key); + if (operations.containsKey(key)) { + OperationDescription existing = operations.get(key).getDescription(); + if (existing.category() == operation.getDescription().category() + && existing.category() == OperationDescription.Category.CUSTOM) { + // It's a custom operation that can be changed at runtime, allow it + // (But first remove the existing operation) + operations.remove(key); + eventBus.post(new OperationRemovedEvent(existing)); + } else { + // Not a custom operation, this should only happen if someone + // adds a new operation and uses an already-taken name + throw new IllegalArgumentException("Operation name or alias already exists: " + key); + } + } operations.put(key, operation); } diff --git a/core/src/main/java/edu/wpi/grip/core/events/OperationRemovedEvent.java b/core/src/main/java/edu/wpi/grip/core/events/OperationRemovedEvent.java new file mode 100644 index 0000000000..7cfa3e3c1b --- /dev/null +++ b/core/src/main/java/edu/wpi/grip/core/events/OperationRemovedEvent.java @@ -0,0 +1,21 @@ +package edu.wpi.grip.core.events; + +import edu.wpi.grip.core.OperationDescription; + +import static com.google.common.base.Preconditions.checkNotNull; + +/** + * An event fired when an operation is removed from the palette. + */ +public class OperationRemovedEvent { + + private final OperationDescription removedOperation; + + public OperationRemovedEvent(OperationDescription removedOperation) { + this.removedOperation = checkNotNull(removedOperation); + } + + public OperationDescription getRemovedOperation() { + return removedOperation; + } +} diff --git a/ui/src/main/java/edu/wpi/grip/ui/CustomOperationsListController.java b/ui/src/main/java/edu/wpi/grip/ui/CustomOperationsListController.java index 7092de8fbe..fcb854b4b3 100644 --- a/ui/src/main/java/edu/wpi/grip/ui/CustomOperationsListController.java +++ b/ui/src/main/java/edu/wpi/grip/ui/CustomOperationsListController.java @@ -2,32 +2,23 @@ import edu.wpi.grip.core.OperationMetaData; import edu.wpi.grip.core.events.OperationAddedEvent; -import edu.wpi.grip.core.operations.python.PythonOperationUtils; import edu.wpi.grip.core.operations.python.PythonScriptFile; import edu.wpi.grip.core.operations.python.PythonScriptOperation; import edu.wpi.grip.core.sockets.InputSocket; import edu.wpi.grip.core.sockets.OutputSocket; import edu.wpi.grip.ui.annotations.ParametrizedController; +import edu.wpi.grip.ui.python.PythonEditorController; import com.google.common.eventbus.EventBus; -import com.google.common.io.Files; import com.google.inject.Inject; -import org.python.core.PyException; - -import java.io.File; import java.io.IOException; -import java.nio.charset.Charset; -import java.util.Optional; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import javafx.fxml.FXML; -import javafx.scene.control.Alert; -import javafx.scene.control.ButtonType; -import javafx.scene.control.Dialog; -import javafx.scene.control.TextArea; -import javafx.scene.control.TextInputDialog; +import javafx.fxml.FXMLLoader; +import javafx.scene.Scene; +import javafx.scene.control.Button; +import javafx.stage.Stage; /** * Controller for the custom operations list. @@ -35,81 +26,43 @@ @ParametrizedController(url = "CustomOperationsList.fxml") public class CustomOperationsListController extends OperationListController { + @FXML private Button createNewButton; @Inject private EventBus eventBus; @Inject private InputSocket.Factory isf; @Inject private OutputSocket.Factory osf; - private final Pattern namePattern = Pattern.compile("name *= *\"(.*)\" *"); @FXML @SuppressWarnings("PMD.UnusedPrivateMethod") private void createNewPythonOperation() { - Dialog dialog = new TextInputDialog(); - dialog.getDialogPane().setContent(new TextArea(PythonScriptFile.TEMPLATE)); - dialog.setResultConverter(bt -> { - if (bt == ButtonType.OK) { - return ((TextArea) dialog.getDialogPane().getContent()).getText(); - } - return null; - }); - Optional result = dialog.showAndWait(); - if (result.isPresent()) { - String code = result.get(); - String[] lines = code.split("\n"); - String name = null; - // Find the name in the user code - for (String line : lines) { - Matcher m = namePattern.matcher(line); - if (m.matches()) { - name = m.group(1); - break; - } - } - if (name == null) { - Alert a = new Alert(Alert.AlertType.ERROR); - a.setContentText("A name must be specified"); - a.showAndWait(); - return; - } else if (name.isEmpty() || name.matches("[ \t]+")) { - // Empty names are not allowed - Alert a = new Alert(Alert.AlertType.ERROR); - a.setContentText("Name cannot be empty"); - a.showAndWait(); - return; - } else if (!name.matches("[a-zA-Z0-9_\\- ]+")) { - // Name can only contain (English) letters, numbers, underscores, dashes, and spaces - Alert a = new Alert(Alert.AlertType.ERROR); - a.setContentText("Name contains illegal characters"); - a.showAndWait(); - return; - } - File file = new File(PythonOperationUtils.DIRECTORY, name.replaceAll("[\\s]+", "_") + ".py"); - if (file.exists()) { - Alert a = new Alert(Alert.AlertType.ERROR); - a.setContentText("A file for the custom operation \"" + name + "\" already exists"); - a.showAndWait(); - return; - } - try { - Files.write(code, file, Charset.defaultCharset()); - } catch (IOException e) { - Alert a = new Alert(Alert.AlertType.ERROR); - a.setContentText("Could not save custom operation to " + file.getAbsolutePath()); - a.showAndWait(); - return; - } - try { + PythonEditorController editorController = loadEditor(); + Stage stage = new Stage(); + stage.setTitle("Python Script Editor"); + stage.setScene(new Scene(editorController.getRoot())); + createNewButton.setDisable(true); + try { + stage.showAndWait(); + String code = editorController.getScript(); + if (code != null) { PythonScriptFile script = PythonScriptFile.create(code); eventBus.post(new OperationAddedEvent(new OperationMetaData( PythonScriptOperation.descriptionFor(script), () -> new PythonScriptOperation(isf, osf, script) ))); - } catch (PyException e) { - // Malformed script, alert the user - Alert a = new Alert(Alert.AlertType.ERROR); - a.setTitle("Error in python script"); - a.setContentText("Error message: " + e.value.__getitem__(0).toString()); // wow - a.showAndWait(); } + } finally { + // make sure the button is re-enabled even if an exception gets thrown + createNewButton.setDisable(false); + } + } + + private PythonEditorController loadEditor() { + FXMLLoader loader = new FXMLLoader(); + loader.setLocation(getClass().getResource("/edu/wpi/grip/ui/python/PythonEditor.fxml")); + try { + loader.load(); + return loader.getController(); + } catch (IOException e) { + throw new RuntimeException("Couldn't load python editor", e); } } diff --git a/ui/src/main/java/edu/wpi/grip/ui/OperationListController.java b/ui/src/main/java/edu/wpi/grip/ui/OperationListController.java index 6e56d531fa..45c7a2dac9 100644 --- a/ui/src/main/java/edu/wpi/grip/ui/OperationListController.java +++ b/ui/src/main/java/edu/wpi/grip/ui/OperationListController.java @@ -1,7 +1,9 @@ package edu.wpi.grip.ui; +import edu.wpi.grip.core.OperationDescription; import edu.wpi.grip.core.OperationMetaData; import edu.wpi.grip.core.events.OperationAddedEvent; +import edu.wpi.grip.core.events.OperationRemovedEvent; import edu.wpi.grip.ui.annotations.ParametrizedController; import edu.wpi.grip.ui.util.ControllerMap; import edu.wpi.grip.ui.util.SearchUtility; @@ -91,6 +93,18 @@ public void onOperationAdded(OperationAddedEvent event) { } } + @Subscribe + public void onOperationRemoved(OperationRemovedEvent event) { + OperationDescription removedOperation = event.getRemovedOperation(); + if (root.getUserData() == null || removedOperation.category() == root.getUserData()) { + PlatformImpl.runAndWait(() -> operationsMapManager.remove(operationsMapManager.keySet() + .stream() + .filter(c -> c.getOperationDescription().equals(removedOperation)) + .findFirst() + .orElse(null))); + } + } + /** * Remove all operations. Used for tests. */ diff --git a/ui/src/main/java/edu/wpi/grip/ui/components/PreviousNextButtons.java b/ui/src/main/java/edu/wpi/grip/ui/components/PreviousNextButtons.java index 23a3c170dc..d1cd86fbca 100644 --- a/ui/src/main/java/edu/wpi/grip/ui/components/PreviousNextButtons.java +++ b/ui/src/main/java/edu/wpi/grip/ui/components/PreviousNextButtons.java @@ -6,6 +6,7 @@ import org.controlsfx.control.SegmentedButton; import java.util.function.Consumer; + import javafx.scene.Node; import javafx.scene.control.ToggleButton; import javafx.scene.control.Tooltip; diff --git a/ui/src/main/java/edu/wpi/grip/ui/preview/LinesSocketPreviewView.java b/ui/src/main/java/edu/wpi/grip/ui/preview/LinesSocketPreviewView.java index c79247d03c..3376f58621 100644 --- a/ui/src/main/java/edu/wpi/grip/ui/preview/LinesSocketPreviewView.java +++ b/ui/src/main/java/edu/wpi/grip/ui/preview/LinesSocketPreviewView.java @@ -9,6 +9,7 @@ import com.google.common.eventbus.Subscribe; import java.util.List; + import javafx.application.Platform; import javafx.geometry.Orientation; import javafx.scene.control.CheckBox; diff --git a/ui/src/main/java/edu/wpi/grip/ui/python/PythonEditorController.java b/ui/src/main/java/edu/wpi/grip/ui/python/PythonEditorController.java new file mode 100644 index 0000000000..f3c59068f8 --- /dev/null +++ b/ui/src/main/java/edu/wpi/grip/ui/python/PythonEditorController.java @@ -0,0 +1,242 @@ +package edu.wpi.grip.ui.python; + +import edu.wpi.grip.core.operations.python.PythonOperationUtils; +import edu.wpi.grip.core.operations.python.PythonScriptFile; +import edu.wpi.grip.ui.annotations.ParametrizedController; + +import org.fxmisc.richtext.CodeArea; +import org.fxmisc.richtext.LineNumberFactory; +import org.fxmisc.richtext.model.StyleSpans; +import org.fxmisc.richtext.model.StyleSpansBuilder; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.Charset; +import java.nio.file.Files; +import java.util.Collection; +import java.util.Collections; +import java.util.Locale; +import java.util.logging.Level; +import java.util.logging.Logger; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import javafx.fxml.FXML; +import javafx.scene.control.Alert; +import javafx.scene.layout.BorderPane; +import javafx.stage.FileChooser; +import javafx.stage.Window; +import javafx.stage.WindowEvent; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +/** + * Controller for the python editor. + */ +@ParametrizedController(url = "PythonEditor.fxml") +public class PythonEditorController { + + private static final Logger logger = Logger.getLogger(PythonEditorController.class.getName()); + + private Window window; + @FXML private BorderPane root; + private final CodeArea codeArea = new CodeArea(); + private File scriptFile = null; + + /** + * Array of python keywords. + */ + private static final String[] KEYWORDS = new String[] { + "and", "as", "assert", "break", "class", "continue", + "def", "del", "elif", "else", "except", "exec", + "finally", "for", "from", "global", "if", "import", + "in", "is", "lambda", "not", "or", "pass", "print", + "raise", "return", "try", "while", "with", "yield" + }; + + private static final String KEYWORD_PATTERN = "\\b(" + String.join("|", KEYWORDS) + ")\\b"; + private static final String PAREN_PATTERN = "\\(|\\)"; + private static final String BRACE_PATTERN = "\\{|\\}"; + private static final String BRACKET_PATTERN = "\\[|\\]"; + private static final String STRING_PATTERN = "\"([^\"\\\\]|\\\\.)*\""; + private static final String COMMENT_PATTERN = "#[^\n]*|'''(.|\\R)*?'''"; + + /** + * Regular expression pattern matching text to be styled differently. + */ + private static final Pattern PATTERN = Pattern.compile( + "(?" + KEYWORD_PATTERN + ")" + + "|(?" + PAREN_PATTERN + ")" + + "|(?" + BRACE_PATTERN + ")" + + "|(?" + BRACKET_PATTERN + ")" + + "|(?" + STRING_PATTERN + ")" + + "|(?" + COMMENT_PATTERN + ")" + ); + + @FXML + private void initialize() { + codeArea.setParagraphGraphicFactory(LineNumberFactory.get(codeArea)); + codeArea.richChanges() + .filter(ch -> !ch.getInserted().equals(ch.getRemoved())) + .subscribe(change -> codeArea.setStyleSpans(0, computeHighlighting(codeArea.getText()))); + codeArea.replaceText(0, 0, PythonScriptFile.TEMPLATE); + codeArea.setStyle("-fx-font-family: 'Consolas'; -fx-font-size: 10pt;"); + codeArea.getStylesheets() + .add(getClass().getResource("python-keywords.css").toExternalForm()); + root.setCenter(codeArea); + } + + /** + * Computes highlighting for python code. + * + * @param text the text to highlight + */ + private static StyleSpans> computeHighlighting(String text) { + Matcher matcher = PATTERN.matcher(text); + int lastKwEnd = 0; + StyleSpansBuilder> spansBuilder = new StyleSpansBuilder<>(); + while (matcher.find()) { + String styleClass = + matcher.group("KEYWORD") != null ? "keyword" : + matcher.group("PAREN") != null ? "paren" : + matcher.group("BRACE") != null ? "brace" : + matcher.group("BRACKET") != null ? "bracket" : + matcher.group("STRING") != null ? "string" : + matcher.group("COMMENT") != null ? "comment" : + null; /* never happens */ + assert styleClass != null; + spansBuilder.add(Collections.emptyList(), matcher.start() - lastKwEnd); + spansBuilder.add(Collections.singleton(styleClass), matcher.end() - matcher.start()); + lastKwEnd = matcher.end(); + } + spansBuilder.add(Collections.emptyList(), text.length() - lastKwEnd); + return spansBuilder.create(); + } + + /** + * Gets the window displaying the editor. + */ + private Window getWindow() { + if (window == null) { + window = root.getScene().getWindow(); + } + return window; + } + + /** + * Gets the root {@code BorderPane} for the editor. + */ + public @Nonnull BorderPane getRoot() { + return root; + } + + /** + * Gets the script text in the editor. + */ + public @Nullable String getScript() { + if (scriptFileName() == null) { + return null; + } else { + return codeArea.getText(); + } + } + + /** + * Creates the name of the file to save the script to based on the operation name. + */ + private @Nullable String scriptFileName() { + final Pattern namePattern = Pattern.compile("name *= *\"(.*)\" *"); + String code = codeArea.getText(); + String[] lines = code.split("\n"); + for (String line : lines) { + Matcher m = namePattern.matcher(line); + if (m.matches()) { + return m.group(1).replaceAll("[ \\t]+", "_").toLowerCase(Locale.ENGLISH) + ".py"; + } + } + return null; + } + + @FXML + private void save() { + if (scriptFile == null) { + String fileName = scriptFileName(); + if (fileName == null) { + // No 'name' property + Alert noNameAlert = new Alert(Alert.AlertType.ERROR); + noNameAlert.setTitle("No operation name"); + noNameAlert.setContentText("This operation needs a name!"); + noNameAlert.showAndWait(); + return; + } + scriptFile = new File(PythonOperationUtils.DIRECTORY, fileName); + } + try { + Files.write( + scriptFile.getAbsoluteFile().toPath(), + codeArea.getText().getBytes(Charset.defaultCharset()) + ); + } catch (IOException e) { + logger.log(Level.WARNING, "Could not save to " + scriptFile, e); + } + } + + @FXML + private void saveAs() { + FileChooser chooser = new FileChooser(); + chooser.setInitialDirectory(PythonOperationUtils.DIRECTORY); + chooser.setTitle("Choose "); + chooser.setSelectedExtensionFilter( + new FileChooser.ExtensionFilter("Custom GRIP operations", "*.py")); + String fileName = scriptFileName(); + if (fileName != null) { + chooser.setInitialFileName(fileName); + } + File file = chooser.showSaveDialog(getWindow()); + if (file == null) { + // Chooser was closed; no file selected + return; + } + scriptFile = file; + save(); + } + + @FXML + private void saveAndExit() { + save(); + exit(); + } + + @FXML + private void exit() { + getWindow().fireEvent(new WindowEvent(getWindow(), WindowEvent.WINDOW_CLOSE_REQUEST)); + } + + @FXML + private void openFile() { + FileChooser chooser = new FileChooser(); + chooser.setInitialDirectory(PythonOperationUtils.DIRECTORY); + chooser.setTitle("Choose an operation to edit"); + chooser.setSelectedExtensionFilter( + new FileChooser.ExtensionFilter("Custom GRIP operations", "*.py")); + File file = chooser.showOpenDialog(getWindow()); + if (file == null) { + // Chooser was closed; no file selected + return; + } + try { + byte[] bytes = Files.readAllBytes(file.getAbsoluteFile().toPath()); + String code = new String(bytes, Charset.defaultCharset()); + codeArea.replaceText(code); + } catch (IOException e) { + logger.log(Level.WARNING, "Could not read file " + file, e); + } + } + + @FXML + private void openWiki() { + // TODO + } + +} diff --git a/ui/src/main/resources/edu/wpi/grip/ui/CustomOperationsList.fxml b/ui/src/main/resources/edu/wpi/grip/ui/CustomOperationsList.fxml index 6e3cf63e4d..c6354c9209 100644 --- a/ui/src/main/resources/edu/wpi/grip/ui/CustomOperationsList.fxml +++ b/ui/src/main/resources/edu/wpi/grip/ui/CustomOperationsList.fxml @@ -11,7 +11,7 @@ - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/ui/src/main/resources/edu/wpi/grip/ui/python/python-keywords.css b/ui/src/main/resources/edu/wpi/grip/ui/python/python-keywords.css new file mode 100644 index 0000000000..1f9374bec0 --- /dev/null +++ b/ui/src/main/resources/edu/wpi/grip/ui/python/python-keywords.css @@ -0,0 +1,31 @@ +.keyword { + -fx-fill: purple; + -fx-font-weight: bold; +} + +.paren { + -fx-fill: firebrick; + -fx-font-weight: bold; +} + +.bracket { + -fx-fill: darkgreen; + -fx-font-weight: bold; +} + +.brace { + -fx-fill: teal; + -fx-font-weight: bold; +} + +.string { + -fx-fill: #669900; +} + +.comment { + -fx-fill: darkgray; +} + +.paragraph-box:has-caret { + -fx-background-color: #f2f9fc; +} \ No newline at end of file From cb063fe8d7e7795c0a4634ba8a596bc121171685 Mon Sep 17 00:00:00 2001 From: Sam Carlberg Date: Thu, 28 Jul 2016 22:16:28 -0400 Subject: [PATCH 7/8] Alert the user if a script has an error or can't be saved/loaded --- .../python/PythonOperationUtils.java | 1 - .../ui/python/PythonEditorController.java | 46 +++++++++++++------ 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/edu/wpi/grip/core/operations/python/PythonOperationUtils.java b/core/src/main/java/edu/wpi/grip/core/operations/python/PythonOperationUtils.java index 94ee10bfd2..0c1f2a9a2f 100644 --- a/core/src/main/java/edu/wpi/grip/core/operations/python/PythonOperationUtils.java +++ b/core/src/main/java/edu/wpi/grip/core/operations/python/PythonOperationUtils.java @@ -69,7 +69,6 @@ public static PythonScriptFile tryCreate(String code) { return PythonScriptFile.create(code); } catch (PyException e) { log.log(Level.WARNING, "Error in python script", e); - log.log(Level.WARNING, "Erroneous code:\n" + code); return null; } } diff --git a/ui/src/main/java/edu/wpi/grip/ui/python/PythonEditorController.java b/ui/src/main/java/edu/wpi/grip/ui/python/PythonEditorController.java index f3c59068f8..60a1e1fbb6 100644 --- a/ui/src/main/java/edu/wpi/grip/ui/python/PythonEditorController.java +++ b/ui/src/main/java/edu/wpi/grip/ui/python/PythonEditorController.java @@ -8,6 +8,7 @@ import org.fxmisc.richtext.LineNumberFactory; import org.fxmisc.richtext.model.StyleSpans; import org.fxmisc.richtext.model.StyleSpansBuilder; +import org.python.core.PyException; import java.io.File; import java.io.IOException; @@ -23,6 +24,7 @@ import javafx.fxml.FXML; import javafx.scene.control.Alert; +import javafx.scene.control.Label; import javafx.scene.layout.BorderPane; import javafx.stage.FileChooser; import javafx.stage.Window; @@ -135,10 +137,12 @@ private Window getWindow() { * Gets the script text in the editor. */ public @Nullable String getScript() { - if (scriptFileName() == null) { + String script = codeArea.getText(); + try { + PythonScriptFile.create(script); + return script; + } catch (PyException e) { return null; - } else { - return codeArea.getText(); } } @@ -159,16 +163,18 @@ private Window getWindow() { } @FXML - private void save() { + private boolean save() { if (scriptFile == null) { String fileName = scriptFileName(); - if (fileName == null) { - // No 'name' property - Alert noNameAlert = new Alert(Alert.AlertType.ERROR); - noNameAlert.setTitle("No operation name"); - noNameAlert.setContentText("This operation needs a name!"); - noNameAlert.showAndWait(); - return; + try { + PythonScriptFile.create(codeArea.getText()); + } catch (PyException e) { + Alert malformed = new Alert(Alert.AlertType.ERROR); + malformed.setTitle("Error in script"); + malformed.setHeaderText("There is an error in the python script"); + malformed.getDialogPane().setContent(new Label(e.toString())); + malformed.showAndWait(); + return false; } scriptFile = new File(PythonOperationUtils.DIRECTORY, fileName); } @@ -177,8 +183,14 @@ private void save() { scriptFile.getAbsoluteFile().toPath(), codeArea.getText().getBytes(Charset.defaultCharset()) ); + return true; } catch (IOException e) { logger.log(Level.WARNING, "Could not save to " + scriptFile, e); + Alert couldNotSave = new Alert(Alert.AlertType.ERROR); + couldNotSave.setTitle("Could not save custom operation"); + couldNotSave.setContentText("Could not save to file: " + scriptFile); + couldNotSave.showAndWait(); + return false; } } @@ -186,7 +198,7 @@ private void save() { private void saveAs() { FileChooser chooser = new FileChooser(); chooser.setInitialDirectory(PythonOperationUtils.DIRECTORY); - chooser.setTitle("Choose "); + chooser.setTitle("Choose save file"); chooser.setSelectedExtensionFilter( new FileChooser.ExtensionFilter("Custom GRIP operations", "*.py")); String fileName = scriptFileName(); @@ -204,8 +216,10 @@ private void saveAs() { @FXML private void saveAndExit() { - save(); - exit(); + if (save() && getScript() != null) { + // Don't exit if there's a problem with the script + exit(); + } } @FXML @@ -231,6 +245,10 @@ private void openFile() { codeArea.replaceText(code); } catch (IOException e) { logger.log(Level.WARNING, "Could not read file " + file, e); + Alert couldNotRead = new Alert(Alert.AlertType.ERROR); + couldNotRead.setTitle("Could not read custom operation"); + couldNotRead.setContentText("Could not read from file: " + file); + couldNotRead.showAndWait(); } } From e5fb209138e7516e410a8f6f2844a5caa91443b1 Mon Sep 17 00:00:00 2001 From: Sam Carlberg Date: Mon, 1 Aug 2016 20:27:28 -0400 Subject: [PATCH 8/8] Improve error alerts, add some tests, make wiki button work. Also some minor fixup from PR --- .../main/java/edu/wpi/grip/core/Palette.java | 7 +- .../operations/python/PythonScriptFile.java | 3 - .../java/edu/wpi/grip/core/PaletteTest.java | 44 ++++++- .../python/PythonOperationUtilsTest.java | 47 +++++++ .../ui/CustomOperationsListController.java | 13 ++ .../wpi/grip/ui/OperationListController.java | 2 +- .../ui/python/PythonEditorController.java | 124 ++++++++++++++++-- .../edu/wpi/grip/ui/python/PythonEditor.fxml | 18 +-- .../wpi/grip/ui/python/python-keywords.css | 2 +- .../java/edu/wpi/grip/ui/PaletteTest.java | 6 +- 10 files changed, 230 insertions(+), 36 deletions(-) create mode 100644 core/src/test/java/edu/wpi/grip/core/operations/python/PythonOperationUtilsTest.java diff --git a/core/src/main/java/edu/wpi/grip/core/Palette.java b/core/src/main/java/edu/wpi/grip/core/Palette.java index ffc44a714f..cb4d2e3d5c 100644 --- a/core/src/main/java/edu/wpi/grip/core/Palette.java +++ b/core/src/main/java/edu/wpi/grip/core/Palette.java @@ -22,9 +22,14 @@ @Singleton public class Palette { - @Inject private EventBus eventBus; + private final EventBus eventBus; private final Map operations = new LinkedHashMap<>(); + @Inject + Palette(EventBus eventBus) { + this.eventBus = eventBus; + } + @Subscribe public void onOperationAdded(OperationAddedEvent event) { final OperationMetaData operation = event.getOperation(); diff --git a/core/src/main/java/edu/wpi/grip/core/operations/python/PythonScriptFile.java b/core/src/main/java/edu/wpi/grip/core/operations/python/PythonScriptFile.java index 3b904e6a9e..d7cba5e461 100644 --- a/core/src/main/java/edu/wpi/grip/core/operations/python/PythonScriptFile.java +++ b/core/src/main/java/edu/wpi/grip/core/operations/python/PythonScriptFile.java @@ -19,7 +19,6 @@ import java.net.URL; import java.util.List; import java.util.Properties; -import java.util.logging.Logger; /** * Converts a string of Python Code or a Python File into something the {@link @@ -28,8 +27,6 @@ @AutoValue public abstract class PythonScriptFile { - private static final Logger logger = Logger.getLogger(PythonScriptFile.class.getName()); - /** * Template for custom python operations. Includes imports for sockets, as well as OpenCV * core and image processing classes. diff --git a/core/src/test/java/edu/wpi/grip/core/PaletteTest.java b/core/src/test/java/edu/wpi/grip/core/PaletteTest.java index caf7ef5f66..a38ad13897 100644 --- a/core/src/test/java/edu/wpi/grip/core/PaletteTest.java +++ b/core/src/test/java/edu/wpi/grip/core/PaletteTest.java @@ -12,6 +12,8 @@ import java.util.Optional; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; public class PaletteTest { private Palette palette; @@ -21,13 +23,13 @@ public class PaletteTest { @Before public void setUp() { eventBus = new EventBus(); - palette = new Palette(); + palette = new Palette(eventBus); eventBus.register(palette); operation = new OperationMetaData(OperationDescription.builder() .name("Find Target") .summary("") .build(), - () -> null); + MockOperation::new); } @Test @@ -47,4 +49,42 @@ public void testGetNonexistantOperation() { eventBus.post(new OperationAddedEvent(operation)); assertEquals(Optional.empty(), palette.getOperationByName("Test")); } + + @Test + public void testReplacedOperation() { + // Only a custom operation may be replaced, and only by another custom operation + // with the same name + final String name = "Custom Operation"; + OperationMetaData first = new OperationMetaData(OperationDescription.builder() + .name(name) + .summary("A summary") + .category(OperationDescription.Category.CUSTOM) + .build(), + MockOperation::new); + OperationMetaData second = new OperationMetaData(OperationDescription.builder() + .name(name) + .summary("Another summary") + .category(OperationDescription.Category.CUSTOM) + .build(), + MockOperation::new); + eventBus.post(new OperationAddedEvent(first)); + assertTrue(palette.getOperations().contains(first)); + assertFalse(palette.getOperations().contains(second)); + eventBus.post(new OperationAddedEvent(second)); + assertFalse(palette.getOperations().contains(first)); + assertTrue(palette.getOperations().contains(second)); + } + + @Test(expected = IllegalArgumentException.class) + public void testAddOperationWithSameName() { + OperationMetaData custom = new OperationMetaData(OperationDescription.builder() + .name(operation.getDescription().name()) + .summary("Custom operation with the name of another operation in the palette") + .category(OperationDescription.Category.CUSTOM) + .build(), + MockOperation::new); + palette.onOperationAdded(new OperationAddedEvent(operation)); // EventBus messes with exceptions + palette.onOperationAdded(new OperationAddedEvent(custom)); + } + } diff --git a/core/src/test/java/edu/wpi/grip/core/operations/python/PythonOperationUtilsTest.java b/core/src/test/java/edu/wpi/grip/core/operations/python/PythonOperationUtilsTest.java new file mode 100644 index 0000000000..185d7f62c1 --- /dev/null +++ b/core/src/test/java/edu/wpi/grip/core/operations/python/PythonOperationUtilsTest.java @@ -0,0 +1,47 @@ +package edu.wpi.grip.core.operations.python; + +import org.junit.Test; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; + +import static edu.wpi.grip.core.operations.python.PythonOperationUtils.DIRECTORY; +import static edu.wpi.grip.core.operations.python.PythonOperationUtils.checkDirExists; +import static edu.wpi.grip.core.operations.python.PythonOperationUtils.read; +import static edu.wpi.grip.core.operations.python.PythonOperationUtils.tryCreate; +import static edu.wpi.grip.core.operations.python.PythonScriptFile.TEMPLATE; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +public class PythonOperationUtilsTest { + + @Test(expected = IllegalArgumentException.class) + public void testNotCorrectFile() { + PythonOperationUtils.read(new File(System.getProperty("user.home"))); + } + + @Test(expected = IllegalArgumentException.class) + public void testNotPythonFile() { + PythonOperationUtils.read(new File(DIRECTORY, "not-a-python-file.txt")); + } + + @Test + public void testReadFile() throws IOException { + File file = new File(DIRECTORY, "a-python-file.py"); + file.deleteOnExit(); + checkDirExists(); + file.createNewFile(); + Files.write(file.toPath(), TEMPLATE.getBytes()); + String read = read(file); + assertEquals(TEMPLATE, read); + } + + @Test + public void testTryCreate() { + assertNotNull(tryCreate(TEMPLATE)); + assertNull(tryCreate("not executable python code")); + } + +} diff --git a/ui/src/main/java/edu/wpi/grip/ui/CustomOperationsListController.java b/ui/src/main/java/edu/wpi/grip/ui/CustomOperationsListController.java index fcb854b4b3..51044b7a60 100644 --- a/ui/src/main/java/edu/wpi/grip/ui/CustomOperationsListController.java +++ b/ui/src/main/java/edu/wpi/grip/ui/CustomOperationsListController.java @@ -1,6 +1,8 @@ package edu.wpi.grip.ui; import edu.wpi.grip.core.OperationMetaData; +import edu.wpi.grip.core.Palette; +import edu.wpi.grip.core.Pipeline; import edu.wpi.grip.core.events.OperationAddedEvent; import edu.wpi.grip.core.operations.python.PythonScriptFile; import edu.wpi.grip.core.operations.python.PythonScriptOperation; @@ -30,11 +32,22 @@ public class CustomOperationsListController extends OperationListController { @Inject private EventBus eventBus; @Inject private InputSocket.Factory isf; @Inject private OutputSocket.Factory osf; + @Inject private Palette palette; + @Inject private Main main; + @Inject private Pipeline pipeline; @FXML @SuppressWarnings("PMD.UnusedPrivateMethod") private void createNewPythonOperation() { PythonEditorController editorController = loadEditor(); + editorController.injectMembers( + name -> palette.getOperationByName(name).isPresent() + || pipeline.getSteps() + .stream() + .map(step -> step.getOperationDescription().name()) + .anyMatch(name::equals), + main.getHostServices() + ); Stage stage = new Stage(); stage.setTitle("Python Script Editor"); stage.setScene(new Scene(editorController.getRoot())); diff --git a/ui/src/main/java/edu/wpi/grip/ui/OperationListController.java b/ui/src/main/java/edu/wpi/grip/ui/OperationListController.java index 45c7a2dac9..87db1f7c4b 100644 --- a/ui/src/main/java/edu/wpi/grip/ui/OperationListController.java +++ b/ui/src/main/java/edu/wpi/grip/ui/OperationListController.java @@ -62,7 +62,7 @@ protected void initialize() { if (!filter.isEmpty() && numMatches > 0) { // If we're filtering some operations and there's at least one match, set the title to - // bold and show the umber of matches. + // bold and show the number of matches. // This lets the user quickly see which tabs have matching operations when searching. root.setText(baseText + " (" + numMatches + ")"); root.styleProperty().setValue("-fx-font-weight: bold"); diff --git a/ui/src/main/java/edu/wpi/grip/ui/python/PythonEditorController.java b/ui/src/main/java/edu/wpi/grip/ui/python/PythonEditorController.java index 60a1e1fbb6..cdd7ac8f43 100644 --- a/ui/src/main/java/edu/wpi/grip/ui/python/PythonEditorController.java +++ b/ui/src/main/java/edu/wpi/grip/ui/python/PythonEditorController.java @@ -17,11 +17,14 @@ import java.util.Collection; import java.util.Collections; import java.util.Locale; +import java.util.function.Predicate; import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Stream; +import javafx.application.HostServices; import javafx.fxml.FXML; import javafx.scene.control.Alert; import javafx.scene.control.Label; @@ -33,6 +36,8 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +import static com.google.common.base.Preconditions.checkNotNull; + /** * Controller for the python editor. */ @@ -45,6 +50,8 @@ public class PythonEditorController { @FXML private BorderPane root; private final CodeArea codeArea = new CodeArea(); private File scriptFile = null; + private Predicate operationNameTaken; + private HostServices hostServices; /** * Array of python keywords. @@ -116,6 +123,17 @@ private static StyleSpans> computeHighlighting(String text) { return spansBuilder.create(); } + /** + * Injects members required for this controller. + * + * @param operationNameTaken predicate testing if an operation with a given name is in the palette + * or pipeline + */ + public void injectMembers(Predicate operationNameTaken, HostServices hostServices) { + this.operationNameTaken = checkNotNull(operationNameTaken, "operationNameTaken"); + this.hostServices = checkNotNull(hostServices, "hostServices"); + } + /** * Gets the window displaying the editor. */ @@ -134,9 +152,13 @@ private Window getWindow() { } /** - * Gets the script text in the editor. + * Gets the script text in the editor. Returns null if no name is present, the name is already + * used for another operation, or if there is an error in the script. */ public @Nullable String getScript() { + if (checkName()) { + return null; + } String script = codeArea.getText(); try { PythonScriptFile.create(script); @@ -147,33 +169,99 @@ private Window getWindow() { } /** - * Creates the name of the file to save the script to based on the operation name. + * Extracts the name of the operation from the script. Returns null if no name is present. */ - private @Nullable String scriptFileName() { + private @Nullable String extractName() { final Pattern namePattern = Pattern.compile("name *= *\"(.*)\" *"); + String name = null; String code = codeArea.getText(); String[] lines = code.split("\n"); for (String line : lines) { Matcher m = namePattern.matcher(line); if (m.matches()) { - return m.group(1).replaceAll("[ \\t]+", "_").toLowerCase(Locale.ENGLISH) + ".py"; + name = m.group(1).trim(); + break; } } - return null; + return name; } + /** + * Checks if the operation name is present and not used by any other operation in the palette + * or pipeline. This will show an alert dialog notifying the user if either does not hold. + */ + private boolean checkName() { + String name = extractName(); + if (name == null || name.isEmpty()) { + Alert noName = new Alert(Alert.AlertType.ERROR); + noName.setTitle("No Name"); + noName.setHeaderText("This operation needs a name!"); + noName.showAndWait(); + } + if (operationNameTaken.test(name)) { + Alert nameInUseAlert = new Alert(Alert.AlertType.ERROR); + nameInUseAlert.setTitle("Already in Use"); + nameInUseAlert.setHeaderText("An operation already exists with the name '" + name + "'"); + nameInUseAlert.setContentText("You won't be able to save this script until you change the " + + "name to something that doesn't belong to another operation."); + nameInUseAlert.showAndWait(); + return true; + } + return false; + } + + /** + * Creates the name of the file to save the script to based on the operation name. + * Returns null if the script has an error, or if the assigned name is already in use. + */ + @SuppressWarnings("PMD") + private @Nullable String scriptFileName() { + String name = extractName(); + if (name != null) { + name = name.trim().replaceAll("[ \\t]+", "_").toLowerCase(Locale.ENGLISH); + final String regex = name + "_?([0-9]*?)\\.py"; // e.g. $name.py, $name_2.py, etc. + boolean needsNumber = Stream.of(PythonOperationUtils.DIRECTORY.list()) + .filter(n -> n.matches(regex)) + .count() > 0; + if (needsNumber) { + int currentMax = Stream.of(PythonOperationUtils.DIRECTORY.list()) + .filter(n -> n.matches(regex)) + .map(n -> n.replaceAll(regex, "$1")) + .mapToInt(n -> n.isEmpty() ? 0 : Integer.valueOf(n)) + .max() + .orElse(1); + name = name + '_' + (currentMax + 1); + } + return name.concat(".py"); + } else { + return null; + } + } + + private void showScriptErrorAlert(String message) { + Alert malformed = new Alert(Alert.AlertType.ERROR); + malformed.setTitle("Error in script"); + malformed.setHeaderText("There is an error in the python script"); + malformed.getDialogPane().setContent(new Label(message)); + malformed.showAndWait(); + } + + /** + * Tries to save the script to disk. Does not save if the script has an error. + * + * @return true if the script was saved, false otherwise + */ @FXML private boolean save() { + if (checkName()) { + return false; + } if (scriptFile == null) { String fileName = scriptFileName(); try { PythonScriptFile.create(codeArea.getText()); } catch (PyException e) { - Alert malformed = new Alert(Alert.AlertType.ERROR); - malformed.setTitle("Error in script"); - malformed.setHeaderText("There is an error in the python script"); - malformed.getDialogPane().setContent(new Label(e.toString())); - malformed.showAndWait(); + showScriptErrorAlert(e.toString()); return false; } scriptFile = new File(PythonOperationUtils.DIRECTORY, fileName); @@ -194,8 +282,22 @@ private boolean save() { } } + /** + * Tries to 'save-as' the script to disk. Does nothing if the script has an error. + */ @FXML private void saveAs() { + if (checkName()) { + // No name + return; + } + try { + PythonScriptFile.create(codeArea.getText()); + } catch (PyException e) { + // Error in script + showScriptErrorAlert(e.toString()); + return; + } FileChooser chooser = new FileChooser(); chooser.setInitialDirectory(PythonOperationUtils.DIRECTORY); chooser.setTitle("Choose save file"); @@ -254,7 +356,7 @@ private void openFile() { @FXML private void openWiki() { - // TODO + hostServices.showDocument("https://github.com/WPIRoboticsProjects/GRIP/wiki"); } } diff --git a/ui/src/main/resources/edu/wpi/grip/ui/python/PythonEditor.fxml b/ui/src/main/resources/edu/wpi/grip/ui/python/PythonEditor.fxml index f6ad0aa6aa..874b84b67d 100644 --- a/ui/src/main/resources/edu/wpi/grip/ui/python/PythonEditor.fxml +++ b/ui/src/main/resources/edu/wpi/grip/ui/python/PythonEditor.fxml @@ -34,21 +34,9 @@ - - - - - - - - - - - - - - - + + + diff --git a/ui/src/main/resources/edu/wpi/grip/ui/python/python-keywords.css b/ui/src/main/resources/edu/wpi/grip/ui/python/python-keywords.css index 1f9374bec0..a87ad9b92a 100644 --- a/ui/src/main/resources/edu/wpi/grip/ui/python/python-keywords.css +++ b/ui/src/main/resources/edu/wpi/grip/ui/python/python-keywords.css @@ -28,4 +28,4 @@ .paragraph-box:has-caret { -fx-background-color: #f2f9fc; -} \ No newline at end of file +} diff --git a/ui/src/test/java/edu/wpi/grip/ui/PaletteTest.java b/ui/src/test/java/edu/wpi/grip/ui/PaletteTest.java index 1ea7b749f1..f16379607e 100644 --- a/ui/src/test/java/edu/wpi/grip/ui/PaletteTest.java +++ b/ui/src/test/java/edu/wpi/grip/ui/PaletteTest.java @@ -6,6 +6,7 @@ import edu.wpi.grip.core.Step; import edu.wpi.grip.core.events.OperationAddedEvent; import edu.wpi.grip.core.events.StepAddedEvent; +import edu.wpi.grip.core.operations.network.MockGripNetworkModule; import edu.wpi.grip.core.sockets.InputSocket; import edu.wpi.grip.core.sockets.OutputSocket; import edu.wpi.grip.util.GripCoreTestModule; @@ -40,7 +41,8 @@ public class PaletteTest extends ApplicationTest { public void start(Stage stage) throws IOException { testModule.setUp(); - Injector injector = Guice.createInjector(Modules.override(testModule).with(new GripUiModule())); + Injector injector = Guice.createInjector( + Modules.override(testModule, new MockGripNetworkModule()).with(new GripUiModule())); eventBus = injector.getInstance(EventBus.class); FXMLLoader loader = new FXMLLoader(getClass().getResource("Palette.fxml")); @@ -62,7 +64,7 @@ public void testPalette() { operation))); // Record when a a StepAddedEvent happens - Step[] step = new Step[]{null}; + Step[] step = new Step[] {null}; eventBus.register(new Object() { @Subscribe public void onStepAdded(StepAddedEvent event) {