From 89ae8a9a5d2743c17fbf92f64636855a1e0a20ee Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 29 Sep 2026 20:34:43 +0000 Subject: [PATCH 1/8] Add collapsed subtopology view to topology diagrams Topology and connection-group diagrams gain a TopologyView (flattened | collapsed). Collapsed draws each directly imported topology as one node:topology carrying its declared ports, the diagrammed topology's own declared ports as node:boundary elements, and connections that reach into an imported topology as implicit edges ending on the collapsed node's boundary (no rendered port is invented). Flattened remains the default. Exposed as fpp-diagram --view {flattened,collapsed}, the fpp/diagram LSP request's optional topologyView, and the FPP: Toggle Collapsed Subtopologies command in the VS Code diagram panel. Co-Authored-By: michael.d.starch --- editors/code/common/models.ts | 13 +- editors/code/package.json | 11 + editors/code/src/diagram/index.ts | 5 + editors/code/src/diagram/layout-config.ts | 17 +- editors/code/src/diagram/manager.ts | 1 + editors/code/src/lsp_ext.ts | 7 + editors/code/webview/css/diagram.css | 29 ++ editors/code/webview/src/di.config.ts | 5 +- editors/code/webview/src/views.tsx | 46 ++- fpp_diagram/src/bin/fpp-diagram.rs | 49 ++- fpp_diagram/src/ir.rs | 63 +++- fpp_diagram/src/lib.rs | 42 ++- fpp_diagram/src/lower.rs | 426 ++++++++++++++++++++-- fpp_diagram/src/sprotty.rs | 65 +++- fpp_diagram/tests/collapsed.rs | 413 +++++++++++++++++++++ fpp_lsp_server/src/handlers.rs | 3 +- fpp_lsp_server/src/lsp_ext.rs | 22 ++ 17 files changed, 1165 insertions(+), 52 deletions(-) create mode 100644 fpp_diagram/tests/collapsed.rs diff --git a/editors/code/common/models.ts b/editors/code/common/models.ts index f0105b50..36c7b522 100644 --- a/editors/code/common/models.ts +++ b/editors/code/common/models.ts @@ -5,12 +5,23 @@ * (`kind` on component nodes, `kind`/`isOutput` on ports) and are shared between * the extension host and the webview. */ -import type { SNode, SPort } from "sprotty-protocol"; +import type { SEdge, SNode, SPort } from "sprotty-protocol"; export interface ComponentSNode extends SNode { kind: string } +/** A declared port of the diagrammed topology, drawn on the diagram boundary. */ +export interface BoundarySNode extends SNode { + kind: string, + isOutput: boolean, // Outputs are pinned to the last ELK layer, inputs to the first. +} + +export interface FppSEdge extends SEdge { + implicit?: boolean, // Ends on a collapsed topology's boundary rather than a rendered port. + detail?: string, // Hover text. +} + export interface PortSNode extends SPort { kind: string, isOutput: boolean, // Store output info here for ELK layout config. Input ports are positioned west, and output ports are positioned east. diff --git a/editors/code/package.json b/editors/code/package.json index 811be071..98e5af1a 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -89,6 +89,12 @@ "icon": "$(eye)", "category": "FPP Diagram" }, + { + "command": "fpp.diagram.toggle-collapsed-subtopologies", + "title": "FPP: Toggle Collapsed Subtopologies", + "icon": "$(group-by-ref-type)", + "category": "FPP Diagram" + }, { "command": "fpp.stateMachine.fit", "title": "FPP: Fit State Machine Diagram", @@ -178,6 +184,11 @@ "when": "activeWebviewPanelId == 'fppDiagrams'", "group": "navigation" }, + { + "command": "fpp.diagram.toggle-collapsed-subtopologies", + "when": "activeWebviewPanelId == 'fppDiagrams'", + "group": "navigation" + }, { "command": "fpp.stateMachine.fit", "when": "activeWebviewPanelId == 'fppStateMachineDiagram'", diff --git a/editors/code/src/diagram/index.ts b/editors/code/src/diagram/index.ts index 4a87a703..12c6d0c2 100644 --- a/editors/code/src/diagram/index.ts +++ b/editors/code/src/diagram/index.ts @@ -108,6 +108,11 @@ export function registerDiagramSupport( !webviewPanelManager.diagramConfig.hideUnusedPorts; void webviewPanelManager.updateDiagram(); }), + vscode.commands.registerCommand("fpp.diagram.toggle-collapsed-subtopologies", () => { + webviewPanelManager.diagramConfig.collapseSubtopologies = + !webviewPanelManager.diagramConfig.collapseSubtopologies; + void webviewPanelManager.updateDiagram(); + }), vscode.commands.registerCommand("fpp.stateMachine.fit", () => mermaidPanel.fit()), vscode.commands.registerCommand("fpp.stateMachine.export", () => mermaidPanel.export()), vscode.commands.registerCommand("fpp.stateMachine.view-source", () => mermaidPanel.viewSource()), diff --git a/editors/code/src/diagram/layout-config.ts b/editors/code/src/diagram/layout-config.ts index 402a2afe..00d790de 100644 --- a/editors/code/src/diagram/layout-config.ts +++ b/editors/code/src/diagram/layout-config.ts @@ -2,7 +2,7 @@ import { LayoutOptions } from "elkjs"; import { DefaultLayoutConfigurator } from "sprotty-elk"; import { SGraph, SEdge, SNode, SLabel } from 'sprotty-protocol'; import { SModelIndex } from "sprotty-protocol"; -import { PortSNode } from "../../common/models"; +import { BoundarySNode, PortSNode } from "../../common/models"; /** * The kinds of diagram the extension can request from the language server. @@ -21,6 +21,7 @@ export class FppDiagramConfig extends DefaultLayoutConfigurator { // Stateful diagram options public hideUnusedPorts = true; // By default, hide unused ports. + public collapseSubtopologies = false; // By default, flatten imported topologies into the diagram. public currentDiagramType: DiagramType | undefined; // Current diagram type public fullyQualifiedName: string = ""; // Fully qualified name of the element currently displayed @@ -103,11 +104,25 @@ export class FppDiagramConfig extends DefaultLayoutConfigurator { 'elk.padding': '[top=8,left=10,bottom=8,right=10]', }; } + if (snode.type === 'node:boundary') { + // A boundary port of the diagrammed topology: pin inputs to the first + // layer and outputs to the last so they sit on the diagram's edge. + const isOutput = (snode as BoundarySNode).isOutput; + return { + 'elk.layered.layerConstraint': isOutput ? 'LAST_SEPARATE' : 'FIRST_SEPARATE', + 'elk.nodeLabels.placement': 'INSIDE, H_CENTER, V_CENTER', + 'elk.nodeSize.constraints': 'NODE_LABELS, MINIMUM_SIZE', + 'elk.nodeSize.minimum': '(40, 22)', + 'elk.padding': '[top=4,left=14,bottom=4,right=14]', + }; + } return { "elk.nodeLabels.placement": "INSIDE, H_CENTER, V_CENTER", "elk.portLabels.nextToPortIfPossible": 'true', 'elk.portConstraints': 'FIXED_SIDE', // So that elk.port.side can take effect. "elk.nodeSize.constraints": "PORTS, PORT_LABELS, NODE_LABELS, MINIMUM_SIZE", + // Collapsed topologies stand in for several components; keep them boxy. + ...(snode.type === 'node:topology' ? { 'elk.nodeSize.minimum': '(180, 90)' } : {}), }; } diff --git a/editors/code/src/diagram/manager.ts b/editors/code/src/diagram/manager.ts index 318e3d54..a98f8216 100644 --- a/editors/code/src/diagram/manager.ts +++ b/editors/code/src/diagram/manager.ts @@ -76,6 +76,7 @@ export class FppWebviewPanelManager extends WebviewPanelManager { kind: kind as unknown as lsp_ext.DiagramKind, name: this.diagramConfig.fullyQualifiedName, hideUnusedPorts: this.diagramConfig.hideUnusedPorts, + topologyView: this.diagramConfig.collapseSubtopologies ? "collapsed" : "flattened", }); return model as SGraph; } catch (e) { diff --git a/editors/code/src/lsp_ext.ts b/editors/code/src/lsp_ext.ts index e265445d..4202dd17 100644 --- a/editors/code/src/lsp_ext.ts +++ b/editors/code/src/lsp_ext.ts @@ -24,6 +24,13 @@ export type DiagramParams = { * full flattened executed action sequence. Defaults to `"uml"` when omitted. */ transitionActionMode?: "uml" | "flattened", + /** + * How imported topologies are drawn (topology and connection-group diagrams + * only). `"flattened"` makes every component instance a node; `"collapsed"` + * draws each directly imported topology as one node with its declared + * ports. Defaults to `"flattened"` when omitted. + */ + topologyView?: "flattened" | "collapsed", }; /** diff --git a/editors/code/webview/css/diagram.css b/editors/code/webview/css/diagram.css index d8fac540..e415109f 100644 --- a/editors/code/webview/css/diagram.css +++ b/editors/code/webview/css/diagram.css @@ -78,6 +78,35 @@ tspan { stroke: #fe92a8; } +/* A collapsed imported topology: dashed outline marks a boundary, not a component. */ +.topology-node { + fill: var(--vscode-editor-inactiveSelectionBackground); + stroke: var(--vscode-foreground); + stroke-width: 1.5; + stroke-dasharray: 8 4; +} + +/* A declared port of the diagrammed topology, on the diagram boundary. */ +.boundary-port { + stroke: var(--vscode-foreground); + stroke-width: 1; +} + +.boundary-input { + fill: var(--vscode-charts-blue); +} + +.boundary-output { + fill: var(--vscode-charts-orange); +} + +/* An implicit connection: it crosses a collapsed topology's boundary rather than + ending on a rendered port. */ +.sprotty-edge>path.edge-implicit { + stroke-dasharray: 5 4; + opacity: 0.85; +} + .port-sync { /* Red expresses the notion of "caution", because the component is synchronous. */ fill: #fe92a8; diff --git a/editors/code/webview/src/di.config.ts b/editors/code/webview/src/di.config.ts index 50d076b8..e40b3720 100644 --- a/editors/code/webview/src/di.config.ts +++ b/editors/code/webview/src/di.config.ts @@ -3,7 +3,7 @@ import '../css/diagram.css'; import { Container, ContainerModule } from 'inversify'; import { boundsFeature, configureModelElement, configureViewerOptions, editFeature, layoutableChildFeature, loadDefaultModules, moveFeature, overrideViewerOptions, SEdgeImpl, SGraphImpl, SGraphView, SLabelImpl, SLabelView, SNodeImpl, SPortImpl, SRoutingHandleImpl, SRoutingHandleView } from 'sprotty'; -import { ArrowEdgeView, ChoiceNodeView, ComponentNodeView, InitialNodeView, MultiLineLabelView, RectanglePortView, StateNodeView } from './views'; +import { ArrowEdgeView, BoundaryNodeView, ChoiceNodeView, ComponentNodeView, InitialNodeView, MultiLineLabelView, RectanglePortView, StateNodeView, TopologyNodeView } from './views'; const myModule = new ContainerModule((bind, unbind, isBound, rebind) => { const context = { bind, unbind, isBound, rebind }; @@ -11,6 +11,9 @@ const myModule = new ContainerModule((bind, unbind, isBound, rebind) => { // Component / topology (port-based) diagrams. configureModelElement(context, 'node:component', SNodeImpl, ComponentNodeView, { enable: [layoutableChildFeature], disable: [moveFeature] }); + // Collapsed topology view: imported topologies as one node, own ports on the boundary. + configureModelElement(context, 'node:topology', SNodeImpl, TopologyNodeView, { enable: [layoutableChildFeature], disable: [moveFeature] }); + configureModelElement(context, 'node:boundary', SNodeImpl, BoundaryNodeView, { enable: [layoutableChildFeature], disable: [moveFeature] }); configureModelElement(context, 'label:node:component', SLabelImpl, SLabelView); configureModelElement(context, 'port', SPortImpl, RectanglePortView, { disable: [boundsFeature] }); // By default, boundsFeature is enabled for SPort. We want fixed-size ports so this feature needs to be disabled. configureModelElement(context, 'label:port', SLabelImpl, SLabelView); diff --git a/editors/code/webview/src/views.tsx b/editors/code/webview/src/views.tsx index e64553d6..1a5be053 100644 --- a/editors/code/webview/src/views.tsx +++ b/editors/code/webview/src/views.tsx @@ -4,7 +4,7 @@ import { injectable } from 'inversify'; import { VNode } from 'snabbdom'; import { IView, IViewArgs, PolylineEdgeView, RenderingContext, SEdgeImpl, SLabelImpl, SLabelView, SNodeImpl, SPortImpl } from 'sprotty'; import { Point, Selectable } from 'sprotty-protocol'; -import { ComponentSNode, PortSNode } from '../../common/models'; +import { BoundarySNode, ComponentSNode, FppSEdge, PortSNode } from '../../common/models'; @injectable() export class ComponentNodeView implements IView { @@ -25,6 +25,47 @@ export class ComponentNodeView implements IView { } } +/** A collapsed imported topology: a dashed box carrying its declared ports. */ +@injectable() +export class TopologyNodeView implements IView { + render(node: Readonly, context: RenderingContext): VNode { + return + + + {context.renderChildren(node)} + ; + } +} + +/** + * A declared port of the diagrammed topology, on the diagram boundary: a + * chevron pointing the way data flows (into the diagram for inputs, out for + * outputs). + */ +@injectable() +export class BoundaryNodeView implements IView { + render(node: Readonly, context: RenderingContext): VNode { + const w = node.size.width; + const h = node.size.height; + const tip = Math.min(10, h / 2); + const chevron = `0,0 ${w - tip},0 ${w},${h / 2} ${w - tip},${h} 0,${h} ${tip},${h / 2}`; + return + + {context.renderChildren(node)} + ; + } +} + @injectable() export class TrianglePortView implements IView { render(node: Readonly, context: RenderingContext): VNode { @@ -73,7 +114,7 @@ export class RightAlignedLabelView extends SLabelView { @injectable() export class ArrowEdgeView extends PolylineEdgeView { - override renderLine(edge: SEdgeImpl & { detail?: string }, segments: Point[], context: RenderingContext, args?: IViewArgs): VNode { + override renderLine(edge: SEdgeImpl & FppSEdge, segments: Point[], context: RenderingContext, args?: IViewArgs): VNode { const firstPoint = segments[0]; let path = `M ${firstPoint.x},${firstPoint.y}`; for (let i = 1; i < segments.length; i++) { @@ -97,6 +138,7 @@ export class ArrowEdgeView extends PolylineEdgeView { diff --git a/fpp_diagram/src/bin/fpp-diagram.rs b/fpp_diagram/src/bin/fpp-diagram.rs index 340dd473..67214628 100644 --- a/fpp_diagram/src/bin/fpp-diagram.rs +++ b/fpp_diagram/src/bin/fpp-diagram.rs @@ -9,7 +9,7 @@ use std::io::Read; use std::process::exit; use clap::{Parser, ValueEnum}; -use fpp_diagram::{DiagramKind, TransitionActionMode}; +use fpp_diagram::{DiagramKind, TopologyView, TransitionActionMode}; #[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum)] enum ModeArg { @@ -30,6 +30,27 @@ impl From for TransitionActionMode { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum)] +enum ViewArg { + /// Every component instance is a node, including those of imported + /// topologies. + Flattened, + /// Directly imported topologies are single collapsed nodes exposing their + /// declared ports; the topology's own declared ports are drawn on the + /// boundary; connections reaching into an imported topology are drawn as + /// implicit edges onto its boundary. + Collapsed, +} + +impl From for TopologyView { + fn from(v: ViewArg) -> Self { + match v { + ViewArg::Flattened => TopologyView::Flattened, + ViewArg::Collapsed => TopologyView::Collapsed, + } + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum)] enum FormatArg { /// Mermaid `stateDiagram-v2` source text (state machines only). @@ -83,6 +104,11 @@ struct Args { #[arg(long)] hide_unused_ports: bool, + /// How imported topologies are drawn (topology and connection-group + /// diagrams only). + #[arg(long, value_enum, default_value_t = ViewArg::Flattened)] + view: ViewArg, + /// The FPP source file to read. Reads from stdin when omitted. file: Option, } @@ -141,6 +167,7 @@ fn main() { fn lower(a: &fpp_analysis::Analysis, args: &Args) -> Result { let kind: DiagramKind = args.kind.into(); let mode: TransitionActionMode = args.mode.into(); + let view: TopologyView = args.view.into(); match args.format { FormatArg::Mermaid if kind == DiagramKind::StateMachine => { @@ -149,13 +176,25 @@ fn lower(a: &fpp_analysis::Analysis, args: &Args) -> Result { // Mermaid is only implemented for state machines; other kinds fall // back to sprotty JSON. - let json = - fpp_diagram::lower_to_smodel(a, kind, &args.name, args.hide_unused_ports, mode)?; + let json = fpp_diagram::lower_to_smodel_view( + a, + kind, + &args.name, + args.hide_unused_ports, + mode, + view, + )?; Ok(json.to_string()) } FormatArg::Sprotty => { - let json = - fpp_diagram::lower_to_smodel(a, kind, &args.name, args.hide_unused_ports, mode)?; + let json = fpp_diagram::lower_to_smodel_view( + a, + kind, + &args.name, + args.hide_unused_ports, + mode, + view, + )?; Ok(json.to_string()) } } diff --git a/fpp_diagram/src/ir.rs b/fpp_diagram/src/ir.rs index c5abb636..ecd07ef7 100644 --- a/fpp_diagram/src/ir.rs +++ b/fpp_diagram/src/ir.rs @@ -46,6 +46,23 @@ pub enum TransitionActionMode { Flattened, } +/// How a topology diagram treats the topologies imported into the diagrammed +/// topology. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub enum TopologyView { + /// Every component instance is a node, including those brought in by + /// imported topologies (the default). + #[default] + Flattened, + /// Each directly imported topology is a single collapsed node exposing its + /// declared topology ports, and the diagrammed topology's own declared + /// ports are drawn as boundary ports. Connections that reach into an + /// imported topology without going through one of its declared ports are + /// drawn as implicit edges ending on the collapsed node's boundary. + Collapsed, +} + /// The kind of a component, which drives node styling. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -140,20 +157,48 @@ pub struct Node { pub ports: Vec, } +/// A directly imported topology drawn as a single collapsed node +/// ([`TopologyView::Collapsed`] only). +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct TopologyNode { + /// Stable identifier: the qualified topology name. + pub id: String, + /// The unqualified topology name. + pub name: String, + /// The fully qualified topology name. + pub qualified_name: String, + /// The topology's declared ports (`port x = inst.p`), expanded per index. + pub ports: Vec, +} + /// A connection between two ports in the diagram. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct Edge { /// Stable identifier. pub id: String, - /// The source port id (an output port). Matches a [`Port::id`]. + /// The source port id (an output port). Matches a [`Port::id`]; for an + /// implicit edge it may instead be a [`TopologyNode::id`]. pub from_port: String, - /// The target port id (an input port). Matches a [`Port::id`]. + /// The target port id (an input port). Matches a [`Port::id`]; for an + /// implicit edge it may instead be a [`TopologyNode::id`]. pub to_port: String, - /// The connection graph (group) name this edge belongs to. + /// The connection graph (group) name this edge belongs to. Empty for the + /// edges joining a boundary port to the port it aliases. pub graph_name: String, /// Whether this connection is declared `unmatched`. pub unmatched: bool, + /// Whether an endpoint reaches into a collapsed topology without going + /// through one of its declared ports. Such an endpoint is the collapsed + /// node's id, and the edge is drawn onto the node's boundary rather than a + /// rendered port. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub implicit: bool, + /// Hover text naming the underlying component ports the edge stands for + /// (one connection per line); empty when the endpoints already say so. + #[serde(default, skip_serializing_if = "String::is_empty")] + pub detail: String, } /// A complete diagram in intermediate form. @@ -165,6 +210,14 @@ pub struct Diagram { pub name: String, pub nodes: Vec, pub edges: Vec, + /// Collapsed imported topologies ([`TopologyView::Collapsed`] only). + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub topology_nodes: Vec, + /// The diagrammed topology's own declared ports, drawn on the diagram + /// boundary ([`TopologyView::Collapsed`] only). Their ids are + /// `..`. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub boundary_ports: Vec, } impl Port { @@ -179,7 +232,9 @@ impl Diagram { /// /// This is the IR-level equivalent of the legacy "hide unused ports" toggle. /// It is a no-op for [`DiagramKind::Component`] diagrams, which have no edges - /// and are meant to show a component's full port surface. + /// and are meant to show a component's full port surface. The declared + /// ports of collapsed topology nodes are always kept: they are the + /// interface the collapsed view exists to show. pub fn prune_unused_ports(&mut self) { if self.kind == DiagramKind::Component { return; diff --git a/fpp_diagram/src/lib.rs b/fpp_diagram/src/lib.rs index 069e7721..75c72611 100644 --- a/fpp_diagram/src/lib.rs +++ b/fpp_diagram/src/lib.rs @@ -19,7 +19,7 @@ pub mod lower_sm; pub mod mermaid; pub mod sprotty; -pub use ir::{Diagram, DiagramKind, StateMachineDiagram, TransitionActionMode}; +pub use ir::{Diagram, DiagramKind, StateMachineDiagram, TopologyView, TransitionActionMode}; pub use layout::SmLayout; pub use lower::LowerError; @@ -32,12 +32,23 @@ use fpp_analysis::Analysis; /// [`DiagramKind::StateMachine`] is not a port graph and is not handled here; /// use [`lower_state_machine`] (or [`lower_to_smodel`], which dispatches). pub fn lower(a: &Analysis, kind: DiagramKind, name: &str) -> Result { + lower_view(a, kind, name, TopologyView::Flattened) +} + +/// Like [`lower`], with `view` selecting how topology and connection-group +/// diagrams treat imported topologies (ignored for components). +pub fn lower_view( + a: &Analysis, + kind: DiagramKind, + name: &str, + view: TopologyView, +) -> Result { match kind { DiagramKind::Component => lower::lower_component(a, name), - DiagramKind::Topology => lower::lower_topology(a, name), + DiagramKind::Topology => lower::lower_topology_view(a, name, view), DiagramKind::ConnectionGroup => { let (topology, group) = split_group_name(name); - lower::lower_connection_group(a, topology, group) + lower::lower_connection_group_view(a, topology, group, view) } DiagramKind::StateMachine => Err(LowerError::NotFound { kind, @@ -73,19 +84,40 @@ pub fn lower_state_machine_to_mermaid( /// /// When `hide_unused_ports` is set, ports not referenced by any connection are /// pruned (a no-op for component and state machine diagrams). See -/// [`Diagram::prune_unused_ports`]. +/// [`Diagram::prune_unused_ports`]. Topologies use the flattened view; see +/// [`lower_to_smodel_view`]. pub fn lower_to_smodel( a: &Analysis, kind: DiagramKind, name: &str, hide_unused_ports: bool, mode: TransitionActionMode, +) -> Result { + lower_to_smodel_view( + a, + kind, + name, + hide_unused_ports, + mode, + TopologyView::Flattened, + ) +} + +/// Like [`lower_to_smodel`], with `view` selecting how topology and +/// connection-group diagrams treat imported topologies; see [`TopologyView`]. +pub fn lower_to_smodel_view( + a: &Analysis, + kind: DiagramKind, + name: &str, + hide_unused_ports: bool, + mode: TransitionActionMode, + view: TopologyView, ) -> Result { if kind == DiagramKind::StateMachine { let diagram = lower_state_machine(a, name, mode)?; return Ok(sprotty::state_machine_to_smodel_json(&diagram)); } - let mut diagram = lower(a, kind, name)?; + let mut diagram = lower_view(a, kind, name, view)?; if hide_unused_ports { diagram.prune_unused_ports(); } diff --git a/fpp_diagram/src/lower.rs b/fpp_diagram/src/lower.rs index 06d59059..8c8854ad 100644 --- a/fpp_diagram/src/lower.rs +++ b/fpp_diagram/src/lower.rs @@ -6,12 +6,15 @@ //! numbering) and produces an [`ir::Diagram`]. It performs no layout and knows //! nothing about any rendering framework. -use crate::ir::{self, Diagram, DiagramKind, Edge, Node, Port}; +use crate::ir::{self, Diagram, DiagramKind, Edge, Node, Port, TopologyNode, TopologyView}; use fpp_analysis::Analysis; use fpp_analysis::semantics::{ - Component, ComponentInstance, Connection, Direction as SemDirection, GeneralKind, - InterfaceInstance, PortInstance, PortInstanceType, Symbol, SymbolInterface, Topology, + Component, ComponentInstance, Connection, Direction as SemDirection, Endpoint, GeneralKind, + InterfaceInstance, PortInstance, PortInstanceIdentifier, PortInstanceType, Symbol, + SymbolInterface, Topology, }; +use rustc_hash::{FxHashMap, FxHashSet}; +use std::collections::BTreeMap; /// Errors that can occur while lowering an element to a diagram. #[derive(Debug, Clone, PartialEq, Eq)] @@ -52,35 +55,78 @@ pub fn lower_component(a: &Analysis, name: &str) -> Result name: qualified_name, nodes: vec![node], edges: vec![], + topology_nodes: vec![], + boundary_ports: vec![], }) } /// Lower a topology (by fully qualified name) into a diagram of its component -/// instances and all their connections. +/// instances and all their connections ([`TopologyView::Flattened`]). pub fn lower_topology(a: &Analysis, name: &str) -> Result { + lower_topology_view(a, name, TopologyView::Flattened) +} + +/// Lower a topology (by fully qualified name) into a diagram, with `view` +/// selecting how imported topologies are drawn. +pub fn lower_topology_view( + a: &Analysis, + name: &str, + view: TopologyView, +) -> Result { let topology = find_topology(a, name).ok_or_else(|| LowerError::NotFound { kind: DiagramKind::Topology, name: name.to_string(), })?; - let nodes = instance_nodes(a, topology); - let edges = topology_edges(a, topology, None); - - Ok(Diagram { + let mut diagram = Diagram { kind: DiagramKind::Topology, name: topology.qualified_name.clone(), - nodes, - edges, - }) + nodes: vec![], + edges: vec![], + topology_nodes: vec![], + boundary_ports: vec![], + }; + match view { + TopologyView::Flattened => { + diagram.nodes = instance_nodes(a, topology); + diagram.edges = topology_edges(a, topology, None); + } + TopologyView::Collapsed => { + let scope = CollapsedScope::new(a, topology); + diagram.nodes = scope.component_nodes(); + diagram.topology_nodes = scope.topology_nodes(); + diagram.edges = scope.edges(None); + let (ports, edges) = scope.boundary(); + diagram.boundary_ports = ports; + diagram.edges.extend(edges); + } + } + Ok(diagram) } -/// Lower a single named connection group within a topology into a diagram. +/// Lower a single named connection group within a topology into a diagram +/// ([`TopologyView::Flattened`]). /// /// Only the instances participating in the group's connections are included. pub fn lower_connection_group( a: &Analysis, topology_name: &str, group: &str, +) -> Result { + lower_connection_group_view(a, topology_name, group, TopologyView::Flattened) +} + +/// Lower a single named connection group within a topology into a diagram, +/// with `view` selecting how imported topologies are drawn. +/// +/// Only the nodes participating in the group's connections are included. In the +/// collapsed view the topology's own boundary ports are omitted, since they +/// belong to no connection group. +pub fn lower_connection_group_view( + a: &Analysis, + topology_name: &str, + group: &str, + view: TopologyView, ) -> Result { let topology = find_topology(a, topology_name).ok_or_else(|| LowerError::NotFound { kind: DiagramKind::ConnectionGroup, @@ -94,15 +140,34 @@ pub fn lower_connection_group( }); } - let edges = topology_edges(a, topology, Some(group)); + let (edges, nodes, topology_nodes) = match view { + TopologyView::Flattened => { + let edges = topology_edges(a, topology, Some(group)); + (edges, instance_nodes(a, topology), vec![]) + } + TopologyView::Collapsed => { + let scope = CollapsedScope::new(a, topology); + ( + scope.edges(Some(group)), + scope.component_nodes(), + scope.topology_nodes(), + ) + } + }; - // Keep only the instance nodes that participate in this group. - let used_nodes: rustc_hash::FxHashSet<&str> = edges + // Keep only the nodes that participate in this group. An endpoint is a port + // id, or a collapsed node id for an implicit edge. + let used_nodes: FxHashSet<&str> = edges .iter() - .flat_map(|e| [node_id_of_port(&e.from_port), node_id_of_port(&e.to_port)]) + .flat_map(|e| [e.from_port.as_str(), e.to_port.as_str()]) + .flat_map(|id| [id, node_id_of_port(id)]) .collect(); - let nodes = instance_nodes(a, topology) + let nodes = nodes + .into_iter() + .filter(|n| used_nodes.contains(n.id.as_str())) + .collect(); + let topology_nodes = topology_nodes .into_iter() .filter(|n| used_nodes.contains(n.id.as_str())) .collect(); @@ -112,6 +177,8 @@ pub fn lower_connection_group( name: format!("{}.{}", topology.qualified_name, group), nodes, edges, + topology_nodes, + boundary_ports: vec![], }) } @@ -277,20 +344,7 @@ fn connection_edge( return None; } - // The resolved (auto-assigned) port numbers are keyed by the original - // connection; fall back to any explicit number, then to index 0. - let from_index = topology - .from_port_number_map - .get(connection) - .copied() - .or(connection.from.port_number) - .unwrap_or(0); - let to_index = topology - .to_port_number_map - .get(connection) - .copied() - .or(connection.to.port_number) - .unwrap_or(0); + let (from_index, to_index) = connection_indices(topology, connection); let from_port = Port::make_id( &from_ep.port.interface_instance.qualified_name(), @@ -309,9 +363,319 @@ fn connection_edge( to_port, graph_name: graph_name.to_string(), unmatched: connection.is_unmatched, + implicit: false, + detail: String::new(), }) } +/// The port indices of a connection's endpoints. The resolved (auto-assigned) +/// port numbers are keyed by the original connection; fall back to any explicit +/// number, then to index 0. +fn connection_indices(topology: &Topology, connection: &Connection) -> (i128, i128) { + let from_index = topology + .from_port_number_map + .get(connection) + .copied() + .or(connection.from.port_number) + .unwrap_or(0); + let to_index = topology + .to_port_number_map + .get(connection) + .copied() + .or(connection.to.port_number) + .unwrap_or(0); + (from_index, to_index) +} + +// --- collapsed view --------------------------------------------------------- + +/// Where a connection endpoint lands in the collapsed view. +#[derive(Debug, Clone, PartialEq, Eq)] +enum End { + /// A rendered port: on a direct component instance, or a declared port of a + /// collapsed topology node. + Port(String), + /// The boundary of a collapsed topology node, reached without going through + /// one of its declared ports. + Node(String), +} + +impl End { + fn id(&self) -> &str { + match self { + End::Port(id) | End::Node(id) => id, + } + } +} + +/// What the collapsed view of a topology draws: its direct component instances +/// as nodes, and each directly imported topology as one collapsed node that +/// stands in for every component instance it brings in. +struct CollapsedScope<'a> { + a: &'a Analysis, + topology: &'a Topology, + /// Directly imported topologies by qualified name (the collapsed node id). + imported: BTreeMap, + /// Component instance qualified name → collapsed node id that stands for it. + owner: FxHashMap, +} + +impl<'a> CollapsedScope<'a> { + fn new(a: &'a Analysis, topology: &'a Topology) -> Self { + let imported: BTreeMap = topology + .direct_topologies + .keys() + .filter_map(|sym| a.topology_map.get(sym)) + .map(|t| (t.qualified_name.clone(), t)) + .collect(); + + // An instance the topology declares itself stays a node of its own, even + // if an imported topology also brings it in. + let direct: FxHashSet<&str> = topology + .direct_component_instances + .keys() + .filter_map(|sym| a.component_instance_map.get(sym)) + .map(|ci| ci.qualified_name.as_str()) + .collect(); + + let mut owner = FxHashMap::default(); + for (id, imported_topology) in &imported { + for ci in imported_topology.component_instance_map().into_keys() { + if direct.contains(ci.qualified_name.as_str()) { + continue; + } + owner.entry(ci.qualified_name).or_insert_with(|| id.clone()); + } + } + + CollapsedScope { + a, + topology, + imported, + owner, + } + } + + /// Nodes for the component instances not stood in for by a collapsed node. + fn component_nodes(&self) -> Vec { + self.topology + .component_instance_map() + .into_keys() + .filter(|ci| !self.owner.contains_key(&ci.qualified_name)) + .filter_map(|ci| instance_node(self.a, &ci)) + .collect() + } + + /// One collapsed node per directly imported topology, carrying its declared + /// ports. + fn topology_nodes(&self) -> Vec { + self.imported + .iter() + .map(|(id, t)| TopologyNode { + id: id.clone(), + name: t.get_name().to_string(), + qualified_name: id.clone(), + ports: declared_ports(self.a, t, id), + }) + .collect() + } + + /// Where a port instance identifier (as written in a connection or topology + /// port) lands: a rendered port, or the boundary of a collapsed node. + fn resolve_end(&self, pii: &PortInstanceIdentifier, index: i128) -> Option { + let port_name = pii.port_instance.get_unqualified_name(); + match &pii.interface_instance { + InterfaceInstance::Topology(t) => { + if self.imported.contains_key(&t.qualified_name) { + return Some(End::Port(Port::make_id( + &t.qualified_name, + port_name, + index, + ))); + } + // Not a collapsed node here: follow the alias to what it names. + let aliased = pii.interface_instance.as_topology(self.a)?; + let tp = aliased.port_map.get(port_name)?; + self.resolve_end(&tp.pii, index) + } + InterfaceInstance::Component(ci) => match self.owner.get(&ci.qualified_name) { + Some(node) => Some(End::Node(node.clone())), + None => Some(End::Port(Port::make_id( + &ci.qualified_name, + port_name, + index, + ))), + }, + } + } + + /// Edges for the topology's own connections (not those of imported + /// topologies, which lie inside collapsed nodes). When `only_group` is + /// `Some`, only that named graph is included. + fn edges(&self, only_group: Option<&str>) -> Vec { + let mut edges: Vec = Vec::new(); + // Implicit edges drawn between the same two elements collapse into one. + let mut by_ends: FxHashMap<(String, String), usize> = FxHashMap::default(); + for (graph_name, connections) in &self.topology.local_connection_map { + if let Some(group) = only_group + && group != graph_name + { + continue; + } + for (seq, connection) in connections.iter().enumerate() { + let Some(edge) = self.connection_edge(graph_name, connection, seq) else { + continue; + }; + if !edge.implicit { + edges.push(edge); + continue; + } + let key = (edge.from_port.clone(), edge.to_port.clone()); + match by_ends.get(&key) { + Some(&i) => { + let detail = &mut edges[i].detail; + if !detail.contains(&edge.detail) { + detail.push('\n'); + detail.push_str(&edge.detail); + } + edges[i].unmatched &= edge.unmatched; + } + None => { + by_ends.insert(key, edges.len()); + edges.push(edge); + } + } + } + } + edges + } + + fn connection_edge( + &self, + graph_name: &str, + connection: &Connection, + seq: usize, + ) -> Option { + let (from_index, to_index) = connection_indices(self.topology, connection); + let from = self.resolve_end(&as_written(&connection.from).port, from_index)?; + let to = self.resolve_end(&as_written(&connection.to).port, to_index)?; + + // A connection between two instances of the same collapsed node lies + // entirely inside it. + if let (End::Node(f), End::Node(t)) = (&from, &to) + && f == t + { + return None; + } + + let implicit = matches!(from, End::Node(_)) || matches!(to, End::Node(_)); + let detail = if implicit { + format!( + "{} -> {}", + underlying_label( + self.a, + connection.from.get_underlying_endpoint(self.a).port, + from_index + ), + underlying_label( + self.a, + connection.to.get_underlying_endpoint(self.a).port, + to_index + ), + ) + } else { + String::new() + }; + + Some(Edge { + id: format!("{graph_name}.connection.{seq}"), + from_port: from.id().to_string(), + to_port: to.id().to_string(), + graph_name: graph_name.to_string(), + unmatched: connection.is_unmatched, + implicit, + detail, + }) + } + + /// The topology's own declared ports, drawn on the diagram boundary, and + /// the edges joining each to the port it aliases. + fn boundary(&self) -> (Vec, Vec) { + let node_id = &self.topology.qualified_name; + let mut ports = Vec::new(); + let mut edges = Vec::new(); + for port in declared_ports(self.a, self.topology, node_id) { + let Some(tp) = self.topology.port_map.get(&port.name) else { + continue; + }; + let Some(inner) = self.resolve_end(&tp.pii, port.index) else { + continue; + }; + let implicit = matches!(inner, End::Node(_)); + let detail = if implicit { + underlying_label(self.a, tp.pii.clone(), port.index) + } else { + String::new() + }; + let (from_port, to_port) = match port.direction { + ir::Direction::Input => (port.id.clone(), inner.id().to_string()), + ir::Direction::Output => (inner.id().to_string(), port.id.clone()), + }; + edges.push(Edge { + id: format!("boundary.{}.{}", port.name, port.index), + from_port, + to_port, + graph_name: String::new(), + unmatched: false, + implicit, + detail, + }); + ports.push(port); + } + (ports, edges) + } +} + +/// The endpoint as written in the connection: a resolved endpoint records the +/// topology-port alias it came through, so walk out to the outermost one. +fn as_written(endpoint: &Endpoint) -> &Endpoint { + let mut written = endpoint; + while let Some(alias) = &written.topology_port { + written = alias; + } + written +} + +/// A topology's declared ports (`port x = inst.p`), expanded per index and owned +/// by the node identified by `node_id`. +fn declared_ports(a: &Analysis, topology: &Topology, node_id: &str) -> Vec { + let mut ports: Vec = topology + .port_interface + .port_map + .values() + .flat_map(|pi| expand_port(a, node_id, pi)) + .collect(); + ports.sort_by(|x, y| x.name.cmp(&y.name).then(x.index.cmp(&y.index))); + ports +} + +/// `inst.port` or `inst.port[index]` for the component port a topology-port +/// alias ultimately names. +fn underlying_label(a: &Analysis, pii: PortInstanceIdentifier, index: i128) -> String { + let mut pii = pii; + while let Some(top) = pii.interface_instance.as_topology(a) { + match top.port_map.get(pii.port_instance.get_unqualified_name()) { + Some(tp) => pii = tp.pii.clone(), + None => break, + } + } + if pii.port_instance.get_array_size() > 1 { + format!("{}[{index}]", pii.qualified_name()) + } else { + pii.qualified_name() + } +} + // --- lookup helpers --------------------------------------------------------- /// Find a fully resolved topology by its fully qualified name. diff --git a/fpp_diagram/src/sprotty.rs b/fpp_diagram/src/sprotty.rs index 25d3347e..dcc7138d 100644 --- a/fpp_diagram/src/sprotty.rs +++ b/fpp_diagram/src/sprotty.rs @@ -8,7 +8,7 @@ //! nodes/ports get fixed label sizes but no positions — layout (ELK) runs in the //! JS host. See `docs/visualization-work-to-go.md` §2.1. -use crate::ir::{Diagram, Direction, Node, Port}; +use crate::ir::{Diagram, Direction, Node, Port, TopologyNode}; use serde::{Deserialize, Serialize}; /// Fixed label/port sizes, mirroring the legacy `generator.ts`. Because these @@ -81,12 +81,37 @@ pub enum SModelElement { text: String, size: Dimension, }, + /// A collapsed imported topology node (collapsed topology view). + #[serde(rename = "node:topology", rename_all = "camelCase")] + TopologyNode { + id: String, + children: Vec, + }, + /// A declared port of the diagrammed topology, drawn on the diagram + /// boundary (collapsed topology view). It is a node, not a port, so that + /// ELK can pin it to the first (input) or last (output) layer. + #[serde(rename = "node:boundary", rename_all = "camelCase")] + BoundaryNode { + id: String, + /// FPP extra: the underlying port kind; drives styling. + kind: String, + /// FPP extra: outputs are pinned to the last layer, inputs to the first. + is_output: bool, + children: Vec, + }, /// A connection edge. #[serde(rename = "edge", rename_all = "camelCase")] Edge { id: String, source_id: String, target_id: String, + /// FPP extra: the edge ends on a collapsed topology node's boundary + /// rather than a rendered port; drawn dashed. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + implicit: bool, + /// Hover text; empty if none. Rendered as an SVG ``. + #[serde(default, skip_serializing_if = "String::is_empty")] + detail: String, }, // --- state machine elements --- @@ -152,10 +177,14 @@ pub struct EdgePlacement { /// Lower a diagram IR into a sprotty `SModel` root graph. pub fn to_smodel(diagram: &Diagram) -> SModelElement { let mut children: Vec<SModelElement> = diagram.nodes.iter().map(node_to_smodel).collect(); + children.extend(diagram.topology_nodes.iter().map(topology_node_to_smodel)); + children.extend(diagram.boundary_ports.iter().map(boundary_port_to_smodel)); children.extend(diagram.edges.iter().map(|e| SModelElement::Edge { id: e.id.clone(), source_id: e.from_port.clone(), target_id: e.to_port.clone(), + implicit: e.implicit, + detail: e.detail.clone(), })); SModelElement::Graph { @@ -393,6 +422,40 @@ fn node_to_smodel(node: &Node) -> SModelElement { } } +fn topology_node_to_smodel(node: &TopologyNode) -> SModelElement { + let mut children = vec![ + SModelElement::ComponentLabel { + id: format!("{}.label.name", node.id), + text: node.name.clone(), + size: size::COMPONENT_LABEL.into(), + }, + SModelElement::ComponentLabel { + id: format!("{}.label.class", node.id), + text: format!("topology {}", node.qualified_name), + size: size::COMPONENT_LABEL.into(), + }, + ]; + children.extend(node.ports.iter().map(port_to_smodel)); + SModelElement::TopologyNode { + id: node.id.clone(), + children, + } +} + +fn boundary_port_to_smodel(port: &Port) -> SModelElement { + let label = SModelElement::ComponentLabel { + id: format!("{}.label", port.id), + text: port.label.clone(), + size: text_size(&port.label), + }; + SModelElement::BoundaryNode { + id: port.id.clone(), + kind: port_kind_str(&port.kind), + is_output: matches!(port.direction, Direction::Output), + children: vec![label], + } +} + fn port_to_smodel(port: &Port) -> SModelElement { let label = SModelElement::PortLabel { id: format!("{}.label", port.id), diff --git a/fpp_diagram/tests/collapsed.rs b/fpp_diagram/tests/collapsed.rs new file mode 100644 index 00000000..2f61666e --- /dev/null +++ b/fpp_diagram/tests/collapsed.rs @@ -0,0 +1,413 @@ +//! Integration tests for the collapsed topology view: imported topologies as +//! single nodes, declared ports on the boundary, and implicit edges for +//! connections that reach into an imported topology. + +use fpp_analysis::{Analysis, add_state_enums, check_semantics}; +use fpp_core::SourceFile; +use fpp_diagram::ir::{Diagram, DiagramKind, Direction, Edge, TopologyView}; + +/// Parse `src` and run full semantic analysis, invoking `f` with the resulting +/// [`Analysis`] while the compiler context is installed. +fn with_analysis<R>(src: &str, f: impl FnOnce(&Analysis) -> R) -> R { + let mut sink = Vec::new(); + let mut ctx = fpp_core::CompilerContext::new(fpp_errors::WriteEmitter::new(&mut sink)); + fpp_core::run(&mut ctx, || { + let source = SourceFile::new("test.fpp", src.to_string()); + let mut ast = fpp_parser::parse(source, |p| p.trans_unit(), None); + add_state_enums(&mut ast); + let mut a = Analysis::new(); + let _ = check_semantics(&mut a, vec![&ast]); + f(&a) + }) +} + +/// A deployment `Top` importing a subtopology `Sub.Subtopology`. `Top` reaches +/// the subtopology both through its declared ports (`dataIn`, `dataOut`) and by +/// naming its instances directly (`Sub.inner`). +const MODEL: &str = r#" +port P + +passive component Src { + output port out: P + output port out2: P +} + +active component Sink { + async input port in: P + sync input port in2: P + output port back: P +} + +module Sub { + instance src: Src base id 0x100 + instance inner: Sink base id 0x200 \ + queue size 10 + + topology Subtopology { + instance src + instance inner + + port dataIn = inner.in + port dataOut = src.out2 + port spare = inner.in2 + + connections Internal { + src.out -> inner.in2 + } + } +} + +instance outer: Src base id 0x300 +instance hub: Sink base id 0x400 \ + queue size 10 + +topology Top { + instance outer + instance hub + instance Sub.Subtopology + + connections C { + outer.out -> Sub.Subtopology.dataIn + Sub.Subtopology.dataOut -> hub.in + outer.out2 -> Sub.inner.in2 + Sub.inner.back -> hub.in2 + } +} +"#; + +fn node_ids(d: &Diagram) -> Vec<&str> { + let mut ids: Vec<&str> = d.nodes.iter().map(|n| n.id.as_str()).collect(); + ids.sort(); + ids +} + +fn edge<'a>(d: &'a Diagram, from: &str, to: &str) -> &'a Edge { + d.edges + .iter() + .find(|e| e.from_port == from && e.to_port == to) + .unwrap_or_else(|| panic!("expected an edge {from} -> {to}; have {:?}", d.edges)) +} + +#[test] +fn flattened_view_is_the_default_and_unchanged() { + with_analysis(MODEL, |a| { + let default = fpp_diagram::lower(a, DiagramKind::Topology, "Top").unwrap(); + let flattened = + fpp_diagram::lower_view(a, DiagramKind::Topology, "Top", TopologyView::Flattened) + .unwrap(); + assert_eq!(default, flattened); + + assert_eq!(node_ids(&default), ["Sub.inner", "Sub.src", "hub", "outer"]); + assert!(default.topology_nodes.is_empty()); + assert!(default.boundary_ports.is_empty()); + // Every connection, including the subtopology's own, resolved to + // component ports. + assert_eq!(default.edges.len(), 5); + assert!( + default + .edges + .iter() + .all(|e| !e.implicit && e.detail.is_empty()) + ); + edge(&default, "outer.out.0", "Sub.inner.in.0"); + edge(&default, "Sub.src.out2.0", "hub.in.0"); + edge(&default, "Sub.src.out.0", "Sub.inner.in2.0"); + }); +} + +#[test] +fn collapsed_view_draws_imported_topology_as_one_node() { + with_analysis(MODEL, |a| { + let d = fpp_diagram::lower_view(a, DiagramKind::Topology, "Top", TopologyView::Collapsed) + .unwrap(); + + assert_eq!(node_ids(&d), ["hub", "outer"]); + assert_eq!(d.topology_nodes.len(), 1); + let sub = &d.topology_nodes[0]; + assert_eq!(sub.id, "Sub.Subtopology"); + assert_eq!(sub.name, "Subtopology"); + assert_eq!(sub.qualified_name, "Sub.Subtopology"); + + // The collapsed node carries the subtopology's declared ports, with the + // direction of the port each aliases. + let ports: Vec<(&str, Direction)> = sub + .ports + .iter() + .map(|p| (p.id.as_str(), p.direction)) + .collect(); + assert_eq!( + ports, + [ + ("Sub.Subtopology.dataIn.0", Direction::Input), + ("Sub.Subtopology.dataOut.0", Direction::Output), + ("Sub.Subtopology.spare.0", Direction::Input), + ] + ); + + // `Top` declares no ports of its own. + assert!(d.boundary_ports.is_empty()); + }); +} + +#[test] +fn collapsed_view_edges_use_declared_ports_or_cross_the_boundary() { + with_analysis(MODEL, |a| { + let d = fpp_diagram::lower_view(a, DiagramKind::Topology, "Top", TopologyView::Collapsed) + .unwrap(); + + // Connections through a declared port land on that rendered port. + let e = edge(&d, "outer.out.0", "Sub.Subtopology.dataIn.0"); + assert!(!e.implicit); + assert!(e.detail.is_empty()); + let e = edge(&d, "Sub.Subtopology.dataOut.0", "hub.in.0"); + assert!(!e.implicit); + + // Connections naming an instance inside the subtopology end on the + // collapsed node itself, not on a rendered port, and say what they + // stand for. + let e = edge(&d, "outer.out2.0", "Sub.Subtopology"); + assert!(e.implicit); + assert_eq!(e.detail, "outer.out2 -> Sub.inner.in2"); + let e = edge(&d, "Sub.Subtopology", "hub.in2.0"); + assert!(e.implicit); + assert_eq!(e.detail, "Sub.inner.back -> hub.in2"); + + // The subtopology's internal connection is hidden inside its node, and + // no port was invented on the boundary for the reach-in connections. + assert_eq!(d.edges.len(), 4); + assert!( + d.topology_nodes[0] + .ports + .iter() + .all(|p| p.name != "in2" && p.name != "back") + ); + }); +} + +#[test] +fn collapsed_view_keeps_declared_ports_when_pruning_unused() { + with_analysis(MODEL, |a| { + let mut d = + fpp_diagram::lower_view(a, DiagramKind::Topology, "Top", TopologyView::Collapsed) + .unwrap(); + d.prune_unused_ports(); + // `spare` is declared but unconnected in `Top`; it is still the + // subtopology's interface. + assert_eq!(d.topology_nodes[0].ports.len(), 3); + // Component ports are pruned as usual. + let outer = d.nodes.iter().find(|n| n.id == "outer").unwrap(); + assert_eq!(outer.ports.len(), 2); + let hub = d.nodes.iter().find(|n| n.id == "hub").unwrap(); + assert_eq!(hub.ports.len(), 2); + }); +} + +#[test] +fn collapsed_view_of_subtopology_draws_its_ports_on_the_boundary() { + with_analysis(MODEL, |a| { + let d = fpp_diagram::lower_view( + a, + DiagramKind::Topology, + "Sub.Subtopology", + TopologyView::Collapsed, + ) + .unwrap(); + + assert_eq!(node_ids(&d), ["Sub.inner", "Sub.src"]); + assert!(d.topology_nodes.is_empty()); + + let boundary: Vec<(&str, Direction)> = d + .boundary_ports + .iter() + .map(|p| (p.id.as_str(), p.direction)) + .collect(); + assert_eq!( + boundary, + [ + ("Sub.Subtopology.dataIn.0", Direction::Input), + ("Sub.Subtopology.dataOut.0", Direction::Output), + ("Sub.Subtopology.spare.0", Direction::Input), + ] + ); + + // Each boundary port is joined to the component port it aliases, in + // the direction data flows. + let e = edge(&d, "Sub.Subtopology.dataIn.0", "Sub.inner.in.0"); + assert_eq!(e.id, "boundary.dataIn.0"); + assert!(!e.implicit); + edge(&d, "Sub.src.out2.0", "Sub.Subtopology.dataOut.0"); + edge(&d, "Sub.Subtopology.spare.0", "Sub.inner.in2.0"); + edge(&d, "Sub.src.out.0", "Sub.inner.in2.0"); + assert_eq!(d.edges.len(), 4); + }); +} + +#[test] +fn collapsed_connection_group_keeps_participating_topology_nodes() { + with_analysis(MODEL, |a| { + let d = fpp_diagram::lower_view( + a, + DiagramKind::ConnectionGroup, + "Top.C", + TopologyView::Collapsed, + ) + .unwrap(); + assert_eq!(d.name, "Top.C"); + assert_eq!(node_ids(&d), ["hub", "outer"]); + assert_eq!(d.topology_nodes.len(), 1); + assert_eq!(d.edges.len(), 4); + assert!(d.boundary_ports.is_empty()); + + let flattened = fpp_diagram::lower(a, DiagramKind::ConnectionGroup, "Top.C").unwrap(); + assert_eq!( + node_ids(&flattened), + ["Sub.inner", "Sub.src", "hub", "outer"] + ); + assert!(flattened.topology_nodes.is_empty()); + }); +} + +#[test] +fn sprotty_model_carries_collapsed_elements() { + with_analysis(MODEL, |a| { + let model = fpp_diagram::lower_to_smodel_view( + a, + DiagramKind::Topology, + "Top", + false, + fpp_diagram::TransitionActionMode::Uml, + TopologyView::Collapsed, + ) + .unwrap(); + let children = model["children"].as_array().unwrap(); + + let topology_nodes: Vec<_> = children + .iter() + .filter(|c| c["type"] == "node:topology") + .collect(); + assert_eq!(topology_nodes.len(), 1); + assert_eq!(topology_nodes[0]["id"], "Sub.Subtopology"); + let kids = topology_nodes[0]["children"].as_array().unwrap(); + assert_eq!(kids.iter().filter(|k| k["type"] == "port").count(), 3); + + let implicit: Vec<_> = children + .iter() + .filter(|c| c["type"] == "edge" && c["implicit"] == true) + .collect(); + assert_eq!(implicit.len(), 2); + assert!(implicit.iter().any(|e| { + e["sourceId"] == "outer.out2.0" + && e["targetId"] == "Sub.Subtopology" + && e["detail"] == "outer.out2 -> Sub.inner.in2" + })); + + // Explicit edges carry no `implicit`/`detail` keys, as before. + let explicit = children + .iter() + .find(|c| c["type"] == "edge" && c["targetId"] == "Sub.Subtopology.dataIn.0") + .unwrap(); + assert!(explicit.get("implicit").is_none()); + assert!(explicit.get("detail").is_none()); + + let sub_model = fpp_diagram::lower_to_smodel_view( + a, + DiagramKind::Topology, + "Sub.Subtopology", + false, + fpp_diagram::TransitionActionMode::Uml, + TopologyView::Collapsed, + ) + .unwrap(); + let boundary: Vec<_> = sub_model["children"] + .as_array() + .unwrap() + .iter() + .filter(|c| c["type"] == "node:boundary") + .collect(); + assert_eq!(boundary.len(), 3); + let out = boundary + .iter() + .find(|b| b["id"] == "Sub.Subtopology.dataOut.0") + .unwrap(); + assert_eq!(out["isOutput"], true); + assert_eq!(out["kind"], "output"); + }); +} + +#[test] +fn sprotty_flattened_model_is_unchanged_by_the_view_parameter() { + with_analysis(MODEL, |a| { + let legacy = fpp_diagram::lower_to_smodel( + a, + DiagramKind::Topology, + "Top", + true, + fpp_diagram::TransitionActionMode::Uml, + ) + .unwrap(); + let flattened = fpp_diagram::lower_to_smodel_view( + a, + DiagramKind::Topology, + "Top", + true, + fpp_diagram::TransitionActionMode::Uml, + TopologyView::Flattened, + ) + .unwrap(); + assert_eq!(legacy, flattened); + let children = legacy["children"].as_array().unwrap(); + assert!(children.iter().all(|c| c["type"] != "node:topology")); + assert!(children.iter().all(|c| c.get("implicit").is_none())); + }); +} + +#[test] +fn cli_view_switch_selects_the_collapsed_view() { + let dir = std::env::temp_dir().join(format!("fpp-diagram-collapsed-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let file = dir.join("model.fpp"); + std::fs::write(&file, MODEL).unwrap(); + + let run = |extra: &[&str]| { + let out = std::process::Command::new(env!("CARGO_BIN_EXE_fpp-diagram")) + .args(["--kind", "topology", "--name", "Top", "--format", "sprotty"]) + .args(extra) + .arg(&file) + .output() + .expect("fpp-diagram runs"); + assert!( + out.status.success(), + "{}", + String::from_utf8_lossy(&out.stderr) + ); + serde_json::from_slice::<serde_json::Value>(&out.stdout).expect("sprotty JSON") + }; + + let flattened = run(&[]); + let collapsed = run(&["--view", "collapsed"]); + std::fs::remove_dir_all(&dir).unwrap(); + + let types = |m: &serde_json::Value| -> Vec<String> { + m["children"] + .as_array() + .unwrap() + .iter() + .map(|c| c["type"].as_str().unwrap().to_string()) + .collect() + }; + assert!(!types(&flattened).iter().any(|t| t == "node:topology")); + assert_eq!( + types(&collapsed) + .iter() + .filter(|t| *t == "node:topology") + .count(), + 1 + ); + assert_eq!( + types(&collapsed) + .iter() + .filter(|t| *t == "node:component") + .count(), + 2 + ); +} diff --git a/fpp_lsp_server/src/handlers.rs b/fpp_lsp_server/src/handlers.rs index f2412cd8..439b4d44 100644 --- a/fpp_lsp_server/src/handlers.rs +++ b/fpp_lsp_server/src/handlers.rs @@ -127,12 +127,13 @@ pub fn handle_diagram( .map(serde_json::Value::String) .map_err(|e| anyhow::anyhow!("{e}")); } - fpp_diagram::lower_to_smodel( + fpp_diagram::lower_to_smodel_view( &state.analysis, params.kind.into(), ¶ms.name, params.hide_unused_ports, params.transition_action_mode.into(), + params.topology_view.into(), ) .map_err(|e| anyhow::anyhow!("{e}")) } diff --git a/fpp_lsp_server/src/lsp_ext.rs b/fpp_lsp_server/src/lsp_ext.rs index 30270388..71486511 100644 --- a/fpp_lsp_server/src/lsp_ext.rs +++ b/fpp_lsp_server/src/lsp_ext.rs @@ -95,6 +95,28 @@ pub struct DiagramParams { /// [`TransitionActionMode::Uml`] when the client omits it. #[serde(default)] pub transition_action_mode: TransitionActionMode, + /// How imported topologies are drawn in topology diagrams. Defaults to + /// [`TopologyView::Flattened`] when the client omits it. + #[serde(default)] + pub topology_view: TopologyView, +} + +/// Mirrors [`fpp_diagram::TopologyView`] on the wire. +#[derive(Debug, PartialEq, Eq, Deserialize, Serialize, Clone, Copy, Default)] +#[serde(rename_all = "camelCase")] +pub enum TopologyView { + #[default] + Flattened, + Collapsed, +} + +impl From<TopologyView> for fpp_diagram::TopologyView { + fn from(view: TopologyView) -> Self { + match view { + TopologyView::Flattened => fpp_diagram::TopologyView::Flattened, + TopologyView::Collapsed => fpp_diagram::TopologyView::Collapsed, + } + } } /// A request to lower a topology/component/connection-group into a sprotty From a9887596f06a97c795574791077dae1c0b53e171 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 29 Sep 2026 20:45:33 +0000 Subject: [PATCH 2/8] Collapsed view: draw imported connections that cross a boundary, disambiguate shared instances Edges now come from connection_map (imported connections included), dropping only those wholly inside one collapsed node. An instance owned by two unrelated imports, or declared by the diagrammed topology itself, stays a node of its own; an instance inside nested imports is owned by the innermost. An imported connection group keeps the collapsed node it lives in. Fix the CSS selector so implicit edges render dashed inside the edge's <g>. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> --- editors/code/webview/css/diagram.css | 2 +- fpp_diagram/src/lower.rs | 139 +++++++++++++++------- fpp_diagram/tests/collapsed.rs | 170 +++++++++++++++++++++++++++ 3 files changed, 269 insertions(+), 42 deletions(-) diff --git a/editors/code/webview/css/diagram.css b/editors/code/webview/css/diagram.css index e415109f..c9172d88 100644 --- a/editors/code/webview/css/diagram.css +++ b/editors/code/webview/css/diagram.css @@ -102,7 +102,7 @@ tspan { /* An implicit connection: it crosses a collapsed topology's boundary rather than ending on a rendered port. */ -.sprotty-edge>path.edge-implicit { +.sprotty-edge path.edge-implicit { stroke-dasharray: 5 4; opacity: 0.85; } diff --git a/fpp_diagram/src/lower.rs b/fpp_diagram/src/lower.rs index 8c8854ad..8f470889 100644 --- a/fpp_diagram/src/lower.rs +++ b/fpp_diagram/src/lower.rs @@ -140,38 +140,38 @@ pub fn lower_connection_group_view( }); } + // Keep only the nodes that participate in this group; a collapsed node + // participates even when the group's wiring lies wholly inside it. let (edges, nodes, topology_nodes) = match view { TopologyView::Flattened => { let edges = topology_edges(a, topology, Some(group)); - (edges, instance_nodes(a, topology), vec![]) + let used_nodes: FxHashSet<&str> = edges + .iter() + .flat_map(|e| [node_id_of_port(&e.from_port), node_id_of_port(&e.to_port)]) + .collect(); + let nodes = instance_nodes(a, topology) + .into_iter() + .filter(|n| used_nodes.contains(n.id.as_str())) + .collect(); + (edges, nodes, vec![]) } TopologyView::Collapsed => { let scope = CollapsedScope::new(a, topology); - ( - scope.edges(Some(group)), - scope.component_nodes(), - scope.topology_nodes(), - ) + let touched = scope.touched(Some(group)); + let nodes = scope + .component_nodes() + .into_iter() + .filter(|n| touched.contains(&n.id)) + .collect(); + let topology_nodes = scope + .topology_nodes() + .into_iter() + .filter(|n| touched.contains(&n.id)) + .collect(); + (scope.edges(Some(group)), nodes, topology_nodes) } }; - // Keep only the nodes that participate in this group. An endpoint is a port - // id, or a collapsed node id for an implicit edge. - let used_nodes: FxHashSet<&str> = edges - .iter() - .flat_map(|e| [e.from_port.as_str(), e.to_port.as_str()]) - .flat_map(|id| [id, node_id_of_port(id)]) - .collect(); - - let nodes = nodes - .into_iter() - .filter(|n| used_nodes.contains(n.id.as_str())) - .collect(); - let topology_nodes = topology_nodes - .into_iter() - .filter(|n| used_nodes.contains(n.id.as_str())) - .collect(); - Ok(Diagram { kind: DiagramKind::ConnectionGroup, name: format!("{}.{}", topology.qualified_name, group), @@ -408,9 +408,15 @@ impl End { } } -/// What the collapsed view of a topology draws: its direct component instances -/// as nodes, and each directly imported topology as one collapsed node that -/// stands in for every component instance it brings in. +/// What the collapsed view of a topology draws: each directly imported topology +/// as one collapsed node standing in for the component instances it owns, and +/// every other component instance as a node of its own. +/// +/// An instance is owned by the innermost directly imported topology containing +/// it: of the imports that contain it, those importing another such import are +/// ruled out. An instance the diagrammed topology declares itself, or that two +/// unrelated imports both contain, has no single owner and stays visible as a +/// node of its own, so nothing is hidden inside a box it does not belong to. struct CollapsedScope<'a> { a: &'a Analysis, topology: &'a Topology, @@ -429,8 +435,6 @@ impl<'a> CollapsedScope<'a> { .map(|t| (t.qualified_name.clone(), t)) .collect(); - // An instance the topology declares itself stays a node of its own, even - // if an imported topology also brings it in. let direct: FxHashSet<&str> = topology .direct_component_instances .keys() @@ -438,15 +442,42 @@ impl<'a> CollapsedScope<'a> { .map(|ci| ci.qualified_name.as_str()) .collect(); - let mut owner = FxHashMap::default(); - for (id, imported_topology) in &imported { + // Imports containing each instance, innermost first. + let mut containing: FxHashMap<String, Vec<&Topology>> = FxHashMap::default(); + for imported_topology in imported.values() { for ci in imported_topology.component_instance_map().into_keys() { - if direct.contains(ci.qualified_name.as_str()) { - continue; + if !direct.contains(ci.qualified_name.as_str()) { + containing + .entry(ci.qualified_name) + .or_default() + .push(imported_topology); } - owner.entry(ci.qualified_name).or_insert_with(|| id.clone()); } } + let imports = |t: &Topology, other: &Topology| -> bool { + t.transitive_import_set.iter().any(|sym| { + a.topology_map.get(sym).map(|d| d.qualified_name.as_str()) + == Some(&other.qualified_name) + }) + }; + let owner = containing + .into_iter() + .filter_map(|(instance, candidates)| { + let innermost: Vec<&Topology> = candidates + .iter() + .copied() + .filter(|c| { + !candidates + .iter() + .any(|o| !std::ptr::eq(*c, *o) && imports(c, o)) + }) + .collect(); + match innermost.as_slice() { + [only] => Some((instance, only.qualified_name.clone())), + _ => None, + } + }) + .collect(); CollapsedScope { a, @@ -509,19 +540,45 @@ impl<'a> CollapsedScope<'a> { } } - /// Edges for the topology's own connections (not those of imported - /// topologies, which lie inside collapsed nodes). When `only_group` is + /// The connection graphs to draw: all of them, or just `only_group`. + fn graphs( + &self, + only_group: Option<&str>, + ) -> impl Iterator<Item = (&String, &Vec<Connection>)> { + self.topology + .connection_map + .iter() + .filter(move |(graph_name, _)| only_group.is_none_or(|g| g == graph_name.as_str())) + } + + /// Ids of the nodes the connections of the given graphs touch, including + /// collapsed nodes whose interior wiring they are. + fn touched(&self, only_group: Option<&str>) -> FxHashSet<String> { + self.graphs(only_group) + .flat_map(|(_, connections)| connections) + .flat_map(|c| { + let (from_index, to_index) = connection_indices(self.topology, c); + [ + self.resolve_end(&as_written(&c.from).port, from_index), + self.resolve_end(&as_written(&c.to).port, to_index), + ] + }) + .flatten() + .map(|end| match end { + End::Port(id) => node_id_of_port(&id).to_string(), + End::Node(id) => id, + }) + .collect() + } + + /// Edges for every connection of the topology, imported ones included, + /// except those lying wholly inside one collapsed node. When `only_group` is /// `Some`, only that named graph is included. fn edges(&self, only_group: Option<&str>) -> Vec<Edge> { let mut edges: Vec<Edge> = Vec::new(); // Implicit edges drawn between the same two elements collapse into one. let mut by_ends: FxHashMap<(String, String), usize> = FxHashMap::default(); - for (graph_name, connections) in &self.topology.local_connection_map { - if let Some(group) = only_group - && group != graph_name - { - continue; - } + for (graph_name, connections) in self.graphs(only_group) { for (seq, connection) in connections.iter().enumerate() { let Some(edge) = self.connection_edge(graph_name, connection, seq) else { continue; diff --git a/fpp_diagram/tests/collapsed.rs b/fpp_diagram/tests/collapsed.rs index 2f61666e..9b94aca7 100644 --- a/fpp_diagram/tests/collapsed.rs +++ b/fpp_diagram/tests/collapsed.rs @@ -411,3 +411,173 @@ fn cli_view_switch_selects_the_collapsed_view() { 2 ); } + +#[test] +fn collapsed_view_of_imported_connection_group_keeps_its_topology_node() { + with_analysis(MODEL, |a| { + // `Internal` is defined in the subtopology; in `Top`'s collapsed view + // its wiring lies wholly inside the collapsed node, which is still + // drawn so the group has a home. + let d = fpp_diagram::lower_view( + a, + DiagramKind::ConnectionGroup, + "Top.Internal", + TopologyView::Collapsed, + ) + .unwrap(); + assert!(d.nodes.is_empty()); + assert_eq!(d.topology_nodes.len(), 1); + assert_eq!(d.topology_nodes[0].id, "Sub.Subtopology"); + assert!(d.edges.is_empty()); + + let flattened = + fpp_diagram::lower(a, DiagramKind::ConnectionGroup, "Top.Internal").unwrap(); + assert_eq!(node_ids(&flattened), ["Sub.inner", "Sub.src"]); + assert_eq!(flattened.edges.len(), 1); + }); +} + +/// `Top` also declares `Sub.inner` itself, so the instance stays a node of its +/// own in the collapsed view and the subtopology's wiring to it must show. +const REUSED_INSTANCE_MODEL: &str = r#" +port P + +passive component Src { + output port out: P +} + +passive component Sink { + sync input port in: P + sync input port in2: P +} + +module Sub { + instance src: Src base id 0x100 + instance inner: Sink base id 0x200 + + topology Subtopology { + instance src + instance inner + + port dataIn = inner.in + + connections Internal { + src.out -> inner.in2 + } + } +} + +instance outer: Src base id 0x300 + +topology Top { + instance outer + instance Sub.inner + instance Sub.Subtopology + + connections C { + outer.out -> Sub.Subtopology.dataIn + } +} +"#; + +#[test] +fn collapsed_view_wires_an_instance_the_topology_declares_itself() { + with_analysis(REUSED_INSTANCE_MODEL, |a| { + let d = fpp_diagram::lower_view(a, DiagramKind::Topology, "Top", TopologyView::Collapsed) + .unwrap(); + assert_eq!(node_ids(&d), ["Sub.inner", "outer"]); + assert_eq!(d.topology_nodes.len(), 1); + + // The imported connection crosses out of the collapsed node into the + // visible instance. + let e = edge(&d, "Sub.Subtopology", "Sub.inner.in2.0"); + assert!(e.implicit); + assert_eq!(e.detail, "Sub.src.out -> Sub.inner.in2"); + // `dataIn` aliases a visible instance's port; the declared port is still + // the way `Top` reaches it. + let e = edge(&d, "outer.out.0", "Sub.Subtopology.dataIn.0"); + assert!(!e.implicit); + assert_eq!(d.edges.len(), 2); + }); +} + +/// `Parent` imports two unrelated topologies `A` and `B` that both contain +/// `shared`, plus `Mid` and the `Inner` it imports in turn. +const SHARED_AND_NESTED_MODEL: &str = r#" +port P + +passive component Src { + output port out: P +} + +passive component Sink { + sync input port in: P +} + +instance shared: Sink base id 0x100 +instance a: Src base id 0x200 +instance b: Src base id 0x300 +instance mid: Src base id 0x400 +instance deep: Sink base id 0x500 +instance top: Src base id 0x600 + +topology A { + instance a + instance shared + connections A { a.out -> shared.in } +} + +topology B { + instance b + instance shared + connections B { b.out -> shared.in } +} + +topology Inner { + instance deep +} + +topology Mid { + instance mid + import Inner + connections M { mid.out -> deep.in } +} + +topology Parent { + instance top + import A + import B + import Mid + import Inner + connections C { top.out -> deep.in } +} +"#; + +#[test] +fn collapsed_view_resolves_shared_and_nested_imports() { + with_analysis(SHARED_AND_NESTED_MODEL, |a| { + let d = + fpp_diagram::lower_view(a, DiagramKind::Topology, "Parent", TopologyView::Collapsed) + .unwrap(); + let topology_ids: Vec<&str> = d.topology_nodes.iter().map(|n| n.id.as_str()).collect(); + assert_eq!(topology_ids, ["A", "B", "Inner", "Mid"]); + + // `shared` belongs to two unrelated imports, so it is drawn on its own + // and both reach it. + assert_eq!(node_ids(&d), ["shared", "top"]); + let e = edge(&d, "A", "shared.in.0"); + assert!(e.implicit); + assert_eq!(e.detail, "a.out -> shared.in"); + let e = edge(&d, "B", "shared.in.0"); + assert!(e.implicit); + + // `deep` belongs to the innermost import containing it, `Inner`, not to + // `Mid`; `Mid`'s wiring to it crosses between the two collapsed nodes. + let e = edge(&d, "Mid", "Inner"); + assert!(e.implicit); + assert_eq!(e.detail, "mid.out -> deep.in"); + let e = edge(&d, "top.out.0", "Inner"); + assert!(e.implicit); + assert_eq!(d.edges.len(), 4); + }); +} From 4594da4b840ec745764346b0d3019e6ca350ece4 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 29 Sep 2026 20:50:52 +0000 Subject: [PATCH 3/8] Collapsed view: join a collapsed node's declared port to what it aliases outside the node A declared port of an imported topology that names an instance drawn on its own (shared by unrelated imports, or declared by the parent) or owned by another collapsed node gets an alias edge to it, mirroring the boundary-port edges. Group views bring in those alias targets for the collapsed nodes they draw. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> --- fpp_diagram/src/lower.rs | 68 +++++++++++++++++++++++++++++++--- fpp_diagram/tests/collapsed.rs | 42 ++++++++++++++++++--- 2 files changed, 99 insertions(+), 11 deletions(-) diff --git a/fpp_diagram/src/lower.rs b/fpp_diagram/src/lower.rs index 8f470889..82d3c817 100644 --- a/fpp_diagram/src/lower.rs +++ b/fpp_diagram/src/lower.rs @@ -96,6 +96,9 @@ pub fn lower_topology_view( diagram.nodes = scope.component_nodes(); diagram.topology_nodes = scope.topology_nodes(); diagram.edges = scope.edges(None); + diagram + .edges + .extend(scope.import_alias_edges().into_iter().map(|(_, e)| e)); let (ports, edges) = scope.boundary(); diagram.boundary_ports = ports; diagram.edges.extend(edges); @@ -157,7 +160,18 @@ pub fn lower_connection_group_view( } TopologyView::Collapsed => { let scope = CollapsedScope::new(a, topology); - let touched = scope.touched(Some(group)); + let mut touched = scope.touched(Some(group)); + // A declared port of a drawn collapsed node brings in what it aliases. + let alias_edges: Vec<Edge> = scope + .import_alias_edges() + .into_iter() + .filter(|(import, _)| touched.contains(import)) + .map(|(_, e)| e) + .collect(); + for e in &alias_edges { + touched.insert(scope.node_of(&e.from_port).to_string()); + touched.insert(scope.node_of(&e.to_port).to_string()); + } let nodes = scope .component_nodes() .into_iter() @@ -168,7 +182,9 @@ pub fn lower_connection_group_view( .into_iter() .filter(|n| touched.contains(&n.id)) .collect(); - (scope.edges(Some(group)), nodes, topology_nodes) + let mut edges = scope.edges(Some(group)); + edges.extend(alias_edges); + (edges, nodes, topology_nodes) } }; @@ -540,6 +556,16 @@ impl<'a> CollapsedScope<'a> { } } + /// The node an edge end belongs to: the end is either a collapsed node id + /// or a port id. + fn node_of<'b>(&self, end: &'b str) -> &'b str { + if self.imported.contains_key(end) { + end + } else { + node_id_of_port(end) + } + } + /// The connection graphs to draw: all of them, or just `only_group`. fn graphs( &self, @@ -658,16 +684,46 @@ impl<'a> CollapsedScope<'a> { /// The topology's own declared ports, drawn on the diagram boundary, and /// the edges joining each to the port it aliases. fn boundary(&self) -> (Vec<Port>, Vec<Edge>) { - let node_id = &self.topology.qualified_name; + self.alias_edges(self.topology, &self.topology.qualified_name, "boundary") + } + + /// Edges joining a declared port of a collapsed node to what it aliases + /// when that lies outside the node: an instance drawn on its own, or one + /// owned by another collapsed node. Each is paired with the collapsed + /// node's id. + fn import_alias_edges(&self) -> Vec<(String, Edge)> { + self.imported + .iter() + .flat_map(|(id, t)| { + self.alias_edges(t, id, &format!("alias.{id}")) + .1 + .into_iter() + .map(move |e| (id.clone(), e)) + }) + .collect() + } + + /// `topology`'s declared ports as ports of the node `node_id`, and an edge + /// from each to the port it aliases unless that lies inside the same node. + fn alias_edges( + &self, + topology: &Topology, + node_id: &str, + id_prefix: &str, + ) -> (Vec<Port>, Vec<Edge>) { let mut ports = Vec::new(); let mut edges = Vec::new(); - for port in declared_ports(self.a, self.topology, node_id) { - let Some(tp) = self.topology.port_map.get(&port.name) else { + for port in declared_ports(self.a, topology, node_id) { + let Some(tp) = topology.port_map.get(&port.name) else { continue; }; let Some(inner) = self.resolve_end(&tp.pii, port.index) else { continue; }; + if inner.id() == node_id { + ports.push(port); + continue; + } let implicit = matches!(inner, End::Node(_)); let detail = if implicit { underlying_label(self.a, tp.pii.clone(), port.index) @@ -679,7 +735,7 @@ impl<'a> CollapsedScope<'a> { ir::Direction::Output => (inner.id().to_string(), port.id.clone()), }; edges.push(Edge { - id: format!("boundary.{}.{}", port.name, port.index), + id: format!("{id_prefix}.{}.{}", port.name, port.index), from_port, to_port, graph_name: String::new(), diff --git a/fpp_diagram/tests/collapsed.rs b/fpp_diagram/tests/collapsed.rs index 9b94aca7..35c6d8ab 100644 --- a/fpp_diagram/tests/collapsed.rs +++ b/fpp_diagram/tests/collapsed.rs @@ -493,11 +493,14 @@ fn collapsed_view_wires_an_instance_the_topology_declares_itself() { let e = edge(&d, "Sub.Subtopology", "Sub.inner.in2.0"); assert!(e.implicit); assert_eq!(e.detail, "Sub.src.out -> Sub.inner.in2"); - // `dataIn` aliases a visible instance's port; the declared port is still - // the way `Top` reaches it. + // `dataIn` aliases a visible instance's port: the declared port is still + // the way `Top` reaches it, and is joined to the port it names. let e = edge(&d, "outer.out.0", "Sub.Subtopology.dataIn.0"); assert!(!e.implicit); - assert_eq!(d.edges.len(), 2); + let e = edge(&d, "Sub.Subtopology.dataIn.0", "Sub.inner.in.0"); + assert_eq!(e.id, "alias.Sub.Subtopology.dataIn.0"); + assert!(!e.implicit); + assert_eq!(d.edges.len(), 3); }); } @@ -524,6 +527,7 @@ instance top: Src base id 0x600 topology A { instance a instance shared + port sharedIn = shared.in connections A { a.out -> shared.in } } @@ -540,6 +544,7 @@ topology Inner { topology Mid { instance mid import Inner + port deepIn = deep.in connections M { mid.out -> deep.in } } @@ -549,7 +554,10 @@ topology Parent { import B import Mid import Inner - connections C { top.out -> deep.in } + connections C { + top.out -> deep.in + top.out -> A.sharedIn + } } "#; @@ -578,6 +586,30 @@ fn collapsed_view_resolves_shared_and_nested_imports() { assert_eq!(e.detail, "mid.out -> deep.in"); let e = edge(&d, "top.out.0", "Inner"); assert!(e.implicit); - assert_eq!(d.edges.len(), 4); + + // A declared port aliasing something outside its own node is joined to + // it: `A.sharedIn` to the visible `shared`, `Mid.deepIn` to `Inner`. + let e = edge(&d, "top.out.0", "A.sharedIn.0"); + assert!(!e.implicit); + let e = edge(&d, "A.sharedIn.0", "shared.in.0"); + assert_eq!(e.id, "alias.A.sharedIn.0"); + assert!(!e.implicit); + let e = edge(&d, "Mid.deepIn.0", "Inner"); + assert!(e.implicit); + assert_eq!(e.detail, "deep.in"); + assert_eq!(d.edges.len(), 7); + + // A group view keeps such an alias edge only when both ends are drawn. + let g = fpp_diagram::lower_view( + a, + DiagramKind::ConnectionGroup, + "Parent.C", + TopologyView::Collapsed, + ) + .unwrap(); + assert_eq!(node_ids(&g), ["shared", "top"]); + edge(&g, "A.sharedIn.0", "shared.in.0"); + assert!(g.edges.iter().all(|e| e.id != "alias.Mid.deepIn.0")); + assert_eq!(g.edges.len(), 3); }); } From 49bc2bf528b941ab6a20db59010c0ccfa6f9ce83 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 29 Sep 2026 20:52:16 +0000 Subject: [PATCH 4/8] Use the full ELK option id for boundary layer constraints 'elk.layered.layerConstraint' is not an ELK option; boundary ports were laid out as ordinary nodes next to what they connect to. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> --- editors/code/src/diagram/layout-config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editors/code/src/diagram/layout-config.ts b/editors/code/src/diagram/layout-config.ts index 00d790de..c9d4228e 100644 --- a/editors/code/src/diagram/layout-config.ts +++ b/editors/code/src/diagram/layout-config.ts @@ -109,7 +109,7 @@ export class FppDiagramConfig extends DefaultLayoutConfigurator { // layer and outputs to the last so they sit on the diagram's edge. const isOutput = (snode as BoundarySNode).isOutput; return { - 'elk.layered.layerConstraint': isOutput ? 'LAST_SEPARATE' : 'FIRST_SEPARATE', + 'elk.layered.layering.layerConstraint': isOutput ? 'LAST_SEPARATE' : 'FIRST_SEPARATE', 'elk.nodeLabels.placement': 'INSIDE, H_CENTER, V_CENTER', 'elk.nodeSize.constraints': 'NODE_LABELS, MINIMUM_SIZE', 'elk.nodeSize.minimum': '(40, 22)', From 3ebbe29267eb61551a42e404d7e018cb65570a1b Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 29 Sep 2026 20:54:12 +0000 Subject: [PATCH 5/8] Collapsed group view: follow alias chains across collapsed nodes A collapsed node brought in by an alias edge contributes its own alias edges in turn, so a group reaching a re-exported port shows the whole chain. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> --- fpp_diagram/src/lower.rs | 27 ++++++++----- fpp_diagram/tests/collapsed.rs | 70 ++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 10 deletions(-) diff --git a/fpp_diagram/src/lower.rs b/fpp_diagram/src/lower.rs index 82d3c817..d19be8c0 100644 --- a/fpp_diagram/src/lower.rs +++ b/fpp_diagram/src/lower.rs @@ -161,16 +161,23 @@ pub fn lower_connection_group_view( TopologyView::Collapsed => { let scope = CollapsedScope::new(a, topology); let mut touched = scope.touched(Some(group)); - // A declared port of a drawn collapsed node brings in what it aliases. - let alias_edges: Vec<Edge> = scope - .import_alias_edges() - .into_iter() - .filter(|(import, _)| touched.contains(import)) - .map(|(_, e)| e) - .collect(); - for e in &alias_edges { - touched.insert(scope.node_of(&e.from_port).to_string()); - touched.insert(scope.node_of(&e.to_port).to_string()); + // A declared port of a drawn collapsed node brings in what it + // aliases, which may be another collapsed node with aliases of its own. + let mut pending = scope.import_alias_edges(); + let mut alias_edges: Vec<Edge> = Vec::new(); + loop { + let (drawn, rest): (Vec<_>, Vec<_>) = pending + .into_iter() + .partition(|(import, _)| touched.contains(import)); + if drawn.is_empty() { + break; + } + for (_, e) in drawn { + touched.insert(scope.node_of(&e.from_port).to_string()); + touched.insert(scope.node_of(&e.to_port).to_string()); + alias_edges.push(e); + } + pending = rest; } let nodes = scope .component_nodes() diff --git a/fpp_diagram/tests/collapsed.rs b/fpp_diagram/tests/collapsed.rs index 35c6d8ab..b409fcd5 100644 --- a/fpp_diagram/tests/collapsed.rs +++ b/fpp_diagram/tests/collapsed.rs @@ -613,3 +613,73 @@ fn collapsed_view_resolves_shared_and_nested_imports() { assert_eq!(g.edges.len(), 3); }); } + +/// `A` re-exports a port of the `B` it imports, which in turn names `shared`; +/// `Parent` imports both and declares `shared` itself. +const CHAINED_ALIAS_MODEL: &str = r#" +port P + +passive component Src { + output port out: P +} + +passive component Sink { + sync input port in: P +} + +instance shared: Sink base id 0x100 +instance source: Src base id 0x200 +instance other: Src base id 0x300 + +topology B { + instance shared + port in = shared.in +} + +topology A { + import B + port in = B.in +} + +topology Parent { + instance source + instance other + instance shared + import A + import B + connections C { source.out -> A.in } + connections D { other.out -> shared.in } +} +"#; + +#[test] +fn collapsed_connection_group_follows_alias_chains() { + with_analysis(CHAINED_ALIAS_MODEL, |a| { + let g = fpp_diagram::lower_view( + a, + DiagramKind::ConnectionGroup, + "Parent.C", + TopologyView::Collapsed, + ) + .unwrap(); + let topology_ids: Vec<&str> = g.topology_nodes.iter().map(|n| n.id.as_str()).collect(); + assert_eq!(topology_ids, ["A", "B"]); + assert_eq!(node_ids(&g), ["shared", "source"]); + edge(&g, "source.out.0", "A.in.0"); + edge(&g, "A.in.0", "B.in.0"); + edge(&g, "B.in.0", "shared.in.0"); + assert_eq!(g.edges.len(), 3); + + // A group not touching the imports draws none of the chain. + let g = fpp_diagram::lower_view( + a, + DiagramKind::ConnectionGroup, + "Parent.D", + TopologyView::Collapsed, + ) + .unwrap(); + assert!(g.topology_nodes.is_empty()); + assert_eq!(node_ids(&g), ["other", "shared"]); + assert_eq!(g.edges.len(), 1); + }); +} From 2fe518e19219fca9072626f408910f197b40ecfa Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 29 Sep 2026 20:56:25 +0000 Subject: [PATCH 6/8] Lay out diagrams with boundary ports as a single piece ELK packs connected components separately by default, so first/last layer constraints only lined boundary ports up per component. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> --- editors/code/src/diagram/layout-config.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/editors/code/src/diagram/layout-config.ts b/editors/code/src/diagram/layout-config.ts index c9d4228e..64f30c8c 100644 --- a/editors/code/src/diagram/layout-config.ts +++ b/editors/code/src/diagram/layout-config.ts @@ -58,8 +58,13 @@ export class FppDiagramConfig extends DefaultLayoutConfigurator { 'elk.layered.selfLoopDistribution': 'EQUALLY', }; } + // Boundary ports are pinned to the first/last layer, which only lines + // them up along the diagram's edges if ELK lays the graph out as one + // piece rather than packing each connected component separately. + const hasBoundary = sgraph.children.some(c => c.type === 'node:boundary'); return { 'elk.algorithm': 'layered', + ...(hasBoundary ? { 'elk.separateConnectedComponents': 'false' } : {}), // Apply some spacing at the graph level to ensure the layered algorithm picks it up. 'elk.spacing.labelPortHorizontal': '5', 'elk.spacing.portPort': '10', From cd53e58eb1396c10cd3c04b41f02378cb205ca29 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 30 Sep 2026 19:59:11 +0000 Subject: [PATCH 7/8] Bus notation for parallel wires; double-click opens collapsed subtopologies Diagram::bundle_edges folds edges joining the same two rendered elements (component, collapsed topology, or boundary port) into one edge with a wire count, listing the wires in its hover detail. The sprotty model carries the count and a '/N' edge label; the webview draws buses as a fat wire with a slash. Bundling is opt-in: --bundle-edges on the CLI, bundleEdges on fpp/diagram, and on by default in VS Code with a toggle command. node:topology now carries qualifiedName and is openable: double-clicking sends OpenAction, which the extension host turns into displayDiagram for that topology. lower_to_smodel_view (added on this branch) is replaced by lower_to_smodel_with(SmodelOptions). Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> --- editors/code/common/models.ts | 6 + editors/code/package.json | 11 + editors/code/src/diagram/index.ts | 5 + editors/code/src/diagram/layout-config.ts | 1 + editors/code/src/diagram/manager.ts | 21 +- editors/code/src/lsp_ext.ts | 5 + editors/code/webview/css/diagram.css | 18 ++ editors/code/webview/src/di.config.ts | 7 +- editors/code/webview/src/views.tsx | 55 +++- fpp_diagram/src/bin/fpp-diagram.rs | 43 ++- fpp_diagram/src/ir.rs | 108 +++++++ fpp_diagram/src/lib.rs | 46 ++- fpp_diagram/src/lower.rs | 25 +- fpp_diagram/src/sprotty.rs | 62 +++- fpp_diagram/tests/bundle.rs | 360 ++++++++++++++++++++++ fpp_diagram/tests/collapsed.rs | 30 +- fpp_lsp_server/src/handlers.rs | 11 +- fpp_lsp_server/src/lsp_ext.rs | 5 + 18 files changed, 734 insertions(+), 85 deletions(-) create mode 100644 fpp_diagram/tests/bundle.rs diff --git a/editors/code/common/models.ts b/editors/code/common/models.ts index 36c7b522..da282193 100644 --- a/editors/code/common/models.ts +++ b/editors/code/common/models.ts @@ -17,9 +17,15 @@ export interface BoundarySNode extends SNode { isOutput: boolean, // Outputs are pinned to the last ELK layer, inputs to the first. } +/** A collapsed imported topology; activating it opens that topology's diagram. */ +export interface TopologySNode extends SNode { + qualifiedName: string +} + export interface FppSEdge extends SEdge { implicit?: boolean, // Ends on a collapsed topology's boundary rather than a rendered port. detail?: string, // Hover text. + count?: number, // Wires the edge stands for; more than one makes it a bus. Absent means 1. } export interface PortSNode extends SPort { diff --git a/editors/code/package.json b/editors/code/package.json index 98e5af1a..f7356161 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -95,6 +95,12 @@ "icon": "$(group-by-ref-type)", "category": "FPP Diagram" }, + { + "command": "fpp.diagram.toggle-bus-wires", + "title": "FPP: Toggle Bus Wires", + "icon": "$(combine)", + "category": "FPP Diagram" + }, { "command": "fpp.stateMachine.fit", "title": "FPP: Fit State Machine Diagram", @@ -189,6 +195,11 @@ "when": "activeWebviewPanelId == 'fppDiagrams'", "group": "navigation" }, + { + "command": "fpp.diagram.toggle-bus-wires", + "when": "activeWebviewPanelId == 'fppDiagrams'", + "group": "navigation" + }, { "command": "fpp.stateMachine.fit", "when": "activeWebviewPanelId == 'fppStateMachineDiagram'", diff --git a/editors/code/src/diagram/index.ts b/editors/code/src/diagram/index.ts index 12c6d0c2..23f4bd86 100644 --- a/editors/code/src/diagram/index.ts +++ b/editors/code/src/diagram/index.ts @@ -113,6 +113,11 @@ export function registerDiagramSupport( !webviewPanelManager.diagramConfig.collapseSubtopologies; void webviewPanelManager.updateDiagram(); }), + vscode.commands.registerCommand("fpp.diagram.toggle-bus-wires", () => { + webviewPanelManager.diagramConfig.bundleEdges = + !webviewPanelManager.diagramConfig.bundleEdges; + void webviewPanelManager.updateDiagram(); + }), vscode.commands.registerCommand("fpp.stateMachine.fit", () => mermaidPanel.fit()), vscode.commands.registerCommand("fpp.stateMachine.export", () => mermaidPanel.export()), vscode.commands.registerCommand("fpp.stateMachine.view-source", () => mermaidPanel.viewSource()), diff --git a/editors/code/src/diagram/layout-config.ts b/editors/code/src/diagram/layout-config.ts index 64f30c8c..1bfdd46b 100644 --- a/editors/code/src/diagram/layout-config.ts +++ b/editors/code/src/diagram/layout-config.ts @@ -22,6 +22,7 @@ export class FppDiagramConfig extends DefaultLayoutConfigurator { // Stateful diagram options public hideUnusedPorts = true; // By default, hide unused ports. public collapseSubtopologies = false; // By default, flatten imported topologies into the diagram. + public bundleEdges = true; // By default, draw parallel wires as one counted bus. public currentDiagramType: DiagramType | undefined; // Current diagram type public fullyQualifiedName: string = ""; // Fully qualified name of the element currently displayed diff --git a/editors/code/src/diagram/manager.ts b/editors/code/src/diagram/manager.ts index a98f8216..8971a7a5 100644 --- a/editors/code/src/diagram/manager.ts +++ b/editors/code/src/diagram/manager.ts @@ -15,7 +15,7 @@ * rendering) is unchanged. */ import { createWebviewPanel, SprottyDiagramIdentifier, WebviewEndpoint, WebviewPanelManager, WebviewPanelManagerOptions } from "sprotty-vscode"; -import { RequestModelAction, ComputedBoundsAction, UpdateModelAction, FitToScreenAction, SGraph, RequestBoundsAction, applyBounds, SelectAllAction } from 'sprotty-protocol'; +import { RequestModelAction, ComputedBoundsAction, UpdateModelAction, FitToScreenAction, SGraph, RequestBoundsAction, applyBounds, SelectAllAction, OpenAction } from 'sprotty-protocol'; import * as vscode from "vscode"; import { LanguageClient } from "vscode-languageclient/node"; import { ElkLayoutEngine } from "sprotty-elk/lib/elk-layout"; @@ -23,6 +23,7 @@ import ELK from 'elkjs/lib/elk-api.js'; import { FppLayoutEngine } from "./layout"; import { FppDiagramConfig, DiagramType } from "./layout-config"; import * as lsp_ext from "../lsp_ext"; +import { TopologySNode } from "../../common/models"; /** A provider of the current language client, since the client is recreated on restart. */ export type ClientProvider = () => LanguageClient | undefined; @@ -77,6 +78,7 @@ export class FppWebviewPanelManager extends WebviewPanelManager { name: this.diagramConfig.fullyQualifiedName, hideUnusedPorts: this.diagramConfig.hideUnusedPorts, topologyView: this.diagramConfig.collapseSubtopologies ? "collapsed" : "flattened", + bundleEdges: this.diagramConfig.bundleEdges, }); return model as SGraph; } catch (e) { @@ -92,6 +94,7 @@ export class FppWebviewPanelManager extends WebviewPanelManager { const activeWebview = super.createEndpoint(identifier); this.addRequestModelHandler(activeWebview); this.addComputedBoundsHandler(activeWebview); + this.addOpenHandler(activeWebview); return activeWebview; } @@ -156,6 +159,22 @@ export class FppWebviewPanelManager extends WebviewPanelManager { endpoint.addActionHandler(ComputedBoundsAction.KIND, handler); } + /** + * The webview sends an `OpenAction` when the user double-clicks an element + * that can be opened. A collapsed topology node opens the diagram of the + * topology it stands for, replacing the current one in the same panel. + */ + protected addOpenHandler(endpoint: WebviewEndpoint) { + const handler = async (action: OpenAction) => { + const element = this.sGraph?.children.find(c => c.id === action.elementId); + if (element?.type !== 'node:topology') { + return; + } + await this.displayDiagram(DiagramType.topology, (element as TopologySNode).qualifiedName); + }; + endpoint.addActionHandler(OpenAction.KIND, handler); + } + /**************************************************************************/ /* Handlers for sending messages to webview upon user's actions in editor */ /**************************************************************************/ diff --git a/editors/code/src/lsp_ext.ts b/editors/code/src/lsp_ext.ts index 4202dd17..66fc164a 100644 --- a/editors/code/src/lsp_ext.ts +++ b/editors/code/src/lsp_ext.ts @@ -31,6 +31,11 @@ export type DiagramParams = { * ports. Defaults to `"flattened"` when omitted. */ topologyView?: "flattened" | "collapsed", + /** + * When true, parallel wires between the same two elements are bundled into + * one bus edge carrying a wire count. Defaults to false when omitted. + */ + bundleEdges?: boolean, }; /** diff --git a/editors/code/webview/css/diagram.css b/editors/code/webview/css/diagram.css index c9172d88..eb18f6f2 100644 --- a/editors/code/webview/css/diagram.css +++ b/editors/code/webview/css/diagram.css @@ -107,6 +107,24 @@ tspan { opacity: 0.85; } +/* A bus: several parallel wires drawn as one fat wire with a slash and a count. */ +.sprotty-edge path.edge-bus { + stroke-width: 3px; +} + +.sprotty-edge path.edge-bus.edge-implicit { + stroke-dasharray: 8 5; +} + +.sprotty-edge .edge-bus-slash { + stroke-width: 1.5px; +} + +/* A collapsed topology opens on double-click. */ +.topology-node { + cursor: pointer; +} + .port-sync { /* Red expresses the notion of "caution", because the component is synchronous. */ fill: #fe92a8; diff --git a/editors/code/webview/src/di.config.ts b/editors/code/webview/src/di.config.ts index e40b3720..0bb5f118 100644 --- a/editors/code/webview/src/di.config.ts +++ b/editors/code/webview/src/di.config.ts @@ -2,7 +2,7 @@ import 'sprotty/css/sprotty.css'; import '../css/diagram.css'; import { Container, ContainerModule } from 'inversify'; -import { boundsFeature, configureModelElement, configureViewerOptions, editFeature, layoutableChildFeature, loadDefaultModules, moveFeature, overrideViewerOptions, SEdgeImpl, SGraphImpl, SGraphView, SLabelImpl, SLabelView, SNodeImpl, SPortImpl, SRoutingHandleImpl, SRoutingHandleView } from 'sprotty'; +import { boundsFeature, configureModelElement, configureViewerOptions, editFeature, layoutableChildFeature, loadDefaultModules, moveFeature, openFeature, overrideViewerOptions, SEdgeImpl, SGraphImpl, SGraphView, SLabelImpl, SLabelView, SNodeImpl, SPortImpl, SRoutingHandleImpl, SRoutingHandleView } from 'sprotty'; import { ArrowEdgeView, BoundaryNodeView, ChoiceNodeView, ComponentNodeView, InitialNodeView, MultiLineLabelView, RectanglePortView, StateNodeView, TopologyNodeView } from './views'; const myModule = new ContainerModule((bind, unbind, isBound, rebind) => { @@ -11,13 +11,14 @@ const myModule = new ContainerModule((bind, unbind, isBound, rebind) => { // Component / topology (port-based) diagrams. configureModelElement(context, 'node:component', SNodeImpl, ComponentNodeView, { enable: [layoutableChildFeature], disable: [moveFeature] }); - // Collapsed topology view: imported topologies as one node, own ports on the boundary. - configureModelElement(context, 'node:topology', SNodeImpl, TopologyNodeView, { enable: [layoutableChildFeature], disable: [moveFeature] }); + // Collapsed topology view: imported topologies as one node (double-click opens it), own ports on the boundary. + configureModelElement(context, 'node:topology', SNodeImpl, TopologyNodeView, { enable: [layoutableChildFeature, openFeature], disable: [moveFeature] }); configureModelElement(context, 'node:boundary', SNodeImpl, BoundaryNodeView, { enable: [layoutableChildFeature], disable: [moveFeature] }); configureModelElement(context, 'label:node:component', SLabelImpl, SLabelView); configureModelElement(context, 'port', SPortImpl, RectanglePortView, { disable: [boundsFeature] }); // By default, boundsFeature is enabled for SPort. We want fixed-size ports so this feature needs to be disabled. configureModelElement(context, 'label:port', SLabelImpl, SLabelView); configureModelElement(context, 'edge', SEdgeImpl, ArrowEdgeView, { disable: [editFeature] }); + configureModelElement(context, 'label:edge', SLabelImpl, SLabelView); // State machine diagrams. configureModelElement(context, 'node:state', SNodeImpl, StateNodeView, { enable: [layoutableChildFeature], disable: [moveFeature] }); diff --git a/editors/code/webview/src/views.tsx b/editors/code/webview/src/views.tsx index 1a5be053..88fb5d74 100644 --- a/editors/code/webview/src/views.tsx +++ b/editors/code/webview/src/views.tsx @@ -4,7 +4,7 @@ import { injectable } from 'inversify'; import { VNode } from 'snabbdom'; import { IView, IViewArgs, PolylineEdgeView, RenderingContext, SEdgeImpl, SLabelImpl, SLabelView, SNodeImpl, SPortImpl } from 'sprotty'; import { Point, Selectable } from 'sprotty-protocol'; -import { BoundarySNode, ComponentSNode, FppSEdge, PortSNode } from '../../common/models'; +import { BoundarySNode, ComponentSNode, FppSEdge, PortSNode, TopologySNode } from '../../common/models'; @injectable() export class ComponentNodeView implements IView { @@ -28,8 +28,9 @@ export class ComponentNodeView implements IView { /** A collapsed imported topology: a dashed box carrying its declared ports. */ @injectable() export class TopologyNodeView implements IView { - render(node: Readonly<SNodeImpl & Selectable>, context: RenderingContext): VNode { + render(node: Readonly<SNodeImpl & TopologySNode & Selectable>, context: RenderingContext): VNode { return <g> + <title>{`Double-click to open ${node.qualifiedName}`} 1; return {detail ? {detail} : null} + {bus ? renderBusSlash(segments) : null} ; } } +/** + * The slash of bus notation: a short stroke across the wire at its midpoint, + * leaning 60° off the wire's direction there. The wire count is a label placed + * by the edge layout beside the same point. + */ +function renderBusSlash(segments: Point[]): VNode | null { + const total = polylineLength(segments); + if (total === 0) { + return null; + } + let remaining = total / 2; + for (let i = 1; i < segments.length; i++) { + const a = segments[i - 1]; + const b = segments[i]; + const length = Math.hypot(b.x - a.x, b.y - a.y); + if (length < remaining && i < segments.length - 1) { + remaining -= length; + continue; + } + const t = length === 0 ? 0 : remaining / length; + const mid = { x: a.x + (b.x - a.x) * t, y: a.y + (b.y - a.y) * t }; + const dir = length === 0 ? { x: 1, y: 0 } : { x: (b.x - a.x) / length, y: (b.y - a.y) / length }; + const angle = Math.PI / 3; + const slash = { + x: dir.x * Math.cos(angle) - dir.y * Math.sin(angle), + y: dir.x * Math.sin(angle) + dir.y * Math.cos(angle), + }; + const half = 7; + return ; + } + return null; +} + +function polylineLength(segments: Point[]): number { + let total = 0; + for (let i = 1; i < segments.length; i++) { + total += Math.hypot(segments[i].x - segments[i - 1].x, segments[i].y - segments[i - 1].y); + } + return total; +} + // --- state machine views ---------------------------------------------------- /** diff --git a/fpp_diagram/src/bin/fpp-diagram.rs b/fpp_diagram/src/bin/fpp-diagram.rs index 67214628..f3d03716 100644 --- a/fpp_diagram/src/bin/fpp-diagram.rs +++ b/fpp_diagram/src/bin/fpp-diagram.rs @@ -9,7 +9,7 @@ use std::io::Read; use std::process::exit; use clap::{Parser, ValueEnum}; -use fpp_diagram::{DiagramKind, TopologyView, TransitionActionMode}; +use fpp_diagram::{DiagramKind, SmodelOptions, TopologyView, TransitionActionMode}; #[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum)] enum ModeArg { @@ -109,6 +109,11 @@ struct Args { #[arg(long, value_enum, default_value_t = ViewArg::Flattened)] view: ViewArg, + /// Bundle parallel wires between the same two elements into one bus edge + /// carrying a wire count (topology and connection-group diagrams only). + #[arg(long)] + bundle_edges: bool, + /// The FPP source file to read. Reads from stdin when omitted. file: Option, } @@ -166,35 +171,25 @@ fn main() { /// Lower the requested element to its output string. fn lower(a: &fpp_analysis::Analysis, args: &Args) -> Result { let kind: DiagramKind = args.kind.into(); - let mode: TransitionActionMode = args.mode.into(); - let view: TopologyView = args.view.into(); + let options = SmodelOptions { + hide_unused_ports: args.hide_unused_ports, + transition_action_mode: args.mode.into(), + topology_view: args.view.into(), + bundle_edges: args.bundle_edges, + }; match args.format { FormatArg::Mermaid if kind == DiagramKind::StateMachine => { - fpp_diagram::lower_state_machine_to_mermaid(a, &args.name, mode) - } - FormatArg::Mermaid => { - // Mermaid is only implemented for state machines; other kinds fall - // back to sprotty JSON. - let json = fpp_diagram::lower_to_smodel_view( + fpp_diagram::lower_state_machine_to_mermaid( a, - kind, &args.name, - args.hide_unused_ports, - mode, - view, - )?; - Ok(json.to_string()) + options.transition_action_mode, + ) } - FormatArg::Sprotty => { - let json = fpp_diagram::lower_to_smodel_view( - a, - kind, - &args.name, - args.hide_unused_ports, - mode, - view, - )?; + // Mermaid is only implemented for state machines; other kinds fall + // back to sprotty JSON. + FormatArg::Mermaid | FormatArg::Sprotty => { + let json = fpp_diagram::lower_to_smodel_with(a, kind, &args.name, options)?; Ok(json.to_string()) } } diff --git a/fpp_diagram/src/ir.rs b/fpp_diagram/src/ir.rs index ecd07ef7..a1c2d8e5 100644 --- a/fpp_diagram/src/ir.rs +++ b/fpp_diagram/src/ir.rs @@ -199,6 +199,18 @@ pub struct Edge { /// (one connection per line); empty when the endpoints already say so. #[serde(default, skip_serializing_if = "String::is_empty")] pub detail: String, + /// The number of connections the edge stands for: 1 for a single wire, more + /// for a bus (see [`Diagram::bundle_edges`]). + #[serde(default = "one", skip_serializing_if = "is_one")] + pub count: u32, +} + +fn one() -> u32 { + 1 +} + +fn is_one(n: &u32) -> bool { + *n == 1 } /// A complete diagram in intermediate form. @@ -225,6 +237,12 @@ impl Port { pub fn make_id(node_id: &str, port_name: &str, index: i128) -> String { format!("{node_id}.{port_name}.{index}") } + + /// Recover the node id from a port id built by [`Port::make_id`], i.e. + /// strip the final two dot-separated segments. + pub fn node_id(port_id: &str) -> &str { + port_id.rsplitn(3, '.').nth(2).unwrap_or(port_id) + } } impl Diagram { @@ -248,6 +266,96 @@ impl Diagram { node.ports.retain(|p| used.contains(p.id.as_str())); } } + + /// Bundle parallel wires into buses. + /// + /// Edges joining the same two elements in the same direction — an element + /// being a component node, a collapsed topology node, or a boundary port — + /// become one edge between the elements themselves. Its `count` is the + /// number of wires it stands for, its `detail` lists them one per line, and + /// it is `implicit` only if every wire was. An edge alone between its two + /// elements is left untouched, still ending on its ports. + pub fn bundle_edges(&mut self) { + let elements: rustc_hash::FxHashSet<&str> = self + .nodes + .iter() + .map(|n| n.id.as_str()) + .chain(self.topology_nodes.iter().map(|n| n.id.as_str())) + .chain(self.boundary_ports.iter().map(|p| p.id.as_str())) + .collect(); + fn element_of<'e>(elements: &rustc_hash::FxHashSet<&str>, end: &'e str) -> &'e str { + if elements.contains(end) { + end + } else { + Port::node_id(end) + } + } + + // `node.port[index]` for each rendered port, to name a bus's wires. + let port_names: rustc_hash::FxHashMap<&str, String> = self + .nodes + .iter() + .map(|n| (n.id.as_str(), &n.ports)) + .chain( + self.topology_nodes + .iter() + .map(|n| (n.id.as_str(), &n.ports)), + ) + .flat_map(|(node, ports)| { + ports + .iter() + .map(move |p| (p.id.as_str(), format!("{node}.{}", p.label))) + }) + .chain( + self.boundary_ports + .iter() + .map(|p| (p.id.as_str(), format!("{}.{}", self.name, p.label))), + ) + .collect(); + let wire = |e: &Edge| -> String { + if e.detail.is_empty() { + let name = |end: &str| port_names.get(end).cloned().unwrap_or(end.to_string()); + format!("{} -> {}", name(&e.from_port), name(&e.to_port)) + } else { + e.detail.clone() + } + }; + + let mut bundled: Vec = Vec::new(); + let mut by_elements: rustc_hash::FxHashMap<(String, String), usize> = + rustc_hash::FxHashMap::default(); + let mut buses: rustc_hash::FxHashSet = rustc_hash::FxHashSet::default(); + for edge in std::mem::take(&mut self.edges) { + let key = ( + element_of(&elements, &edge.from_port).to_string(), + element_of(&elements, &edge.to_port).to_string(), + ); + let Some(&i) = by_elements.get(&key) else { + by_elements.insert(key, bundled.len()); + bundled.push(edge); + continue; + }; + if buses.insert(i) { + let first = wire(&bundled[i]); + let bus = &mut bundled[i]; + bus.id = format!("bus.{}.{}", key.0, key.1); + bus.from_port = key.0; + bus.to_port = key.1; + bus.detail = first; + } + let line = wire(&edge); + let bus = &mut bundled[i]; + bus.detail.push('\n'); + bus.detail.push_str(&line); + bus.count += edge.count; + bus.unmatched &= edge.unmatched; + bus.implicit &= edge.implicit; + if bus.graph_name != edge.graph_name { + bus.graph_name.clear(); + } + } + self.edges = bundled; + } } // --- state machine diagrams ------------------------------------------------- diff --git a/fpp_diagram/src/lib.rs b/fpp_diagram/src/lib.rs index 75c72611..bc423aea 100644 --- a/fpp_diagram/src/lib.rs +++ b/fpp_diagram/src/lib.rs @@ -80,12 +80,28 @@ pub fn lower_state_machine_to_mermaid( Ok(mermaid::state_machine_to_mermaid(&diagram, mode)) } +/// Options for lowering an element to a sprotty model; see +/// [`lower_to_smodel_with`]. The default is the plain flattened view. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub struct SmodelOptions { + /// Prune ports not referenced by any connection (a no-op for component and + /// state machine diagrams). See [`Diagram::prune_unused_ports`]. + pub hide_unused_ports: bool, + /// How state machine transition actions are presented. + pub transition_action_mode: TransitionActionMode, + /// How topology and connection-group diagrams treat imported topologies. + pub topology_view: TopologyView, + /// Bundle parallel wires between the same two elements into one bus edge + /// carrying a wire count. See [`Diagram::bundle_edges`]. + pub bundle_edges: bool, +} + /// Lower an element directly to a sprotty `SModel` JSON value. /// /// When `hide_unused_ports` is set, ports not referenced by any connection are /// pruned (a no-op for component and state machine diagrams). See /// [`Diagram::prune_unused_ports`]. Topologies use the flattened view; see -/// [`lower_to_smodel_view`]. +/// [`lower_to_smodel_with`] for the other options. pub fn lower_to_smodel( a: &Analysis, kind: DiagramKind, @@ -93,32 +109,34 @@ pub fn lower_to_smodel( hide_unused_ports: bool, mode: TransitionActionMode, ) -> Result { - lower_to_smodel_view( + lower_to_smodel_with( a, kind, name, - hide_unused_ports, - mode, - TopologyView::Flattened, + SmodelOptions { + hide_unused_ports, + transition_action_mode: mode, + ..SmodelOptions::default() + }, ) } -/// Like [`lower_to_smodel`], with `view` selecting how topology and -/// connection-group diagrams treat imported topologies; see [`TopologyView`]. -pub fn lower_to_smodel_view( +/// Like [`lower_to_smodel`], with every option of [`SmodelOptions`]. +pub fn lower_to_smodel_with( a: &Analysis, kind: DiagramKind, name: &str, - hide_unused_ports: bool, - mode: TransitionActionMode, - view: TopologyView, + options: SmodelOptions, ) -> Result { if kind == DiagramKind::StateMachine { - let diagram = lower_state_machine(a, name, mode)?; + let diagram = lower_state_machine(a, name, options.transition_action_mode)?; return Ok(sprotty::state_machine_to_smodel_json(&diagram)); } - let mut diagram = lower_view(a, kind, name, view)?; - if hide_unused_ports { + let mut diagram = lower_view(a, kind, name, options.topology_view)?; + if options.bundle_edges { + diagram.bundle_edges(); + } + if options.hide_unused_ports { diagram.prune_unused_ports(); } Ok(sprotty::to_smodel_json(&diagram)) diff --git a/fpp_diagram/src/lower.rs b/fpp_diagram/src/lower.rs index d19be8c0..72e99efa 100644 --- a/fpp_diagram/src/lower.rs +++ b/fpp_diagram/src/lower.rs @@ -150,7 +150,7 @@ pub fn lower_connection_group_view( let edges = topology_edges(a, topology, Some(group)); let used_nodes: FxHashSet<&str> = edges .iter() - .flat_map(|e| [node_id_of_port(&e.from_port), node_id_of_port(&e.to_port)]) + .flat_map(|e| [Port::node_id(&e.from_port), Port::node_id(&e.to_port)]) .collect(); let nodes = instance_nodes(a, topology) .into_iter() @@ -205,21 +205,6 @@ pub fn lower_connection_group_view( }) } -/// Recover the node id from a port id (`..`), i.e. strip -/// the final two dot-separated segments. -fn node_id_of_port(port_id: &str) -> &str { - match ( - port_id.rfind('.'), - port_id.get(..port_id.rfind('.').unwrap_or(0)), - ) { - (Some(_), Some(head)) => match head.rfind('.') { - Some(cut) => &head[..cut], - None => head, - }, - _ => port_id, - } -} - // --- node construction ------------------------------------------------------ /// Build the single node for a component-definition diagram. @@ -388,6 +373,7 @@ fn connection_edge( unmatched: connection.is_unmatched, implicit: false, detail: String::new(), + count: 1, }) } @@ -569,7 +555,7 @@ impl<'a> CollapsedScope<'a> { if self.imported.contains_key(end) { end } else { - node_id_of_port(end) + Port::node_id(end) } } @@ -598,7 +584,7 @@ impl<'a> CollapsedScope<'a> { }) .flatten() .map(|end| match end { - End::Port(id) => node_id_of_port(&id).to_string(), + End::Port(id) => Port::node_id(&id).to_string(), End::Node(id) => id, }) .collect() @@ -629,6 +615,7 @@ impl<'a> CollapsedScope<'a> { detail.push_str(&edge.detail); } edges[i].unmatched &= edge.unmatched; + edges[i].count += edge.count; } None => { by_ends.insert(key, edges.len()); @@ -685,6 +672,7 @@ impl<'a> CollapsedScope<'a> { unmatched: connection.is_unmatched, implicit, detail, + count: 1, }) } @@ -749,6 +737,7 @@ impl<'a> CollapsedScope<'a> { unmatched: false, implicit, detail, + count: 1, }); ports.push(port); } diff --git a/fpp_diagram/src/sprotty.rs b/fpp_diagram/src/sprotty.rs index dcc7138d..c014ad75 100644 --- a/fpp_diagram/src/sprotty.rs +++ b/fpp_diagram/src/sprotty.rs @@ -85,6 +85,8 @@ pub enum SModelElement { #[serde(rename = "node:topology", rename_all = "camelCase")] TopologyNode { id: String, + /// FPP extra: the topology to open when the node is activated. + qualified_name: String, children: Vec, }, /// A declared port of the diagrammed topology, drawn on the diagram @@ -112,6 +114,22 @@ pub enum SModelElement { /// Hover text; empty if none. Rendered as an SVG ``. #[serde(default, skip_serializing_if = "String::is_empty")] detail: String, + /// FPP extra: the number of wires the edge stands for; more than one + /// makes it a bus, drawn fat with a `/count` label. + #[serde(default = "one", skip_serializing_if = "is_one")] + count: u32, + /// The bus label, if any. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + children: Vec<SModelElement>, + }, + /// The `/count` label of a bus edge. + #[serde(rename = "label:edge", rename_all = "camelCase")] + EdgeLabel { + id: String, + text: String, + size: Dimension, + /// Sprotty edge-label placement (0..1 along the edge). + edge_placement: EdgePlacement, }, // --- state machine elements --- @@ -165,6 +183,14 @@ pub enum SModelElement { }, } +fn one() -> u32 { + 1 +} + +fn is_one(n: &u32) -> bool { + *n == 1 +} + /// Placement of a label along an edge, matching sprotty's `EdgePlacement`. #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -179,13 +205,7 @@ pub fn to_smodel(diagram: &Diagram) -> SModelElement { let mut children: Vec<SModelElement> = diagram.nodes.iter().map(node_to_smodel).collect(); children.extend(diagram.topology_nodes.iter().map(topology_node_to_smodel)); children.extend(diagram.boundary_ports.iter().map(boundary_port_to_smodel)); - children.extend(diagram.edges.iter().map(|e| SModelElement::Edge { - id: e.id.clone(), - source_id: e.from_port.clone(), - target_id: e.to_port.clone(), - implicit: e.implicit, - detail: e.detail.clone(), - })); + children.extend(diagram.edges.iter().map(edge_to_smodel)); SModelElement::Graph { id: "root".to_string(), @@ -193,6 +213,33 @@ pub fn to_smodel(diagram: &Diagram) -> SModelElement { } } +/// Build an edge element; a bus (more than one wire) carries a `/count` label. +fn edge_to_smodel(edge: &crate::ir::Edge) -> SModelElement { + let mut children = Vec::new(); + if edge.count > 1 { + let text = format!("/{}", edge.count); + children.push(SModelElement::EdgeLabel { + id: format!("{}.label", edge.id), + size: text_size(&text), + text, + edge_placement: EdgePlacement { + position: 0.5, + side: "top".to_string(), + rotate: false, + }, + }); + } + SModelElement::Edge { + id: edge.id.clone(), + source_id: edge.from_port.clone(), + target_id: edge.to_port.clone(), + implicit: edge.implicit, + detail: edge.detail.clone(), + count: edge.count, + children, + } +} + /// Serialize a diagram IR directly to a sprotty `SModel` JSON value. pub fn to_smodel_json(diagram: &Diagram) -> serde_json::Value { serde_json::to_value(to_smodel(diagram)).expect("SModel serialization is infallible") @@ -438,6 +485,7 @@ fn topology_node_to_smodel(node: &TopologyNode) -> SModelElement { children.extend(node.ports.iter().map(port_to_smodel)); SModelElement::TopologyNode { id: node.id.clone(), + qualified_name: node.qualified_name.clone(), children, } } diff --git a/fpp_diagram/tests/bundle.rs b/fpp_diagram/tests/bundle.rs new file mode 100644 index 00000000..0c383ace --- /dev/null +++ b/fpp_diagram/tests/bundle.rs @@ -0,0 +1,360 @@ +//! Integration tests for bus notation: parallel wires between the same two +//! elements bundled into one counted edge. + +use fpp_analysis::{Analysis, add_state_enums, check_semantics}; +use fpp_core::SourceFile; +use fpp_diagram::SmodelOptions; +use fpp_diagram::ir::{Diagram, DiagramKind, Edge, TopologyView}; + +/// Parse `src` and run full semantic analysis, invoking `f` with the resulting +/// [`Analysis`] while the compiler context is installed. +fn with_analysis<R>(src: &str, f: impl FnOnce(&Analysis) -> R) -> R { + let mut sink = Vec::new(); + let mut ctx = fpp_core::CompilerContext::new(fpp_errors::WriteEmitter::new(&mut sink)); + fpp_core::run(&mut ctx, || { + let source = SourceFile::new("test.fpp", src.to_string()); + let mut ast = fpp_parser::parse(source, |p| p.trans_unit(), None); + add_state_enums(&mut ast); + let mut a = Analysis::new(); + let _ = check_semantics(&mut a, vec![&ast]); + f(&a) + }) +} + +/// `hub` fans four wires out to `sink` from two connection groups (a bus), one +/// wire back from `sink` to `hub` (the other direction), one wire to `lone` (no +/// bus), and reaches into the imported `Sub.Subtopology` twice without going +/// through a declared port plus once through its declared port. +const MODEL: &str = r#" +port P + +passive component Hub { + output port out: [4] P + output port aux: [4] P + sync input port back: P +} + +passive component Sink { + sync input port in: [3] P + sync input port extra: P + output port back: P +} + +module Sub { + instance inner: Sink base id 0x100 + topology Subtopology { + instance inner + port declared = inner.extra + } +} + +instance hub: Hub base id 0x200 +instance sink: Sink base id 0x300 +instance lone: Sink base id 0x400 + +topology Top { + instance hub + instance sink + instance lone + instance Sub.Subtopology + + connections Fan { + hub.out[0] -> sink.in[0] + hub.out[1] -> sink.in[1] + hub.out[2] -> sink.in[2] + sink.back -> hub.back + } + + connections Other { + hub.aux[0] -> lone.extra + hub.aux[1] -> sink.extra + hub.out[3] -> Sub.inner.in[0] + hub.aux[2] -> Sub.inner.in[1] + hub.aux[3] -> Sub.Subtopology.declared + } +} +"#; + +fn edge<'a>(d: &'a Diagram, from: &str, to: &str) -> &'a Edge { + d.edges + .iter() + .find(|e| e.from_port == from && e.to_port == to) + .unwrap_or_else(|| panic!("expected an edge {from} -> {to}; have {:?}", d.edges)) +} + +#[test] +fn bundling_is_opt_in_and_every_wire_starts_at_count_one() { + with_analysis(MODEL, |a| { + let d = fpp_diagram::lower(a, DiagramKind::Topology, "Top").unwrap(); + assert_eq!(d.edges.len(), 9); + assert!(d.edges.iter().all(|e| e.count == 1)); + // `count` is omitted from the wire format when it is 1. + let json = serde_json::to_value(&d).unwrap(); + assert!(json["edges"][0].get("count").is_none()); + let back: Diagram = serde_json::from_value(json).unwrap(); + assert_eq!(back, d); + }); +} + +#[test] +fn parallel_wires_bundle_into_one_counted_edge_between_nodes() { + with_analysis(MODEL, |a| { + let mut d = fpp_diagram::lower(a, DiagramKind::Topology, "Top").unwrap(); + d.bundle_edges(); + + // hub -> sink: four wires from two groups become one bus ending on + // the nodes. + let bus = edge(&d, "hub", "sink"); + assert_eq!(bus.count, 4); + assert_eq!(bus.id, "bus.hub.sink"); + assert_eq!(bus.graph_name, ""); + assert!(!bus.implicit); + assert_eq!( + bus.detail, + "hub.out[0] -> sink.in[0]\nhub.out[1] -> sink.in[1]\nhub.out[2] -> sink.in[2]\nhub.aux[1] -> sink.extra" + ); + + // The other direction is a separate, single wire still on its ports. + let back = edge(&d, "sink.back.0", "hub.back.0"); + assert_eq!(back.count, 1); + assert!(back.detail.is_empty()); + + // A lone wire keeps its ports. + edge(&d, "hub.aux.0", "lone.extra.0"); + + // hub -> Sub.inner (flattened): three wires, all from one group. + let bus = edge(&d, "hub", "Sub.inner"); + assert_eq!(bus.count, 3); + assert_eq!(bus.graph_name, "Other"); + + assert_eq!(d.edges.len(), 4); + assert_eq!(d.edges.iter().map(|e| e.count).sum::<u32>(), 9); + }); +} + +#[test] +fn bundling_then_pruning_drops_ports_only_the_bus_used() { + with_analysis(MODEL, |a| { + let mut d = fpp_diagram::lower(a, DiagramKind::Topology, "Top").unwrap(); + d.bundle_edges(); + d.prune_unused_ports(); + let ports = |node: &str| -> Vec<String> { + d.nodes + .iter() + .find(|n| n.id == node) + .unwrap() + .ports + .iter() + .map(|p| p.label.clone()) + .collect() + }; + assert_eq!(ports("hub"), ["aux[0]", "back"]); + assert_eq!(ports("sink"), ["back"]); + assert!(ports("Sub.inner").is_empty()); + }); +} + +#[test] +fn collapsed_view_counts_merged_implicit_wires_and_bundles_onto_the_node() { + with_analysis(MODEL, |a| { + let d = fpp_diagram::lower_view(a, DiagramKind::Topology, "Top", TopologyView::Collapsed) + .unwrap(); + // Two reach-ins from hub.out[3] and hub.aux are distinct ports, so they + // are two implicit edges before bundling, each of one wire. + let e = edge(&d, "hub.out.3", "Sub.Subtopology"); + assert_eq!((e.count, e.implicit), (1, true)); + let e = edge(&d, "hub.aux.2", "Sub.Subtopology"); + assert_eq!((e.count, e.implicit), (1, true)); + let e = edge(&d, "hub.aux.3", "Sub.Subtopology.declared.0"); + assert_eq!((e.count, e.implicit), (1, false)); + + let mut d = d; + d.bundle_edges(); + // The three wires into the collapsed node become one bus; since one of + // them lands on a declared port the bus is not implicit. + let bus = edge(&d, "hub", "Sub.Subtopology"); + assert_eq!(bus.count, 3); + assert!(!bus.implicit); + assert_eq!( + bus.detail, + "hub.out[3] -> Sub.inner.in[0]\nhub.aux[2] -> Sub.inner.in[1]\nhub.aux[3] -> Sub.Subtopology.declared" + ); + // Its declared port stays on the collapsed node even after pruning. + d.prune_unused_ports(); + assert_eq!(d.topology_nodes[0].ports.len(), 1); + }); +} + +#[test] +fn a_bus_of_only_implicit_wires_stays_implicit() { + with_analysis( + r#" +port P +passive component Hub { output port out: [2] P } +passive component Sink { sync input port in: [2] P } +module Sub { + instance inner: Sink base id 0x100 + topology Subtopology { instance inner } +} +instance hub: Hub base id 0x200 +topology Top { + instance hub + instance Sub.Subtopology + connections C { + hub.out[0] -> Sub.inner.in[0] + hub.out[1] -> Sub.inner.in[1] + } +} +"#, + |a| { + let mut d = + fpp_diagram::lower_view(a, DiagramKind::Topology, "Top", TopologyView::Collapsed) + .unwrap(); + d.bundle_edges(); + assert_eq!(d.edges.len(), 1); + let bus = &d.edges[0]; + assert_eq!( + (bus.from_port.as_str(), bus.to_port.as_str()), + ("hub", "Sub.Subtopology") + ); + assert_eq!(bus.count, 2); + assert!(bus.implicit); + }, + ); +} + +#[test] +fn sprotty_bus_edges_carry_count_and_a_slash_label() { + with_analysis(MODEL, |a| { + let model = fpp_diagram::lower_to_smodel_with( + a, + DiagramKind::Topology, + "Top", + SmodelOptions { + bundle_edges: true, + ..SmodelOptions::default() + }, + ) + .unwrap(); + let edges: Vec<&serde_json::Value> = model["children"] + .as_array() + .unwrap() + .iter() + .filter(|c| c["type"] == "edge") + .collect(); + assert_eq!(edges.len(), 4); + + let bus = edges + .iter() + .find(|e| e["sourceId"] == "hub" && e["targetId"] == "sink") + .unwrap(); + assert_eq!(bus["count"], 4); + assert_eq!(bus["detail"].as_str().unwrap().lines().count(), 4); + let label = &bus["children"][0]; + assert_eq!(label["type"], "label:edge"); + assert_eq!(label["text"], "/4"); + assert_eq!(label["edgePlacement"]["position"], 0.5); + + let single = edges + .iter() + .find(|e| e["sourceId"] == "sink.back.0") + .unwrap(); + assert!(single.get("count").is_none()); + assert!(single.get("children").is_none()); + + // Without the option nothing is bundled and the model is the legacy one. + let plain = fpp_diagram::lower_to_smodel_with( + a, + DiagramKind::Topology, + "Top", + SmodelOptions::default(), + ) + .unwrap(); + let legacy = fpp_diagram::lower_to_smodel( + a, + DiagramKind::Topology, + "Top", + false, + fpp_diagram::TransitionActionMode::Uml, + ) + .unwrap(); + assert_eq!(plain, legacy); + assert_eq!( + plain["children"] + .as_array() + .unwrap() + .iter() + .filter(|c| c["type"] == "edge") + .count(), + 9 + ); + }); +} + +#[test] +fn sprotty_topology_node_names_the_topology_to_open() { + with_analysis(MODEL, |a| { + let model = fpp_diagram::lower_to_smodel_with( + a, + DiagramKind::Topology, + "Top", + SmodelOptions { + topology_view: TopologyView::Collapsed, + ..SmodelOptions::default() + }, + ) + .unwrap(); + let node = model["children"] + .as_array() + .unwrap() + .iter() + .find(|c| c["type"] == "node:topology") + .unwrap(); + assert_eq!(node["qualifiedName"], "Sub.Subtopology"); + }); +} + +#[test] +fn cli_bundle_switch_bundles_wires() { + let dir = std::env::temp_dir().join(format!("fpp-diagram-bundle-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let file = dir.join("model.fpp"); + std::fs::write(&file, MODEL).unwrap(); + + let run = |extra: &[&str]| { + let out = std::process::Command::new(env!("CARGO_BIN_EXE_fpp-diagram")) + .args(["--kind", "topology", "--name", "Top", "--format", "sprotty"]) + .args(extra) + .arg(&file) + .output() + .expect("fpp-diagram runs"); + assert!( + out.status.success(), + "{}", + String::from_utf8_lossy(&out.stderr) + ); + serde_json::from_slice::<serde_json::Value>(&out.stdout).expect("sprotty JSON") + }; + let plain = run(&[]); + let bundled = run(&["--bundle-edges", "--view", "collapsed"]); + std::fs::remove_dir_all(&dir).unwrap(); + + let edges = |m: &serde_json::Value| -> usize { + m["children"] + .as_array() + .unwrap() + .iter() + .filter(|c| c["type"] == "edge") + .count() + }; + assert_eq!(edges(&plain), 9); + // hub->sink bus, sink->hub, hub->lone, hub->Sub.Subtopology bus. + assert_eq!(edges(&bundled), 4); + assert!( + bundled["children"] + .as_array() + .unwrap() + .iter() + .any(|c| c["type"] == "edge" && c["count"] == 4) + ); +} diff --git a/fpp_diagram/tests/collapsed.rs b/fpp_diagram/tests/collapsed.rs index b409fcd5..cbc103ca 100644 --- a/fpp_diagram/tests/collapsed.rs +++ b/fpp_diagram/tests/collapsed.rs @@ -270,13 +270,15 @@ fn collapsed_connection_group_keeps_participating_topology_nodes() { #[test] fn sprotty_model_carries_collapsed_elements() { with_analysis(MODEL, |a| { - let model = fpp_diagram::lower_to_smodel_view( + let model = fpp_diagram::lower_to_smodel_with( a, DiagramKind::Topology, "Top", - false, - fpp_diagram::TransitionActionMode::Uml, - TopologyView::Collapsed, + fpp_diagram::SmodelOptions { + hide_unused_ports: false, + topology_view: TopologyView::Collapsed, + ..fpp_diagram::SmodelOptions::default() + }, ) .unwrap(); let children = model["children"].as_array().unwrap(); @@ -309,13 +311,15 @@ fn sprotty_model_carries_collapsed_elements() { assert!(explicit.get("implicit").is_none()); assert!(explicit.get("detail").is_none()); - let sub_model = fpp_diagram::lower_to_smodel_view( + let sub_model = fpp_diagram::lower_to_smodel_with( a, DiagramKind::Topology, "Sub.Subtopology", - false, - fpp_diagram::TransitionActionMode::Uml, - TopologyView::Collapsed, + fpp_diagram::SmodelOptions { + hide_unused_ports: false, + topology_view: TopologyView::Collapsed, + ..fpp_diagram::SmodelOptions::default() + }, ) .unwrap(); let boundary: Vec<_> = sub_model["children"] @@ -345,13 +349,15 @@ fn sprotty_flattened_model_is_unchanged_by_the_view_parameter() { fpp_diagram::TransitionActionMode::Uml, ) .unwrap(); - let flattened = fpp_diagram::lower_to_smodel_view( + let flattened = fpp_diagram::lower_to_smodel_with( a, DiagramKind::Topology, "Top", - true, - fpp_diagram::TransitionActionMode::Uml, - TopologyView::Flattened, + fpp_diagram::SmodelOptions { + hide_unused_ports: true, + topology_view: TopologyView::Flattened, + ..fpp_diagram::SmodelOptions::default() + }, ) .unwrap(); assert_eq!(legacy, flattened); diff --git a/fpp_lsp_server/src/handlers.rs b/fpp_lsp_server/src/handlers.rs index 439b4d44..312e8f99 100644 --- a/fpp_lsp_server/src/handlers.rs +++ b/fpp_lsp_server/src/handlers.rs @@ -127,13 +127,16 @@ pub fn handle_diagram( .map(serde_json::Value::String) .map_err(|e| anyhow::anyhow!("{e}")); } - fpp_diagram::lower_to_smodel_view( + fpp_diagram::lower_to_smodel_with( &state.analysis, params.kind.into(), ¶ms.name, - params.hide_unused_ports, - params.transition_action_mode.into(), - params.topology_view.into(), + fpp_diagram::SmodelOptions { + hide_unused_ports: params.hide_unused_ports, + transition_action_mode: params.transition_action_mode.into(), + topology_view: params.topology_view.into(), + bundle_edges: params.bundle_edges, + }, ) .map_err(|e| anyhow::anyhow!("{e}")) } diff --git a/fpp_lsp_server/src/lsp_ext.rs b/fpp_lsp_server/src/lsp_ext.rs index 71486511..f6126db6 100644 --- a/fpp_lsp_server/src/lsp_ext.rs +++ b/fpp_lsp_server/src/lsp_ext.rs @@ -99,6 +99,11 @@ pub struct DiagramParams { /// [`TopologyView::Flattened`] when the client omits it. #[serde(default)] pub topology_view: TopologyView, + /// When true, parallel wires between the same two elements are bundled + /// into one bus edge carrying a wire count. Defaults to false when the + /// client omits it. + #[serde(default)] + pub bundle_edges: bool, } /// Mirrors [`fpp_diagram::TopologyView`] on the wire. From 1001d9ecac512de9e3fb4279f99793b88b888533 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 30 Sep 2026 20:08:48 +0000 Subject: [PATCH 8/8] Tighten doc comments on bus and open-action code Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> --- editors/code/src/diagram/manager.ts | 6 +----- editors/code/webview/src/views.tsx | 6 +----- fpp_diagram/src/ir.rs | 12 ++++-------- 3 files changed, 6 insertions(+), 18 deletions(-) diff --git a/editors/code/src/diagram/manager.ts b/editors/code/src/diagram/manager.ts index 8971a7a5..6f644486 100644 --- a/editors/code/src/diagram/manager.ts +++ b/editors/code/src/diagram/manager.ts @@ -159,11 +159,7 @@ export class FppWebviewPanelManager extends WebviewPanelManager { endpoint.addActionHandler(ComputedBoundsAction.KIND, handler); } - /** - * The webview sends an `OpenAction` when the user double-clicks an element - * that can be opened. A collapsed topology node opens the diagram of the - * topology it stands for, replacing the current one in the same panel. - */ + /** Double-clicking a collapsed topology node (`OpenAction`) shows that topology's diagram in the same panel. */ protected addOpenHandler(endpoint: WebviewEndpoint) { const handler = async (action: OpenAction) => { const element = this.sGraph?.children.find(c => c.id === action.elementId); diff --git a/editors/code/webview/src/views.tsx b/editors/code/webview/src/views.tsx index 88fb5d74..a1302b4a 100644 --- a/editors/code/webview/src/views.tsx +++ b/editors/code/webview/src/views.tsx @@ -150,11 +150,7 @@ export class ArrowEdgeView extends PolylineEdgeView { } } -/** - * The slash of bus notation: a short stroke across the wire at its midpoint, - * leaning 60° off the wire's direction there. The wire count is a label placed - * by the edge layout beside the same point. - */ +/** The bus slash: a short stroke across the wire at its midpoint, 60° off the wire's direction. */ function renderBusSlash(segments: Point[]): VNode | null { const total = polylineLength(segments); if (total === 0) { diff --git a/fpp_diagram/src/ir.rs b/fpp_diagram/src/ir.rs index a1c2d8e5..231e6a54 100644 --- a/fpp_diagram/src/ir.rs +++ b/fpp_diagram/src/ir.rs @@ -267,14 +267,10 @@ impl Diagram { } } - /// Bundle parallel wires into buses. - /// - /// Edges joining the same two elements in the same direction — an element - /// being a component node, a collapsed topology node, or a boundary port — - /// become one edge between the elements themselves. Its `count` is the - /// number of wires it stands for, its `detail` lists them one per line, and - /// it is `implicit` only if every wire was. An edge alone between its two - /// elements is left untouched, still ending on its ports. + /// Bundle parallel wires into buses: edges joining the same two elements + /// (node, topology node, or boundary port) in the same direction become one + /// edge between the elements, with `count` wires listed in `detail` and + /// `implicit` only if every wire was. A lone wire keeps its ports. pub fn bundle_edges(&mut self) { let elements: rustc_hash::FxHashSet<&str> = self .nodes