Bundle styling updates

That was actually the problem

Author: oowekyala

pmd-ui.iml

@@ -8,6 +8,11 @@
       </list>
     </option>
   </component>
+  <component name="CheckStyle-IDEA-Module">
+    <option name="configuration">
+      <map />
+    </option>
+  </component>
   <component name="FacetManager">
     <facet type="kotlin-language" name="Kotlin">
       <configuration version="3" platform="JVM 1.8" allPlatforms="JVM [1.8]" useProjectSettings="false">

src/main/java/net/sourceforge/pmd/util/fxdesigner/app/NodeSelectionSource.java

@@ -19,8 +19,6 @@
 import net.sourceforge.pmd.util.fxdesigner.util.controls.AstTreeView;
 import net.sourceforge.pmd.util.fxdesigner.util.reactfx.ReactfxUtil;
 
-import javafx.application.Platform;
-
 
 /**
  * A control or controller that somehow displays nodes in a form that the user can select.
@@ -64,7 +62,6 @@ public interface NodeSelectionSource extends ApplicationComponent {
      * @param mySelectionEvents     Stream of nodes that should push an event each time the user selects a node
      *                              from this control. The whole app will sync to this new selection.
      * @param alwaysHandleSelection Whether the component should handle selection events that originated from itself.
-     *
      * @return A Val reflecting the current global selection for the app.
      * Note that that Val is lazy and so if you don't subscribe to it or
      * {@linkplain Val#pin() pin it} you won't see updates!
@@ -77,7 +74,7 @@ default Val<Node> initNodeSelectionHandling(DesignerRoot root,
         EventStream<NodeSelectionEvent> selection = channel.messageStream(alwaysHandleSelection, this);
         selection.subscribe(evt -> {
             try {
-                Platform.runLater(() -> setFocusNode(evt.selected, evt.options));
+                setFocusNode(evt.selected, evt.options);
             } catch (Exception e) {
                 logInternalException(e);
                 printShortStackTrace(e);

src/main/java/net/sourceforge/pmd/util/fxdesigner/util/RichRunnable.java

@@ -0,0 +1,17 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.util.fxdesigner.util;
+
+@FunctionalInterface
+public interface RichRunnable extends Runnable {
+
+    default RichRunnable andThen(Runnable r) {
+        return () -> {
+            this.run();
+            r.run();
+        };
+    }
+
+}

src/main/java/net/sourceforge/pmd/util/fxdesigner/util/codearea/HighlightLayerCodeArea.java

@@ -15,11 +15,13 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 
+import org.apache.commons.lang3.StringUtils;
 import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.fxmisc.richtext.model.StyleSpans;
 
 import net.sourceforge.pmd.lang.ast.Node;
+import net.sourceforge.pmd.util.fxdesigner.util.RichRunnable;
 import net.sourceforge.pmd.util.fxdesigner.util.codearea.HighlightLayerCodeArea.LayerId;
 
 import javafx.application.Platform;
@@ -74,18 +76,22 @@ public HighlightLayerCodeArea(@NamedArg("idEnum") Class<K> idEnum) {
     // to highlight errors that are not bound to a node, eg parsing errors
     // We'll need to abstract away NodeStyleSpan
     public void styleNodes(Collection<? extends Node> nodes, K layerId, boolean resetLayer) {
+        updateStyling(styleNodesUpdate(nodes, layerId, resetLayer));
+    }
+
+
+    public RichRunnable styleNodesUpdate(Collection<? extends Node> nodes, K layerId, boolean resetLayer) {
         Objects.requireNonNull(nodes, "Pass an empty collection to represent absence, not null!");
 
         if (nodes.isEmpty() && resetLayer) {
-            clearStyleLayer(layerId);
-            return;
+            return layersById.get(layerId)::clearStyles;
         }
 
         List<NodeStyleSpan> wrappedNodes = nodes.stream().map(n -> NodeStyleSpan.fromNode(n, this)).collect(Collectors.toList());
 
         UniformStyleCollection collection = new UniformStyleCollection(Collections.singleton(layerId.getStyleClass()), wrappedNodes);
 
-        updateStyling(() -> layersById.get(layerId).styleNodes(resetLayer, collection));
+        return () -> layersById.get(layerId).styleNodes(resetLayer, collection);
     }
 
 
@@ -97,7 +103,7 @@ public void styleNodes(Collection<? extends Node> nodes, K layerId, boolean rese
      *
      * @param update Update to carry out
      */
