diff --git a/CHANGELOG.md b/CHANGELOG.md index dbf7b611d379..29b7332910d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ - [Fixed issue where drag'n'dropped files were not uploaded in cloud projects.][11014] - [Fixed files associations not properly registered on Windows][11030] +- [Input components corresponding to function arguments are now + displayed.][11165] - [Fixed "rename project" button being broken after not changing project name][11103] - [Numbers starting with dot (`.5`) are accepted in Numeric Widget][11108] @@ -41,6 +43,7 @@ [11001]: https://github.com/enso-org/enso/pull/11001 [11014]: https://github.com/enso-org/enso/pull/11014 [11030]: https://github.com/enso-org/enso/pull/11030 +[11165]: https://github.com/enso-org/enso/pull/11165 [11103]: https://github.com/enso-org/enso/pull/11103 [11108]: https://github.com/enso-org/enso/pull/11108 [11056]: https://github.com/enso-org/enso/pull/11056 diff --git a/app/gui2/e2e/actions.ts b/app/gui2/e2e/actions.ts index ccf21b5c29a6..a7e285488cdc 100644 --- a/app/gui2/e2e/actions.ts +++ b/app/gui2/e2e/actions.ts @@ -21,7 +21,7 @@ export async function goToGraph(page: Page, closeDocPanel: boolean = true) { await expect(page.getByTestId('rightDock')).toBeHidden() } // Wait for position initialization - await expectNodePositionsInitialized(page, 72) + await expectNodePositionsInitialized(page, -16) } export async function expectNodePositionsInitialized(page: Page, yPos: number) { diff --git a/app/gui2/e2e/collapsingAndEntering.spec.ts b/app/gui2/e2e/collapsingAndEntering.spec.ts index 07a2b5684e62..6c2b07377cc8 100644 --- a/app/gui2/e2e/collapsingAndEntering.spec.ts +++ b/app/gui2/e2e/collapsingAndEntering.spec.ts @@ -88,11 +88,9 @@ test('Collapsing nodes', async ({ page }) => { module: 'local.Mock_Project', name: 'collapsed', isStatic: true, - aliases: [], arguments: [{ name: 'five', reprType: 'Any', isSuspended: false, hasDefault: false }], selfType: 'local.Mock_Project', returnType: 'Standard.Base.Any.Any', - documentation: [], annotations: [], }) const collapsedNode = locate.graphNodeByBinding(page, 'prod') @@ -102,8 +100,8 @@ test('Collapsing nodes', async ({ page }) => { await expect(collapsedNode.locator('.WidgetTopLevelArgument')).toHaveText('five') await locate.graphNodeIcon(collapsedNode).dblclick() - await actions.ensureNoCircularMenusVisibleDueToHovering(page) - await expect(locate.graphNode(page)).toHaveCount(4) + await expect(locate.graphNode(page)).toHaveCount(5) + await expect(locate.inputNode(page)).toHaveCount(1) await expect(locate.graphNodeByBinding(page, 'ten')).toExist() await expect(locate.graphNodeByBinding(page, 'sum')).toExist() await expect(locate.graphNodeByBinding(page, 'prod')).toExist() @@ -114,7 +112,8 @@ test('Collapsing nodes', async ({ page }) => { // Wait till node is selected. await expect(locate.graphNodeByBinding(page, 'ten').and(page.locator('.selected'))).toHaveCount(1) await page.keyboard.press(COLLAPSE_SHORTCUT) - await expect(locate.graphNode(page)).toHaveCount(4) + await expect(locate.graphNode(page)).toHaveCount(5) + await expect(locate.inputNode(page)).toHaveCount(1) const secondCollapsedNode = locate.graphNodeByBinding(page, 'ten') await expect(secondCollapsedNode.locator('.WidgetToken')).toHaveText(['Main', '.', 'collapsed1']) @@ -124,6 +123,23 @@ test('Collapsing nodes', async ({ page }) => { await expect(locate.graphNodeByBinding(page, 'ten')).toExist() }) +test('Input node', async ({ page }) => { + await actions.goToGraph(page) + await enterToFunc2(page) + + const inputNode = locate.inputNode(page) + await expect(inputNode).toHaveCount(1) + // Input node with identifier should have the icon and an identifier. + await expect(inputNode.locator('.WidgetIcon')).toHaveCount(1) + await expect(inputNode.locator('.WidgetToken')).toContainText('a') + + await inputNode.click() + await page.keyboard.press('Delete') + await expect(inputNode).toHaveCount(1) + await inputNode.locator('.More').click({}) + await expect(inputNode.getByTestId('removeNode')).toHaveClass(/(?<=^| )disabled(?=$| )/) +}) + test('Output node', async ({ page }) => { await actions.goToGraph(page) await enterToFunc2(page) @@ -145,11 +161,31 @@ test('Output node is not collapsed', async ({ page }) => { await actions.goToGraph(page) await enterToFunc2(page) + await locate.outputNode(page).click({ modifiers: ['Shift'] }) await locate .graphNodeByBinding(page, 'r') .locator('.grab-handle') .click({ modifiers: ['Shift'] }) - await locate.outputNode(page).click({ modifiers: ['Shift'] }) + + await page.getByLabel('Group Selected Components').click() + await expect(locate.graphNodeByBinding(page, 'r').locator('.WidgetToken')).toHaveText([ + 'Main', + '.', + 'collapsed', + 'a', + ]) + await expect(locate.inputNode(page)).toHaveCount(1) +}) + +test('Input node is not collapsed', async ({ page }) => { + await actions.goToGraph(page) + await enterToFunc2(page) + + await locate + .graphNodeByBinding(page, 'r') + .locator('.grab-handle') + .click({ modifiers: ['Shift'] }) + await locate.inputNode(page).click({ modifiers: ['Shift'] }) await page.getByLabel('Group Selected Components').click() await expect(locate.graphNodeByBinding(page, 'r').locator('.WidgetToken')).toHaveText([ @@ -162,7 +198,7 @@ test('Output node is not collapsed', async ({ page }) => { }) async function expectInsideMain(page: Page) { - await actions.expectNodePositionsInitialized(page, 72) + await actions.expectNodePositionsInitialized(page, -16) await expect(locate.graphNode(page)).toHaveCount(MAIN_FILE_NODES) await expect(locate.graphNodeByBinding(page, 'five')).toExist() await expect(locate.graphNodeByBinding(page, 'ten')).toExist() @@ -177,16 +213,18 @@ async function expectInsideMain(page: Page) { } async function expectInsideFunc1(page: Page) { - await actions.expectNodePositionsInitialized(page, 216) - await expect(locate.graphNode(page)).toHaveCount(3) + await actions.expectNodePositionsInitialized(page, -88) + await expect(locate.graphNode(page)).toHaveCount(4) + await expect(locate.inputNode(page)).toHaveCount(1) await expect(locate.graphNodeByBinding(page, 'f2')).toExist() await expect(locate.graphNodeByBinding(page, 'result')).toExist() await expect(locate.outputNode(page)).toHaveCount(1) } async function expectInsideFunc2(page: Page) { - await actions.expectNodePositionsInitialized(page, 144) - await expect(locate.graphNode(page)).toHaveCount(2) + await actions.expectNodePositionsInitialized(page, -88) + await expect(locate.graphNode(page)).toHaveCount(3) + await expect(locate.inputNode(page)).toHaveCount(1) await expect(locate.graphNodeByBinding(page, 'r')).toExist() await expect(locate.outputNode(page)).toHaveCount(1) } diff --git a/app/gui2/e2e/locate.ts b/app/gui2/e2e/locate.ts index d41174d88ce7..cc6d31ceea0c 100644 --- a/app/gui2/e2e/locate.ts +++ b/app/gui2/e2e/locate.ts @@ -81,6 +81,9 @@ export function graphNodeIcon(node: Node) { export function selectedNodes(page: Page | Locator): Node { return page.locator('.GraphNode.selected') as Node } +export function inputNode(page: Page | Locator): Node { + return page.locator('.GraphNode.inputNode') as Node +} export function outputNode(page: Page | Locator): Node { return page.locator('.GraphNode.outputNode') as Node } diff --git a/app/gui2/src/components/ComponentBrowser/__tests__/placement.test.ts b/app/gui2/src/components/ComponentBrowser/__tests__/placement.test.ts index 4509818ef898..3882e6d67985 100644 --- a/app/gui2/src/components/ComponentBrowser/__tests__/placement.test.ts +++ b/app/gui2/src/components/ComponentBrowser/__tests__/placement.test.ts @@ -1,9 +1,11 @@ import { collapsedNodePlacement, + inputNodePlacement, mouseDictatedPlacement, nonDictatedPlacement, previousNodeDictatedPlacement, type Environment, + type InputNodeEnvironment, } from '@/components/ComponentBrowser/placement' import * as iterable from '@/util/data/iterable' import { chain, map, range } from '@/util/data/iterable' @@ -423,6 +425,38 @@ describe('Collapsed node placement', () => { }) }) +describe('Input node placement', () => { + function environment(inputNodeRects: Rect[], nonInputNodeRects: Rect[]): InputNodeEnvironment { + return { + screenBounds, + nodeRects: [...inputNodeRects, ...nonInputNodeRects], + nonInputNodeRects: nonInputNodeRects, + selectedNodeRects: [], + } + } + + test('No input nodes, single component', () => { + const X = 1100 + const Y = 700 + const nonInputNodeRects = [rectAt(X, Y)] + const result = inputNodePlacement(size, environment([], nonInputNodeRects), gap) + expect(result).toEqual(new Vec2(X, 656)) + }) + + test('No input nodes, two components', () => { + const nonInputNodeRects = [rectAt(1000, 600), rectAt(1300, 800)] + const result = inputNodePlacement(size, environment([], nonInputNodeRects), gap) + expect(result).toEqual(new Vec2(1000, 556)) + }) + + test('One input node, two components', () => { + const nonInputNodeRects = [rectAt(1000, 600), rectAt(1300, 800)] + const inputNodeRects = [rectAt(1000, 556)] + const result = inputNodePlacement(size, environment(inputNodeRects, nonInputNodeRects), gap) + expect(result).toEqual(new Vec2(1124, 556)) + }) +}) + // === Helpers for debugging === function generateVueCodeForNonDictatedPlacement(newNode: Rect, rects: Rect[]) { diff --git a/app/gui2/src/components/ComponentBrowser/placement.ts b/app/gui2/src/components/ComponentBrowser/placement.ts index 8dc7b6221c31..6d317fff5adb 100644 --- a/app/gui2/src/components/ComponentBrowser/placement.ts +++ b/app/gui2/src/components/ComponentBrowser/placement.ts @@ -28,6 +28,8 @@ export function usePlacement(nodeRects: ToValue>, screenBounds: T previousNodeDictatedPlacement(nodeSize, environment(selectedNodeRects), gap), collapse: (selectedNodeRects: Iterable, nodeSize: Vec2 = DEFAULT_NODE_SIZE): Vec2 => collapsedNodePlacement(nodeSize, environment(selectedNodeRects), gap), + input: (nonInputNodeRects: Iterable, nodeSize: Vec2 = DEFAULT_NODE_SIZE): Vec2 => + inputNodePlacement(nodeSize, { ...environment([]), nonInputNodeRects }, gap), } } @@ -40,6 +42,10 @@ export interface Environment extends NonDictatedEnvironment { selectedNodeRects: Iterable } +export interface InputNodeEnvironment extends Environment { + nonInputNodeRects: Iterable +} + function themeGap(): Vec2 { return new Vec2(theme.node.horizontal_gap, theme.node.vertical_gap) } @@ -119,6 +125,26 @@ export function mouseDictatedPlacement( return mousePosition.add(new Vec2(nodeRadius, nodeRadius)) } +/** The new node should appear above non-input nodes, left aligned to the leftmost node and vertically aligned with the other input nodes. */ +export function inputNodePlacement( + nodeSize: Vec2, + { nonInputNodeRects, nodeRects }: InputNodeEnvironment, + gap = themeGap(), +): Vec2 { + let topMostY + let leftMostX + for (const rect of nonInputNodeRects) { + topMostY = topMostY == null ? rect.top : Math.min(topMostY, rect.top) + leftMostX = leftMostX == null ? rect.left : Math.min(leftMostX, rect.left) + } + assert( + topMostY != null && leftMostX != null, + 'inputNodePlacement works only if at least one non-input node is present.', + ) + const initialPosition = new Vec2(leftMostX, topMostY - nodeSize.y - gap.y) + return seekHorizontal(new Rect(initialPosition, nodeSize), nodeRects, gap) +} + /** The new node should appear at the average Y-position of selected nodes and with the X-position of the leftmost node. * * If the desired place is already occupied by non-selected node, it should be moved down to the closest free space. @@ -155,7 +181,7 @@ export function collapsedNodePlacement( return seekVertical(new Rect(initialPosition, nodeSize), nonSelectedNodeRects, gap) } -/** Given a preferred location for a node, adjust the top as much as necessary for it not to collide with any of the +/** Given a preferred location for a node, adjust the top as low as necessary for it not to collide with any of the * provided `otherRects`. */ export function seekVertical(preferredRect: Rect, otherRects: Iterable, gap = themeGap()) { const initialRect = orDefaultSize(preferredRect) diff --git a/app/gui2/src/components/GraphEditor.vue b/app/gui2/src/components/GraphEditor.vue index 6c9487bc0ce5..7bbcb76498ec 100644 --- a/app/gui2/src/components/GraphEditor.vue +++ b/app/gui2/src/components/GraphEditor.vue @@ -620,7 +620,7 @@ function collapseNodes() { } const selectedNodeRects = filterDefined(Array.from(selected, graphStore.visibleArea)) graphStore.edit((edit) => { - const { refactoredExpressionAstId, collapsedNodeIds, outputNodeId } = performCollapse( + const { refactoredExpressionAstId, collapsedNodeIds, outputAstId } = performCollapse( info.value, edit.getVersion(topLevel), graphStore.db, @@ -628,13 +628,13 @@ function collapseNodes() { ) const position = collapsedNodePlacement(selectedNodeRects) edit.get(refactoredExpressionAstId).mutableNodeMetadata().set('position', position.xy()) - if (outputNodeId != null) { + if (outputAstId != null) { const collapsedNodeRects = filterDefined( Array.from(collapsedNodeIds, graphStore.visibleArea), ) const { place } = usePlacement(collapsedNodeRects, graphNavigator.viewport) const position = place(collapsedNodeRects) - edit.get(refactoredExpressionAstId).mutableNodeMetadata().set('position', position.xy()) + edit.get(outputAstId).mutableNodeMetadata().set('position', position.xy()) } }) } catch (err) { diff --git a/app/gui2/src/components/GraphEditor/GraphEdge.vue b/app/gui2/src/components/GraphEditor/GraphEdge.vue index b8a882dad201..aeeca5d8869e 100644 --- a/app/gui2/src/components/GraphEditor/GraphEdge.vue +++ b/app/gui2/src/components/GraphEditor/GraphEdge.vue @@ -35,7 +35,7 @@ const hoveredPort = computed(() => (mouseAnchor.value ? selection?.hoveredPort : const isSuggestion = computed(() => 'suggestion' in props.edge && props.edge.suggestion) const connectedSourceNode = computed( - () => props.edge.source && graph.db.getPatternExpressionNodeId(props.edge.source), + () => props.edge.source && graph.getSourceNodeId(props.edge.source), ) const sourceNode = computed(() => { diff --git a/app/gui2/src/components/GraphEditor/GraphEdges.vue b/app/gui2/src/components/GraphEditor/GraphEdges.vue index c25c7b4d8141..2f4ced63b329 100644 --- a/app/gui2/src/components/GraphEditor/GraphEdges.vue +++ b/app/gui2/src/components/GraphEditor/GraphEdges.vue @@ -89,7 +89,7 @@ function createEdge(source: AstId, target: PortId) { const ident = graph.db.getOutputPortIdentifier(source) if (ident == null) return - const sourceNode = graph.db.getPatternExpressionNodeId(source) + const sourceNode = graph.getSourceNodeId(source) const targetNode = graph.getPortNodeId(target) if (sourceNode == null || targetNode == null) { return console.error(`Failed to connect edge, source or target node not found.`) diff --git a/app/gui2/src/components/GraphEditor/GraphNode.vue b/app/gui2/src/components/GraphEditor/GraphNode.vue index eab1b87efd74..2b44743533e4 100644 --- a/app/gui2/src/components/GraphEditor/GraphNode.vue +++ b/app/gui2/src/components/GraphEditor/GraphNode.vue @@ -82,6 +82,12 @@ const navigator = injectGraphNavigator(true) const nodeId = computed(() => asNodeId(props.node.rootExpr.externalId)) const potentialSelfArgumentId = computed(() => props.node.primarySubject) +const nodePosition = computed(() => { + // Positions of nodes that are not yet placed are set to `Infinity`. + if (props.node.position.equals(Vec2.Infinity)) return Vec2.Zero + return props.node.position +}) + onUnmounted(() => graph.unregisterNodeRect(nodeId.value)) const rootNode = ref() @@ -187,7 +193,7 @@ watchEffect(() => { } const inZone = (pos: Vec2 | undefined) => pos != null && - pos.sub(props.node.position).x < + pos.sub(nodePosition.value).x < CONTENT_PADDING + ICON_WIDTH + GRAB_HANDLE_X_MARGIN_L + GRAB_HANDLE_X_MARGIN_R const hovered = menuHovered.value || @@ -234,7 +240,7 @@ watch(isVisualizationPreviewed, (newVal, oldVal) => { }) const transform = computed(() => { - const { x, y } = props.node.position + const { x, y } = nodePosition.value return `translate(${x}px, ${y}px)` }) @@ -413,6 +419,7 @@ watchEffect(() => { selected, selectionVisible, ['executionState-' + executionState]: true, + inputNode: props.node.type === 'input', outputNode: props.node.type === 'output', menuVisible, menuFull, @@ -425,7 +432,7 @@ watchEffect(() => { { v-if="isVisualizationVisible" :nodeSize="nodeSize" :scale="navigator?.scale ?? 1" - :nodePosition="props.node.position" + :nodePosition="nodePosition" :isCircularMenuVisible="menuVisible" :currentType="props.node.vis?.identifier" :dataSource="{ type: 'node', nodeId: props.node.rootExpr.externalId }" diff --git a/app/gui2/src/components/GraphEditor/__tests__/collapsing.test.ts b/app/gui2/src/components/GraphEditor/__tests__/collapsing.test.ts index 12df2896c99c..2bf20ad5c9f3 100644 --- a/app/gui2/src/components/GraphEditor/__tests__/collapsing.test.ts +++ b/app/gui2/src/components/GraphEditor/__tests__/collapsing.test.ts @@ -199,15 +199,15 @@ main = setupGraphDb(initialCode, graphDb) const nodes = Array.from(graphDb.nodeIdToNode.keys()) const { extracted, refactored } = unwrap( - prepareCollapsedInfo(new Set(nodes.slice(1, 2)), graphDb), + prepareCollapsedInfo(new Set(nodes.slice(2, 3)), graphDb), ) - expect(extracted.ids).toEqual(new Set(nodes.slice(1, 2))) + expect(extracted.ids).toEqual(new Set(nodes.slice(2, 3))) expect(extracted.inputs).toEqual(['input', 'four']) expect(extracted.output).toEqual({ - node: nodes[1], + node: nodes[2], identifier: 'sum', }) - expect(refactored.id).toEqual(nodes[1]) + expect(refactored.id).toEqual(nodes[2]) expect(refactored.pattern).toEqual('sum') expect(refactored.arguments).toEqual(['input', 'four']) }) diff --git a/app/gui2/src/components/GraphEditor/collapsing.ts b/app/gui2/src/components/GraphEditor/collapsing.ts index 307f40ea84ef..830e4f09ce7e 100644 --- a/app/gui2/src/components/GraphEditor/collapsing.ts +++ b/app/gui2/src/components/GraphEditor/collapsing.ts @@ -63,7 +63,7 @@ export function prepareCollapsedInfo( const leaves = new Set(selected) const inputSet: Set = new Set() let output: Output | null = null - for (const [targetExprId, sourceExprIds] of graphDb.allConnections.allReverse()) { + for (const [targetExprId, sourceExprIds] of graphDb.connections.allReverse()) { const targetNode = graphDb.getExpressionNodeId(targetExprId) if (targetNode == null) continue for (const sourceExprId of sourceExprIds) { @@ -98,7 +98,7 @@ export function prepareCollapsedInfo( } } // If there is no output found so far, it means that none of our nodes is used outside - // the extracted function. In such we will return value from arbitrarily chosen leaf. + // the extracted function. In such case we will return value from arbitrarily chosen leaf. if (output == null) { const arbitraryLeaf = set.first(leaves) if (arbitraryLeaf == null) throw Error('Cannot select the output node, no leaf nodes found.') @@ -157,8 +157,8 @@ interface CollapsingResult { * The order of these IDs is reversed comparing to the order of nodes in the source code. */ collapsedNodeIds: NodeId[] - /** ID of the output node inside the collapsed function. */ - outputNodeId?: NodeId | undefined + /** ID of the output AST node inside the collapsed function. */ + outputAstId?: Ast.AstId | undefined } /** Perform the actual AST refactoring for collapsing nodes. */ @@ -204,18 +204,18 @@ export function performCollapse( // Insert a new function. const collapsedNodeIds = [...filterDefined(collapsed.map(nodeIdFromOuterExpr))].reverse() - let outputNodeId: NodeId | undefined + let outputAstId: Ast.AstId | undefined const outputIdentifier = info.extracted.output?.identifier if (outputIdentifier != null) { const ident = Ast.Ident.new(edit, outputIdentifier) collapsed.push(ident) - outputNodeId = asNodeId(ident.externalId) + outputAstId = ident.id } const argNames = info.extracted.inputs const collapsedFunction = Ast.Function.fromStatements(edit, collapsedName, argNames, collapsed) const collapsedFunctionWithIcon = Ast.Documented.new('ICON group', collapsedFunction) topLevel.insert(posToInsert, collapsedFunctionWithIcon, undefined) - return { refactoredNodeId, refactoredExpressionAstId, collapsedNodeIds, outputNodeId } + return { refactoredNodeId, refactoredExpressionAstId, collapsedNodeIds, outputAstId } } /** Prepare a method call expression for collapsed method. */ diff --git a/app/gui2/src/components/GraphEditor/widgets/WidgetPort.vue b/app/gui2/src/components/GraphEditor/widgets/WidgetPort.vue index 0a9f6f1ed6b9..f2e8bfc475c8 100644 --- a/app/gui2/src/components/GraphEditor/widgets/WidgetPort.vue +++ b/app/gui2/src/components/GraphEditor/widgets/WidgetPort.vue @@ -129,7 +129,8 @@ function relativePortSceneRect(): Rect | undefined { const nodeClientRect = Rect.FromDomRect(rootDomNode.getBoundingClientRect()) const exprSceneRect = navigator.clientToSceneRect(exprClientRect) const exprNodeRect = navigator.clientToSceneRect(nodeClientRect) - return exprSceneRect.offsetBy(exprNodeRect.pos.inverse()) + const rect = exprSceneRect.offsetBy(exprNodeRect.pos.inverse()) + return rect.isFinite() ? rect : undefined } watch( diff --git a/app/gui2/src/composables/nodeColors.ts b/app/gui2/src/composables/nodeColors.ts index 92be1f5b53f1..e0b040770938 100644 --- a/app/gui2/src/composables/nodeColors.ts +++ b/app/gui2/src/composables/nodeColors.ts @@ -40,6 +40,7 @@ export function computeNodeColor( getTypeName: () => string | undefined, ) { if (getType() === 'output') return 'var(--output-node-color)' + if (getType() === 'input') return 'var(--output-node-color)' const group = getGroup() if (group) return groupColorStyle(group) const typeName = getTypeName() diff --git a/app/gui2/src/stores/graph/__tests__/graphDatabase.test.ts b/app/gui2/src/stores/graph/__tests__/graphDatabase.test.ts index 5232aaf245d2..31d769461a77 100644 --- a/app/gui2/src/stores/graph/__tests__/graphDatabase.test.ts +++ b/app/gui2/src/stores/graph/__tests__/graphDatabase.test.ts @@ -65,6 +65,7 @@ test('Reading graph from definition', () => { db.updateBindings(func, rawFunc, code, getSpan) expect(Array.from(db.nodeIdToNode.keys())).toEqual([ + eid('parameter'), eid('node1Content'), eid('node2Content'), eid('node3Content'), @@ -92,14 +93,14 @@ test('Reading graph from definition', () => { 'node1', ) - // Commented the connection from input node, as we don't support them yet. expect(Array.from(db.connections.allForward(), ([key]) => key)).toEqual([ + id('parameter'), id('node1Id'), id('node2Id'), ]) - // expect(Array.from(db.connections.lookup(id('parameter')))).toEqual([id('node1LParam)]) + expect(Array.from(db.connections.lookup(id('parameter')))).toEqual([id('node1LParam')]) expect(Array.from(db.connections.lookup(id('node1Id')))).toEqual([id('node2LParam')]) - // expect(db.getOutputPortIdentifier(id('parameter'))).toBe('a') + expect(db.getOutputPortIdentifier(id('parameter'))).toBe('a') expect(db.getOutputPortIdentifier(id('node1Id'))).toBe('node1') expect(Array.from(db.nodeDependents.lookup(asNodeId(eid('node1Content'))))).toEqual([ eid('node2Content'), diff --git a/app/gui2/src/stores/graph/graphDatabase.ts b/app/gui2/src/stores/graph/graphDatabase.ts index a92b220d1de8..40da7ac41982 100644 --- a/app/gui2/src/stores/graph/graphDatabase.ts +++ b/app/gui2/src/stores/graph/graphDatabase.ts @@ -7,7 +7,7 @@ import { Ast, RawAst } from '@/util/ast' import type { AstId, NodeMetadata } from '@/util/ast/abstract' import { autospaced, MutableModule } from '@/util/ast/abstract' import { AliasAnalyzer } from '@/util/ast/aliasAnalysis' -import { nodeFromAst, nodeRootExpr } from '@/util/ast/node' +import { inputNodeFromAst, nodeFromAst, nodeRootExpr } from '@/util/ast/node' import { MappedKeyMap, MappedSet } from '@/util/containers' import { tryGetIndex } from '@/util/data/array' import { recordEqual } from '@/util/data/object' @@ -134,7 +134,7 @@ export class BindingsDb { export class GraphDb { nodeIdToNode = new ReactiveDb() - private readonly blockNodeLines = new Map() + private readonly nodeSources = new Map() private highestZIndex = 0 private readonly idToExternalMap = reactive(new Map()) private readonly idFromExternalMap = reactive(new Map()) @@ -159,21 +159,11 @@ export class GraphDb { }) connections = new ReactiveIndex(this.bindings.bindings, (alias, info) => { - const srcNode = this.getPatternExpressionNodeId(alias) - // Display connection starting from existing node. - //TODO[ao]: When implementing input nodes, they should be taken into account here. + const srcNode = this.getPatternExpressionNodeId(alias) ?? this.getExpressionNodeId(alias) if (srcNode == null) return [] return Array.from(this.connectionsFromBindings(info, alias, srcNode)) }) - /** Same as {@link GraphDb.connections}, but also includes connections without source node, - * e.g. input arguments of the collapsed function. - */ - allConnections = new ReactiveIndex(this.bindings.bindings, (alias, info) => { - const srcNode = this.getPatternExpressionNodeId(alias) - return Array.from(this.connectionsFromBindings(info, alias, srcNode)) - }) - nodeDependents = new ReactiveIndex(this.nodeIdToNode, (id) => { const result = new Set() for (const port of this.getNodeUsages(id)) { @@ -336,41 +326,70 @@ export class GraphDb { ) { const currentNodeIds = new Set() const body = [...functionAst_.bodyExpressions()] - body.forEach((outerAst, index) => { - const nodeId = nodeIdFromOuterExpr(outerAst) - if (!nodeId) return - const isLastInBlock = index === body.length - 1 - const oldNode = nonReactiveView(this.blockNodeLines.get(nodeId)?.data) + const args = functionAst_.argumentDefinitions + const update = ( + nodeId: NodeId, + ast: Ast.Ast, + isInput: boolean, + isOutput: boolean, + argIndex: number | undefined, + ) => { + const oldNode = nonReactiveView(this.nodeSources.get(nodeId)?.data) if (oldNode) { - const node = resumeShallowReactivity(oldNode) - if (oldNode.isLastInBlock !== isLastInBlock) node.isLastInBlock = isLastInBlock - if (oldNode.outerAst.id !== outerAst.id) node.outerAst = outerAst + const node = resumeShallowReactivity(oldNode) + if (oldNode.isOutput !== isOutput) node.isOutput = isOutput + if (oldNode.isInput !== isInput) node.isInput = isInput + if (oldNode.argIndex !== argIndex) node.argIndex = argIndex + if (oldNode.outerAst.id !== ast.id) node.outerAst = ast } else { - const data = shallowReactive({ isLastInBlock, outerAst }) + const data = shallowReactive({ isOutput, outerAst: ast, isInput, argIndex }) const stop = watchEffect(() => - this.updateNodeStructure(nodeId, data.outerAst, data.isLastInBlock), + this.updateNodeStructure( + nodeId, + data.outerAst, + data.isOutput, + data.isInput, + data.argIndex, + ), ) - this.blockNodeLines.set(nodeId, { data, stop }) + this.nodeSources.set(nodeId, { data, stop }) } currentNodeIds.add(nodeId) + } + args.forEach((argDef, index) => { + const argPattern = argDef.pattern.node + const nodeId = asNodeId(argPattern.externalId) + update(nodeId, argPattern, true, false, index) + }) + body.forEach((outerAst, index) => { + const nodeId = nodeIdFromOuterExpr(outerAst) + if (!nodeId) return + const isLastInBlock = index === body.length - 1 + update(nodeId, outerAst, false, isLastInBlock, undefined) }) - for (const [nodeId, info] of this.blockNodeLines.entries()) { + for (const [nodeId, info] of this.nodeSources.entries()) { if (!currentNodeIds.has(nodeId)) { info.stop() this.nodeIdToNode.delete(nodeId) - this.blockNodeLines.delete(nodeId) + this.nodeSources.delete(nodeId) } } } /** Scan a node's content from its outer expression down to, but not including, its inner expression. */ - private updateNodeStructure(nodeId: NodeId, nodeAst: Ast.Ast, isLastInBlock: boolean) { - const newNode = nodeFromAst(nodeAst, isLastInBlock) + private updateNodeStructure( + nodeId: NodeId, + ast: Ast.Ast, + isOutput: boolean, + isInput: boolean, + argIndex?: number, + ) { + const newNode = isInput ? inputNodeFromAst(ast, argIndex ?? 0) : nodeFromAst(ast, isOutput) if (!newNode) return const oldNode = this.nodeIdToNode.getUntracked(nodeId) if (oldNode == null) { const nodeMeta = newNode.rootExpr.nodeMetadata - const pos = nodeMeta.get('position') ?? { x: 0, y: 0 } + const pos = nodeMeta.get('position') ?? { x: Infinity, y: Infinity } const metadataFields = { position: new Vec2(pos.x, pos.y), vis: nodeMeta.get('visualization'), @@ -392,6 +411,7 @@ export class GraphDb { prefixes, conditionalPorts, docs, + argIndex, } = newNode const node = resumeReactivity(oldNode) if (oldNode.type !== type) node.type = type @@ -415,6 +435,7 @@ export class GraphDb { prefixes, conditionalPorts, docs, + argIndex, } satisfies NodeDataFromAst } } @@ -516,6 +537,7 @@ export class GraphDb { rootExpr: Ast.parse(code ?? '0'), innerExpr: Ast.parse(code ?? '0'), zIndex: this.highestZIndex, + argIndex: undefined, } const bindingId = pattern.id this.nodeIdToNode.set(id, node) @@ -524,16 +546,25 @@ export class GraphDb { } } -interface BlockLine { +/** Source code data of the specific node. */ +interface NodeSource { + /** The outer AST of the node (see {@link NodeDataFromAst.outerExpr}). */ outerAst: Ast.Ast - isLastInBlock: boolean + /** Whether the node is `output` of the function or not. Mutually exclusive with `isInput`. + * Output node is the last node in a function body and has no pattern. */ + isOutput: boolean + /** Whether the node is `input` of the function or not. Mutually exclusive with `isOutput`. + * Input node is a function argument. */ + isInput: boolean + /** The index of the argument in the function's argument list, if the node is an input node. */ + argIndex: number | undefined } declare const brandNodeId: unique symbol /** An unique node identifier, shared across all clients. It is the ExternalId of node's root expression. */ export type NodeId = string & ExternalId & { [brandNodeId]: never } -export type NodeType = 'component' | 'output' +export type NodeType = 'component' | 'output' | 'input' export function asNodeId(id: ExternalId): NodeId export function asNodeId(id: ExternalId | undefined): NodeId | undefined export function asNodeId(id: ExternalId | undefined): NodeId | undefined { @@ -567,6 +598,8 @@ export interface NodeDataFromAst { conditionalPorts: Set /** An AST node containing the node's documentation comment. */ docs: Ast.Documented | undefined + /** The index of the argument in the function's argument list, if the node is an input node. */ + argIndex: number | undefined } export interface NodeDataFromMetadata { diff --git a/app/gui2/src/stores/graph/index.ts b/app/gui2/src/stores/graph/index.ts index 2da83509b989..53af5164458c 100644 --- a/app/gui2/src/stores/graph/index.ts +++ b/app/gui2/src/stores/graph/index.ts @@ -33,6 +33,7 @@ import { iteratorFilter } from 'lib0/iterator' import { computed, markRaw, + nextTick, proxyRefs, reactive, ref, @@ -420,7 +421,7 @@ export const { injectFn: useGraphStore, provideFn: provideGraphStore } = createC function updateNodeRect(nodeId: NodeId, rect: Rect) { nodeRects.set(nodeId, rect) - if (rect.pos.equals(Vec2.Zero)) { + if (rect.pos.equals(Vec2.Infinity)) { nodesToPlace.push(nodeId) } } @@ -430,26 +431,48 @@ export const { injectFn: useGraphStore, provideFn: provideGraphStore } = createC } const nodesToPlace = reactive([]) - const { place: placeNode } = usePlacement(visibleNodeAreas, Rect.Zero) - - watch(nodesToPlace, (nodeIds) => { - if (nodeIds.length === 0) return - const nodesToProcess = [...nodeIds] - nodesToPlace.length = 0 - batchEdits(() => { - for (const nodeId of nodesToProcess) { - const rect = nodeRects.get(nodeId) - if (!rect) continue - const nodeAst = syncModule.value?.get(db.idFromExternal(nodeId)) - if (!nodeAst) continue - const metadata = nodeAst.mutableNodeMetadata() - if (metadata.get('position') != null) continue - const position = placeNode([], rect.size) - metadata.set('position', { x: position.x, y: position.y }) - nodeRects.set(nodeId, new Rect(position, rect.size)) - } - }, 'local:autoLayout') - }) + const { place: placeNode, input: placeInputNode } = usePlacement(visibleNodeAreas, Rect.Zero) + + watch(nodesToPlace, (nodeIds) => + nextTick(() => { + if (nodeIds.length === 0) return + const [inputNodes, nonInputNodes] = partition( + nodeIds, + (id) => db.nodeIdToNode.get(id)?.type === 'input', + ) + const nonInputNodesSortedByLines = pickInCodeOrder(new Set(nonInputNodes)) + const inputNodesSortedByArgIndex = inputNodes.sort((a, b) => { + const nodeA = db.nodeIdToNode.get(a) + const nodeB = db.nodeIdToNode.get(b) + if (!nodeA || !nodeB) return 0 + return (nodeA.argIndex ?? 0) - (nodeB.argIndex ?? 0) + }) + const nodesToProcess = [...nonInputNodesSortedByLines, ...inputNodesSortedByArgIndex] + nodesToPlace.length = 0 + batchEdits(() => { + for (const nodeId of nodesToProcess) { + const nodeType = db.nodeIdToNode.get(nodeId)?.type + const rect = nodeRects.get(nodeId) + if (!rect) continue + const nodeAst = syncModule.value?.get(db.idFromExternal(nodeId)) + if (!nodeAst) continue + const metadata = nodeAst.mutableNodeMetadata() + if (metadata.get('position') != null) continue + let position + if (nodeType === 'input') { + const allNodes = [...db.nodeIdToNode.entries()] + const nonInputNodes = allNodes.filter(([_, node]) => node.type !== 'input') + const nonInputNodeRects = nonInputNodes.map(([id]) => nodeRects.get(id) ?? Rect.Zero) + position = placeInputNode(nonInputNodeRects, rect.size) + } else { + position = placeNode([], rect.size) + } + metadata.set('position', { x: position.x, y: position.y }) + nodeRects.set(nodeId, new Rect(position, rect.size)) + } + }, 'local:autoLayout') + }), + ) function updateVizRect(id: NodeId, rect: Rect | undefined) { if (rect) vizRects.set(id, rect) @@ -501,6 +524,12 @@ export const { injectFn: useGraphStore, provideFn: provideGraphStore } = createC return getPortRelativeRect(id) != null } + /** Return the node ID that has the given `id` as its pattern or primary port. + * Technically this is either a component or the input node, as input nodes do not have patterns. */ + function getSourceNodeId(id: AstId): NodeId | undefined { + return db.getPatternExpressionNodeId(id) || getPortPrimaryInstance(id)?.nodeId + } + function getPortNodeId(id: PortId): NodeId | undefined { return (isAstId(id) && db.getExpressionNodeId(id)) || getPortPrimaryInstance(id)?.nodeId } @@ -739,6 +768,7 @@ export const { injectFn: useGraphStore, provideFn: provideGraphStore } = createC removePortInstance, getPortRelativeRect, getPortNodeId, + getSourceNodeId, isPortEnabled, updatePortValue, setEditedNode, diff --git a/app/gui2/src/util/ast/node.ts b/app/gui2/src/util/ast/node.ts index 5994245388a6..65f0b89eb899 100644 --- a/app/gui2/src/util/ast/node.ts +++ b/app/gui2/src/util/ast/node.ts @@ -26,12 +26,27 @@ export function nodeRootExpr(ast: Ast.Ast): { } } +export function inputNodeFromAst(ast: Ast.Ast, argIndex: number): NodeDataFromAst { + return { + type: 'input', + outerExpr: ast, + pattern: undefined, + rootExpr: ast, + innerExpr: ast, + prefixes: { enableRecording: undefined }, + primarySubject: undefined, + conditionalPorts: new Set(), + docs: undefined, + argIndex, + } +} + /** Given a node's outer expression, return all the `Node` fields that depend on its AST structure. */ -export function nodeFromAst(ast: Ast.Ast, isLastLine: boolean): NodeDataFromAst | undefined { +export function nodeFromAst(ast: Ast.Ast, isOutput: boolean): NodeDataFromAst | undefined { const { root, docs, assignment } = nodeRootExpr(ast) if (!root) return const { innerExpr, matches } = prefixes.extractMatches(root) - const type = assignment == null && isLastLine ? 'output' : 'component' + const type = assignment == null && isOutput ? 'output' : 'component' const primaryApplication = primaryApplicationSubject(innerExpr) return { type, @@ -43,6 +58,7 @@ export function nodeFromAst(ast: Ast.Ast, isLastLine: boolean): NodeDataFromAst primarySubject: primaryApplication?.subject, conditionalPorts: new Set(primaryApplication?.accessChain ?? []), docs, + argIndex: undefined, } } diff --git a/app/gui2/src/util/data/vec2.ts b/app/gui2/src/util/data/vec2.ts index 58aa38b0aaed..f5a54a8943cb 100644 --- a/app/gui2/src/util/data/vec2.ts +++ b/app/gui2/src/util/data/vec2.ts @@ -10,6 +10,7 @@ export class Vec2 { ) {} static Zero: Vec2 + static Infinity: Vec2 static FromTuple(point: [x: number, y: number]): Vec2 { return new Vec2(point[0], point[1]) @@ -138,3 +139,4 @@ export class Vec2 { } Vec2.Zero = new Vec2(0, 0) +Vec2.Infinity = new Vec2(Infinity, Infinity) diff --git a/app/gui2/src/util/getIconName.ts b/app/gui2/src/util/getIconName.ts index a12a237b8311..9c5ae776bda6 100644 --- a/app/gui2/src/util/getIconName.ts +++ b/app/gui2/src/util/getIconName.ts @@ -60,5 +60,7 @@ export function iconOfNode(node: NodeId, graphDb: GraphDb) { ) case 'output': return 'data_output' + case 'input': + return 'data_input' } } diff --git a/app/gui2/stories/GraphNode.story.vue b/app/gui2/stories/GraphNode.story.vue index 38ac760f6c6d..b3d4f0d01e73 100644 --- a/app/gui2/stories/GraphNode.story.vue +++ b/app/gui2/stories/GraphNode.story.vue @@ -48,6 +48,7 @@ const node = computed((): Node => { conditionalPorts: new Set(), type: 'component', docs: undefined, + argIndex: undefined, } }) diff --git a/app/ydoc-shared/src/ast/parse.ts b/app/ydoc-shared/src/ast/parse.ts index 138e46ef40cf..a17d294718f5 100644 --- a/app/ydoc-shared/src/ast/parse.ts +++ b/app/ydoc-shared/src/ast/parse.ts @@ -158,7 +158,22 @@ class Abstractor { } case RawAst.Tree.Type.Function: { const name = this.abstractTree(tree.name) - const argumentDefinitions = Array.from(tree.args, arg => this.abstractChildren(arg)) + const argumentDefinitions = Array.from(tree.args, arg => ({ + open: arg.open && this.abstractToken(arg.open), + open2: arg.open2 && this.abstractToken(arg.open2), + suspension: arg.suspension && this.abstractToken(arg.suspension), + pattern: this.abstractTree(arg.pattern), + type: arg.typeNode && { + operator: this.abstractToken(arg.typeNode.operator), + type: this.abstractTree(arg.typeNode.typeNode), + }, + close2: arg.close2 && this.abstractToken(arg.close2), + defaultValue: arg.default && { + equals: this.abstractToken(arg.default.equals), + expression: this.abstractTree(arg.default.expression), + }, + close: arg.close && this.abstractToken(arg.close), + })) const equals = this.abstractToken(tree.equals) const body = tree.body !== undefined ? this.abstractTree(tree.body) : undefined node = Function.concrete(this.module, name, argumentDefinitions, equals, body) diff --git a/app/ydoc-shared/src/ast/tree.ts b/app/ydoc-shared/src/ast/tree.ts index cc1faf71c866..8aa8b4c7a6cd 100644 --- a/app/ydoc-shared/src/ast/tree.ts +++ b/app/ydoc-shared/src/ast/tree.ts @@ -1834,9 +1834,26 @@ export function isNumericLiteral(code: string) { return is_numeric_literal(code) } -/** The actual contents of an `ArgumentDefinition` are complex, but probably of more interest to the compiler than the - * GUI. We just need to represent them faithfully and create the simple cases. */ -type ArgumentDefinition = (T['ast'] | T['token'])[] +export interface ArgumentDefinition { + open?: T['token'] | undefined + open2?: T['token'] | undefined + suspension?: T['token'] | undefined + pattern: T['ast'] + type?: ArgumentType | undefined + close2?: T['token'] | undefined + defaultValue?: ArgumentDefault | undefined + close?: T['token'] | undefined +} + +interface ArgumentDefault { + equals: T['token'] + expression: T['ast'] +} + +interface ArgumentType { + operator: T['token'] + type: T['ast'] +} export interface FunctionFields { name: NodeChild @@ -1864,7 +1881,7 @@ export class Function extends Ast { get argumentDefinitions(): ArgumentDefinition[] { return this.fields .get('argumentDefinitions') - .map(raw => raw.map(part => this.module.getConcrete(part))) + .map(def => mapRefs(def, rawToConcrete(this.module))) } static concrete( @@ -1913,7 +1930,9 @@ export class Function extends Ast { const statements_: OwnedBlockLine[] = statements.map(statement => ({ expression: unspaced(statement), })) - const argumentDefinitions = argumentNames.map(name => [spaced(Ident.new(module, name))]) + const argumentDefinitions = argumentNames.map(name => ({ + pattern: spaced(Ident.new(module, name)), + })) const body = BodyBlock.new(statements_, module) return MutableFunction.new(module, name, argumentDefinitions, body) } @@ -1934,7 +1953,23 @@ export class Function extends Ast { *concreteChildren(_verbatim?: boolean): IterableIterator { const { name, argumentDefinitions, equals, body } = getAll(this.fields) yield name - for (const def of argumentDefinitions) yield* def + for (const def of argumentDefinitions) { + const { open, open2, suspension, pattern, type, close2, defaultValue, close } = def + if (open) yield open + if (open2) yield open2 + if (suspension) yield suspension + yield pattern + if (type) { + yield type.operator + yield type.type + } + if (defaultValue) { + yield defaultValue.equals + yield defaultValue.expression + } + if (close2) yield close2 + if (close) yield close + } yield { whitespace: equals.whitespace ?? ' ', node: this.module.getToken(equals.node) } if (body) yield preferSpacedIf(body, this.module.tryGet(body.node) instanceof BodyBlock) }