gnodet-bot commented on code in PR #28456:
URL: https://github.com/apache/camel/pull/28456#discussion_r4371314776
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/diagram/RouteDiagramWidget.java:
##########
@@ -885,7 +961,89 @@ private List<String> rewrapText(LayoutNode node, int
maxWidth) {
}
}
}
- return wrapText(label, maxWidth);
+ return label;
+ }
+
+ /** The route a link marker names: by id, or a from box's caller in words
in the business view. */
+ private String linkName(LayoutNode node, String linkedRouteId) {
+ if (showDescription && "from".equals(node.type)) {
+ // a from box names its caller in words; a to box already shows
the route it links to in words, so
+ // its marker names the route by id, as the breadcrumb does
+ String desc = routeDescriptions.get(linkedRouteId);
+ if (desc != null && !desc.isBlank()) {
+ return desc;
+ }
+ }
+ return linkedRouteId;
+ }
+
+ private void addFoldedMarker(LayoutNode node, int row, int col) {
+ Integer steps = node.id != null ? foldedSteps.get(node.id) : null;
+ if (steps != null) {
+ markers.add(new Marker(
+ row, col, " ▸ " + steps + (steps == 1 ? " step" : "
steps") + " folded",
+ Theme.muted()));
+ }
+ }
+
+ /** A text with its EIP (up to its first bracket) in the EIP's colour, and
the rest as a label. */
+ private void writeLabel(Buffer buffer, Rect area, int row, int col, String
text, Color eipColor, boolean selected) {
+ int open = text.indexOf('[');
+ String eip = open > 0 ? text.substring(0, open) : text;
+ writeText(buffer, area, row, col, eip,
style(Style.EMPTY.fg(eipColor).bold(), selected));
+ if (open > 0) {
+ writeText(buffer, area, row, col + CharWidth.of(eip),
text.substring(open), style(fromLabelStyle(), selected));
+ }
+ }
+
+ /**
+ * A step at the bare density: a mark of its kind and its text on one row,
with no box; the arrows meet at the mark.
+ * Its text goes on up to half way to the next branch.
+ */
+ private void drawBareNode(Buffer buffer, Rect area, LayoutNode node) {
+ int col = toCol(node.x);
+ int row = toRow(node.y);
+ boolean selected = nodeBoxes.size() == selectedNodeIndex;
+ boolean external = isExternalEndpoint(node);
+ boolean highlighted = node.id != null &&
highlightNodeIds.contains(node.id);
+ Color eipColor;
+ if (highlighted) {
+ eipColor = highlightFailed ? highlightFailColor() :
highlightOkColor();
+ } else {
+ eipColor = external ? externalColor() : getEipColor(node.type);
+ }
+ String text = DiagramText.fit(labelText(node), boxWidth + boxWidth / 2
- 4);
+ writeText(buffer, area, row, col + 1, bareMark(node),
style(Style.EMPTY.fg(eipColor).bold(), selected));
+ if (selected) {
+ setChar(buffer, area, row, col + 2, ' ', selectionStyle());
+ }
+ writeLabel(buffer, area, row, col + 3, text, eipColor, selected);
+ int end = col + 3 + CharWidth.of(text);
+
+ String linkedRouteId = findLinkedRouteId(node);
+ if (linkedRouteId != null) {
+ markers.add(new Marker(row, end, " ↵ " + linkName(node,
linkedRouteId), Theme.label().bold()));
+ end += 3 + CharWidth.of(linkName(node, linkedRouteId));
Review Comment:
💡 `linkName(node, linkedRouteId)` is called twice — once to build the marker
string and again to measure its width. Extract to a local variable:
```suggestion
String link = linkName(node, linkedRouteId);
markers.add(new Marker(row, end, " ↵ " + link,
Theme.label().bold()));
end += 3 + CharWidth.of(link);
```
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/RouteDiagramZoom.java:
##########
@@ -0,0 +1,377 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.dsl.jbang.core.commands.tui;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.WeakHashMap;
+import java.util.concurrent.ConcurrentHashMap;
+
+import dev.tamboui.text.Line;
+import dev.tamboui.text.Span;
+import org.apache.camel.diagram.RouteDiagramLayoutEngine;
+import org.apache.camel.diagram.RouteDiagramLayoutEngine.LayoutNode;
+import org.apache.camel.diagram.RouteDiagramLayoutEngine.LayoutRoute;
+import org.apache.camel.diagram.RouteDiagramLayoutEngine.NodeInfo;
+import org.apache.camel.diagram.RouteDiagramLayoutEngine.RouteInfo;
+import org.apache.camel.diagram.RouteDiagramLayoutEngine.TreeNode;
+import org.apache.camel.diagram.RouteErrorFrame;
+import org.apache.camel.diagram.TopologyErrorFrame;
+import org.apache.camel.dsl.jbang.core.commands.tui.diagram.RouteDensity;
+
+/**
+ * The zoom of the route diagram: how much room a step takes ({@link
RouteDensity}), or the most detail that fits the
+ * screen, and the blocks folded into their first step. A folded block is left
out of the route before the engine lays
+ * it out, so the steps after it move up and its branches give their columns
back.
+ * <p>
+ * The keys change the zoom on the UI thread; the layout reads it on the
background thread, so the state is volatile or
+ * concurrent. When the zoom changes, the selected step stays selected and on
the same row of the screen.
+ */
+final class RouteDiagramZoom {
+
+ /** The zoom chosen last: a density, or fit; the folded blocks are not
kept. */
+ static final String PROP_ROUTE_ZOOM = "camel.tui.routeZoom";
+
+ /** The selected step to select again once the route is laid out anew, and
its row on the screen. */
+ record Restore(int index, int row, int screenRow) {
+ }
+
+ /** The density until another is chosen: boxes of one line, about twice
the steps of the detailed on a screen. */
+ static final RouteDensity DEFAULT = RouteDensity.COMPACT;
+
+ private volatile RouteDensity density = DEFAULT;
+ private volatile boolean fit;
+ /** The folded blocks of each route: the ids of their first steps. */
+ private final Map<String, Set<String>> folded = new ConcurrentHashMap<>();
+ /** The steps of each route as last laid out, without its folds: fold all
folds the blocks hidden in others too. */
+ private final Map<String, RouteInfo> unfolded = new ConcurrentHashMap<>();
+ /** What each layout was made with: its density, and the steps folded into
each folded step. */
+ private final Map<LayoutRoute, RouteDensity> densities =
Collections.synchronizedMap(new WeakHashMap<>());
+ private final Map<LayoutRoute, Map<String, Integer>> foldedSteps
+ = Collections.synchronizedMap(new WeakHashMap<>());
+ // the step to select again after the next layout, and the layout it was
selected on
+ private LayoutRoute anchorLayout;
+ private String anchorNodeId;
+ private int anchorScreenRow;
Review Comment:
⚠️ **Concurrency race: anchor fields must be `volatile`.**
The class javadoc says *"The keys change the zoom on the UI thread; the
layout reads it on the background thread, so the state is volatile or
concurrent."* — but `anchorLayout`, `anchorNodeId`, and `anchorScreenRow` are
plain (non-volatile) fields.
`anchor()` is called on the UI thread (from `handleZoomKeys`,
`handleDiagramEscape`, `followLinkedRoute`); `restore()` is called on the
background layout thread (from `renderNativeRouteDiagram` →
`zoom.restore(routeLayout)`). Without `volatile`, the Java Memory Model gives
no guarantee that the background thread will ever see the UI thread's writes —
the anchor could be invisibly stale, causing the selected step to silently not
be restored after a zoom or fold.
The `density` and `fit` fields two lines above are correctly marked
`volatile`; these three should match.
```suggestion
private volatile LayoutRoute anchorLayout;
private volatile String anchorNodeId;
private volatile int anchorScreenRow;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