-    private void updateStyling(Runnable update) {
+    public void updateStyling(Runnable update) {
         Platform.runLater(() -> {
             update.run();
             try {
@@ -107,7 +113,7 @@ private void updateStyling(Runnable update) {
                 // commonly thrown when the text is being edited while
                 // the layering algorithm runs, and it doesn't matter
                 if ("StyleSpan's length cannot be negative".equals(e.getMessage())
-                    || e.getMessage().contains("is not a valid range within")) {
+                    || StringUtils.contains(e.getMessage(), "is not a valid range within")) {
                     return;
                 }
                 throw new RuntimeException("Unhandled error while recomputing the styling", e);

src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/NodeEditionCodeArea.java

@@ -23,6 +23,7 @@
 import java.util.function.IntFunction;
 
 import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.dataflow.qual.Pure;
 import org.fxmisc.richtext.LineNumberFactory;
 import org.fxmisc.richtext.event.MouseOverTextEvent;
 import org.reactfx.EventSource;
@@ -46,6 +47,7 @@
 import net.sourceforge.pmd.util.fxdesigner.model.testing.LiveViolationRecord;
 import net.sourceforge.pmd.util.fxdesigner.util.DataHolder;
 import net.sourceforge.pmd.util.fxdesigner.util.DesignerUtil;
+import net.sourceforge.pmd.util.fxdesigner.util.RichRunnable;
 import net.sourceforge.pmd.util.fxdesigner.util.codearea.AvailableSyntaxHighlighters;
 import net.sourceforge.pmd.util.fxdesigner.util.codearea.HighlightLayerCodeArea;
 import net.sourceforge.pmd.util.fxdesigner.util.codearea.PmdCoordinatesSystem.TextPos2D;
@@ -113,9 +115,9 @@ public NodeEditionCodeArea(@NamedArg("designerRoot") DesignerRoot root) {
 
         setParagraphGraphicFactory(defaultLineNumberFactory());
 
-        currentRuleResultsProperty().values().subscribe(this::highlightXPathResults);
-        currentErrorNodesProperty().values().subscribe(this::highlightErrorNodes);
-        currentRelatedNodesProperty().values().subscribe(this::highlightRelatedNodes);
+        currentRuleResultsProperty().values().map(this::highlightXPathResults).subscribe(this::updateStyling);
+        currentErrorNodesProperty().values().map(this::highlightErrorNodes).subscribe(this::updateStyling);
+        currentRelatedNodesProperty().values().map(this::highlightRelatedNodes).subscribe(this::updateStyling);
 
         initNodeSelectionHandling(designerRoot, selectionEvts, true);
 
@@ -168,8 +170,8 @@ private void scrollToNode(Node node, boolean scrollToTop) {
         try {
             visibleLength = lastVisibleParToAllParIndex() - firstVisibleParToAllParIndex();
         } catch (AssertionError e) {
-            // may be thrown when many selection events occur in quick succession?
-            // Something like, the paragraphs
+            // May be thrown when many selection events occur in quick succession?
+            // The error is "dead code", probably a corner case in RichTextFX
             return;
         }
 
@@ -271,28 +273,40 @@ public Var<List<Node>> currentRelatedNodesProperty() {
     }
 
 
-    /** Highlights xpath results (xpath highlight). */
-    private void highlightXPathResults(Collection<? extends Node> nodes) {
-        styleNodes(nodes, StyleLayerIds.XPATH_RESULT, true);
+    /**
+     * Highlights xpath results (xpath highlight).
+     */
+    @Pure
+    private RichRunnable highlightXPathResults(Collection<? extends Node> nodes) {
+        return styleNodesUpdate(nodes, StyleLayerIds.XPATH_RESULT, true);
     }
 
 
-    /** Highlights name occurrences (secondary highlight). */
-    private void highlightRelatedNodes(Collection<? extends Node> occs) {
-        styleNodes(occs, StyleLayerIds.NAME_OCCURRENCE, true);
+    /**
+     * Highlights name occurrences (secondary highlight).
+     */
+    @Pure
+    private RichRunnable highlightRelatedNodes(Collection<? extends Node> occs) {
+        return styleNodesUpdate(occs, StyleLayerIds.NAME_OCCURRENCE, true);
     }
 
 
-    /** Highlights nodes that are in error (secondary highlight). */
-    private void highlightErrorNodes(Collection<? extends Node> nodes) {
-        styleNodes(nodes, StyleLayerIds.ERROR, true);
+    /**
+     * Highlights nodes that are in error (secondary highlight).
+     */
+    @Pure
+    private RichRunnable highlightErrorNodes(Collection<? extends Node> nodes) {
+        RichRunnable r = styleNodesUpdate(nodes, StyleLayerIds.ERROR, true);
         if (!nodes.isEmpty()) {
-            scrollToNode(nodes.iterator().next(), true);
+            r = r.andThen(() -> scrollToNode(nodes.iterator().next(), true));
         }
+        return r;
     }
 
 
-    /** Moves the caret to a position and makes the view follow it. */
+    /**
+     * Moves the caret to a position and makes the view follow it.
+     */
     public void moveCaret(int line, int column) {
         moveTo(line, column);
         requestFollowCaret();
@@ -301,32 +315,34 @@ public void moveCaret(int line, int column) {
 
     @Override
     public void setFocusNode(final Node node, DataHolder options) {
+        // editor must not be scrolled when finding a new selection in a
+        // tree that is being edited
+        if (node != null && !options.hasData(SELECTION_RECOVERY)) {
+            // don't randomly jump to top of eg ClassOrInterfaceBody
+            // when selecting from a caret position
+            scrollToNode(node, !options.hasData(CARET_POSITION));
+        }
 
 
-        boolean changed = !Objects.equals(node, currentFocusNode.getValue());
-        if (changed) {
+        RichRunnable update = () -> {};
+        if (Objects.equals(node, currentFocusNode.getValue())) {
+            return;
+        }
 
-            currentFocusNode.setValue(node);
+        currentFocusNode.setValue(node);
 
-            // editor is only restyled if the selection has changed
-            Platform.runLater(() -> styleNodes(
-                node == null ? emptyList() : singleton(node), StyleLayerIds.FOCUS, true));
+        // editor is only restyled if the selection has changed
+        update = update.andThen(styleNodesUpdate(
+            node == null ? emptyList() : singleton(node), StyleLayerIds.FOCUS, true));
 
-            if (node == null) {
-                highlightRelatedNodes(emptyList());
-            } else {
-                Platform.runLater(() -> highlightRelatedNodes(relatedNodesSelector.getValue().getHighlightedNodesWhenSelecting(node)));
-            }
+        if (node == null) {
+            update = update.andThen(highlightRelatedNodes(emptyList()));
+        } else {
+            update = update.andThen(highlightRelatedNodes(relatedNodesSelector.getValue().getHighlightedNodesWhenSelecting(node)));
         }
 
-        // editor must not be scrolled when finding a new selection in a
-        // tree that is being edited
-        // Editor must be scrolled after styling
-        if (node != null && !options.hasData(SELECTION_RECOVERY)) {
-            // don't randomly jump to top of eg ClassOrInterfaceBody
-            // when selecting from a caret position
-            Platform.runLater(() -> scrollToNode(node, !options.hasData(CARET_POSITION)));
-        }
+        Runnable finalUpdate = update;
+        Platform.runLater(() -> updateStyling(finalUpdate));
     }