diff --git a/app/gui2/src/components/ComponentBrowser.vue b/app/gui2/src/components/ComponentBrowser.vue index 2cbe614d1dbf..fc985797a1f4 100644 --- a/app/gui2/src/components/ComponentBrowser.vue +++ b/app/gui2/src/components/ComponentBrowser.vue @@ -91,6 +91,11 @@ const currentFiltering = computed(() => { watch(currentFiltering, () => { selected.value = input.autoSelectFirstComponent.value ? 0 : null scrolling.targetScroll.value = { type: 'bottom' } + + // Update `highlightPosition` synchronously, so the subsequent animation `skip` have an effect. + if (selectedPosition.value != null) { + highlightPosition.value = selectedPosition.value + } animatedHighlightPosition.skip() animatedHighlightHeight.skip() }) @@ -233,18 +238,10 @@ const selectedSuggestion = computed(() => { return suggestionDbStore.entries.get(id) ?? null }) -watch( - selectedPosition, - (newPos) => { - if (newPos == null) return - highlightPosition.value = newPos - if (animatedHighlightHeight.value <= 1.0) { - animatedHighlightPosition.skip() - } - }, - // Needs to be synchronous to make skipping highlight animation work. - { flush: 'sync' }, -) +watch(selectedPosition, (newPos) => { + if (newPos == null) return + highlightPosition.value = newPos +}) const highlightClipPath = computed(() => { let height = animatedHighlightHeight.value diff --git a/app/gui2/src/components/GraphEditor.vue b/app/gui2/src/components/GraphEditor.vue index 91d7bf4a1047..acb7160074e0 100644 --- a/app/gui2/src/components/GraphEditor.vue +++ b/app/gui2/src/components/GraphEditor.vue @@ -196,9 +196,7 @@ const graphBindingsHandler = graphBindings.handler({ }, deleteSelected() { graphStore.transact(() => { - for (const node of nodeSelection.selected) { - graphStore.deleteNode(node) - } + graphStore.deleteNodes([...nodeSelection.selected]) nodeSelection.selected.clear() }) }, @@ -266,11 +264,11 @@ const graphBindingsHandler = graphBindings.handler({ if (currentMethodName == null) { bail(`Cannot get the method name for the current execution stack item. ${currentMethod}`) } - graphStore.editAst((module) => { + graphStore.editScope((edit) => { if (graphStore.moduleRoot == null) bail(`Module root is missing.`) - const topLevel = module.get(graphStore.moduleRoot) + const topLevel = edit.get(graphStore.moduleRoot) assert(topLevel instanceof BodyBlock) - return performCollapse(info, module, topLevel, graphStore.db, currentMethodName) + return performCollapse(info, edit, topLevel, graphStore.db, currentMethodName) }) } catch (err) { console.log('Error while collapsing, this is not normal.', err) diff --git a/app/gui2/src/components/GraphEditor/GraphEdges.vue b/app/gui2/src/components/GraphEditor/GraphEdges.vue index c2948ac127f7..bb9a3e1036c5 100644 --- a/app/gui2/src/components/GraphEditor/GraphEdges.vue +++ b/app/gui2/src/components/GraphEditor/GraphEdges.vue @@ -7,6 +7,7 @@ import type { PortId } from '@/providers/portInfo' import { useGraphStore } from '@/stores/graph' import { Ast } from '@/util/ast' import { Vec2 } from '@/util/data/vec2' +import { toast } from 'react-toastify' import { isUuid, type ExprId } from 'shared/yjsModel.ts' const graph = useGraphStore() @@ -49,28 +50,45 @@ const editingEdge: Interaction = { interaction.setWhen(() => graph.unconnectedEdge != null, editingEdge) function disconnectEdge(target: PortId) { - if (!graph.updatePortValue(target, undefined)) { - const targetStr: string = target - if (isUuid(targetStr)) { - console.warn(`Failed to disconnect edge from port ${target}, falling back to direct edit.`) - graph.setExpressionContent(targetStr as ExprId, '_') - } else { - console.error(`Failed to disconnect edge from port ${target}, no fallback possible.`) + graph.editScope((edit) => { + if (!graph.updatePortValue(edit, target, undefined)) { + const targetStr: string = target + if (isUuid(targetStr)) { + console.warn(`Failed to disconnect edge from port ${target}, falling back to direct edit.`) + edit.replaceRef(Ast.asNodeId(targetStr as ExprId), Ast.Wildcard.new()) + } else { + console.error(`Failed to disconnect edge from port ${target}, no fallback possible.`) + } } - } + }) } function createEdge(source: ExprId, target: PortId) { const ident = graph.db.getOutputPortIdentifier(source) if (ident == null) return const identAst = Ast.parse(ident) - if (!graph.updatePortValue(target, identAst)) { - const targetStr: string = target - if (isUuid(targetStr)) { - console.warn(`Failed to connect edge to port ${target}, falling back to direct edit.`) - graph.setExpressionContent(targetStr as ExprId, ident) - } else { - console.error(`Failed to connect edge to port ${target}, no fallback possible.`) + + const sourceNode = graph.db.getPatternExpressionNodeId(source) + const targetNode = graph.getPortNodeId(target) + if (sourceNode == null || targetNode == null) { + console.log(sourceNode, targetNode, source, target) + return console.error(`Failed to connect edge, source or target node not found.`) + } + + const edit = graph.astModule.edit() + const reorderResult = graph.ensureCorrectNodeOrder(edit, sourceNode, targetNode) + if (reorderResult === 'circular') { + // Creating this edge would create a circular dependency. Prevent that and display error. + toast.error('Could not connect due to circular dependency.') + } else { + if (!graph.updatePortValue(edit, target, identAst)) { + if (isUuid(target)) { + console.warn(`Failed to connect edge to port ${target}, falling back to direct edit.`) + edit.replaceValue(Ast.asNodeId(target), identAst) + graph.commitEdit(edit) + } else { + console.error(`Failed to connect edge to port ${target}, no fallback possible.`) + } } } } diff --git a/app/gui2/src/components/GraphEditor/NodeWidgetTree.vue b/app/gui2/src/components/GraphEditor/NodeWidgetTree.vue index 851ef08979f9..fb7495a561ae 100644 --- a/app/gui2/src/components/GraphEditor/NodeWidgetTree.vue +++ b/app/gui2/src/components/GraphEditor/NodeWidgetTree.vue @@ -4,7 +4,6 @@ import { useTransitioning } from '@/composables/animation' import { WidgetInput, type WidgetUpdate } from '@/providers/widgetRegistry' import { provideWidgetTree } from '@/providers/widgetTree' import { useGraphStore } from '@/stores/graph' -import { assertNever } from '@/util/assert' import { Ast } from '@/util/ast' import { isUuid } from 'shared/yjsModel' import { computed, toRef } from 'vue' diff --git a/app/gui2/src/components/GraphEditor/collapsing.ts b/app/gui2/src/components/GraphEditor/collapsing.ts index 7474dfbf1c13..9f0542d2aea6 100644 --- a/app/gui2/src/components/GraphEditor/collapsing.ts +++ b/app/gui2/src/components/GraphEditor/collapsing.ts @@ -39,7 +39,7 @@ interface Output { interface RefactoredInfo { /** The id of the refactored node. */ id: ExprId - /** The pattern of the refactored node. Included for convinience, collapsing does not affect it. */ + /** The pattern of the refactored node. Included for convenience, collapsing does not affect it. */ pattern: string /** The list of necessary arguments for a call of the collapsed function. */ arguments: Identifier[] @@ -88,12 +88,13 @@ export function prepareCollapsedInfo(selected: Set, graphDb: GraphDb): C // 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. if (output == null) { - const arbitaryLeaf = set.first(leaves) - if (arbitaryLeaf == null) throw new Error('Cannot select the output node, no leaf nodes found.') - const outputNode = graphDb.nodeIdToNode.get(arbitaryLeaf) - if (outputNode == null) throw new Error(`The node with id ${arbitaryLeaf} not found.`) + const arbitraryLeaf = set.first(leaves) + if (arbitraryLeaf == null) + throw new Error('Cannot select the output node, no leaf nodes found.') + const outputNode = graphDb.nodeIdToNode.get(arbitraryLeaf) + if (outputNode == null) throw new Error(`The node with id ${arbitraryLeaf} not found.`) const identifier = unwrap(tryIdentifier(outputNode.pattern?.code() || '')) - output = { node: arbitaryLeaf, identifier } + output = { node: arbitraryLeaf, identifier } } const pattern = graphDb.nodeIdToNode.get(output.node)?.pattern?.code() ?? '' @@ -134,29 +135,28 @@ const COLLAPSED_FUNCTION_NAME = 'collapsed' /** Perform the actual AST refactoring for collapsing nodes. */ export function performCollapse( info: CollapsedInfo, - module: Ast.Module, + edit: Ast.MutableModule, topLevel: Ast.BodyBlock, db: GraphDb, currentMethodName: string, -): Ast.MutableModule { - const functionAst = Ast.findModuleMethod(module, currentMethodName) +) { + const functionAst = Ast.findModuleMethod(edit, currentMethodName) if (!(functionAst instanceof Ast.Function) || !(functionAst.body instanceof Ast.BodyBlock)) { throw new Error(`Expected a collapsable function, found ${functionAst}.`) } const functionBlock = functionAst.body - const posToInsert = findInsertionPos(module, topLevel, currentMethodName) - const collapsedName = findSafeMethodName(module, COLLAPSED_FUNCTION_NAME) + const posToInsert = findInsertionPos(edit, topLevel, currentMethodName) + const collapsedName = findSafeMethodName(edit, COLLAPSED_FUNCTION_NAME) const astIdsToExtract = new Set( [...info.extracted.ids].map((nodeId) => db.nodeIdToNode.get(nodeId)?.outerExprId), ) const astIdToReplace = db.nodeIdToNode.get(info.refactored.id)?.outerExprId const collapsed = [] const refactored = [] - const edit = module.edit() const lines = functionBlock.lines() for (const line of lines) { const astId = line.expression?.node.exprId - const ast = astId != null ? module.get(astId) : null + const ast = astId != null ? edit.get(astId) : null if (ast == null) continue if (astIdsToExtract.has(astId)) { collapsed.push(ast) @@ -172,15 +172,13 @@ export function performCollapse( if (outputIdentifier != null) { collapsed.push(Ast.Ident.new(edit, outputIdentifier)) } - // Update the definiton of refactored function. + // Update the definition of refactored function. const refactoredBlock = Ast.BodyBlock.new(refactored, edit) edit.replaceRef(functionBlock.exprId, refactoredBlock) - // new Ast.BodyBlock(edit, functionBlock.exprId, refactored) const args: Ast.Ast[] = info.extracted.inputs.map((arg) => Ast.Ident.new(edit, arg)) - const collapsedFunction = Ast.Function.new(edit, collapsedName, args, collapsed, true) + const collapsedFunction = Ast.Function.fromExprs(edit, collapsedName, args, collapsed, true) topLevel.insert(edit, posToInsert, collapsedFunction) - return edit } /** Prepare a method call expression for collapsed method. */ diff --git a/app/gui2/src/components/GraphEditor/widgets/WidgetApplication.vue b/app/gui2/src/components/GraphEditor/widgets/WidgetApplication.vue index d989362ee458..fb87cb4f5cc4 100644 --- a/app/gui2/src/components/GraphEditor/widgets/WidgetApplication.vue +++ b/app/gui2/src/components/GraphEditor/widgets/WidgetApplication.vue @@ -2,18 +2,16 @@ import NodeWidget from '@/components/GraphEditor/NodeWidget.vue' import { WidgetInput, defineWidget, widgetProps } from '@/providers/widgetRegistry' import { Ast } from '@/util/ast' -import { ArgumentApplicationKey, ArgumentAst, ArgumentPlaceholder } from '@/util/callTree' +import { ArgumentApplication, ArgumentApplicationKey } from '@/util/callTree' import { computed } from 'vue' const props = defineProps(widgetProps(widgetDefinition)) const application = computed(() => props.input[ArgumentApplicationKey]) const targetMaybePort = computed(() => { const target = application.value.target - return target instanceof ArgumentPlaceholder || target instanceof ArgumentAst - ? { ...target.toWidgetInput(), forcePort: true } - : target instanceof Ast.Ast - ? WidgetInput.FromAst(target) - : target.toWidgetInput() + const targetInput = + target instanceof Ast.Ast ? WidgetInput.FromAst(target) : target.toWidgetInput() + return { ...targetInput, forcePort: !(target instanceof ArgumentApplication) } }) const appClass = computed(() => { diff --git a/app/gui2/src/components/GraphEditor/widgets/WidgetCheckbox.vue b/app/gui2/src/components/GraphEditor/widgets/WidgetCheckbox.vue index 8fe30913024b..b8a7ec83cbaf 100644 --- a/app/gui2/src/components/GraphEditor/widgets/WidgetCheckbox.vue +++ b/app/gui2/src/components/GraphEditor/widgets/WidgetCheckbox.vue @@ -3,7 +3,7 @@ import CheckboxWidget from '@/components/widgets/CheckboxWidget.vue' import { Score, WidgetInput, defineWidget, widgetProps } from '@/providers/widgetRegistry' import { useGraphStore } from '@/stores/graph' import { Ast } from '@/util/ast' -import { tryQualifiedName, type Identifier, type QualifiedName } from '@/util/qualifiedName.ts' +import { type Identifier, type QualifiedName } from '@/util/qualifiedName.ts' import { computed } from 'vue' const props = defineProps(widgetProps(widgetDefinition)) diff --git a/app/gui2/src/components/GraphEditor/widgets/WidgetFunction.vue b/app/gui2/src/components/GraphEditor/widgets/WidgetFunction.vue index e3c77edb79af..859762fc3f35 100644 --- a/app/gui2/src/components/GraphEditor/widgets/WidgetFunction.vue +++ b/app/gui2/src/components/GraphEditor/widgets/WidgetFunction.vue @@ -24,6 +24,7 @@ import { getAccessOprSubject, interpretCall, } from '@/util/callTree' +import { partitionPoint } from '@/util/data/array' import type { Opt } from '@/util/data/opt' import type { ExprId } from 'shared/yjsModel' import { computed, proxyRefs } from 'vue' @@ -46,6 +47,18 @@ const interpreted = computed(() => { return interpretCall(props.input.value, methodCallInfo.value == null) }) +const selfArgumentPreapplied = computed(() => { + const info = methodCallInfo.value + if (info?.staticallyApplied) return false + const analyzed = interpreted.value + if (analyzed.kind !== 'prefix') return false + const subject = getAccessOprSubject(analyzed.func) + if (!subject) return false + const funcType = info?.methodCall.methodPointer.definedOnType + const subjectInfo = graph.db.getExpressionInfo(subject.exprId) + return funcType != null && subjectInfo?.typename !== `${funcType}.type` +}) + const application = computed(() => { const call = interpreted.value if (!call) return null @@ -60,7 +73,7 @@ const application = computed(() => { suggestion: info?.suggestion, widgetCfg: widgetConfiguration.value, }, - !info?.staticallyApplied, + selfArgumentPreapplied.value, ) }) @@ -84,10 +97,12 @@ const selfArgumentAstId = computed>(() => { return analyzed.lhs?.exprId } else { const knownArguments = methodCallInfo.value?.suggestion?.arguments + const hasSelfArgument = knownArguments?.[0]?.name === 'self' const selfArgument = - knownArguments?.[0]?.name === 'self' - ? getAccessOprSubject(analyzed.func) - : analyzed.args[0]?.argument + hasSelfArgument && !selfArgumentPreapplied.value + ? analyzed.args.find((a) => a.argName === 'self' || a.argName == null)?.argument + : getAccessOprSubject(analyzed.func) ?? analyzed.args[0]?.argument + return selfArgument?.exprId } }) @@ -161,7 +176,7 @@ function handleArgUpdate(update: WidgetUpdate): boolean { newArg = Ast.parse(value, edit) } const name = argApp.argument.insertAsNamed ? argApp.argument.argInfo.name : null - edit.takeAndReplaceValue(app.appTree.exprId, (oldAppTree) => + edit.takeAndReplaceValue(argApp.appTree.exprId, (oldAppTree) => Ast.App.new(oldAppTree, name, newArg, edit), ) props.onUpdate({ edit }) @@ -169,6 +184,25 @@ function handleArgUpdate(update: WidgetUpdate): boolean { } else if (value == null && argApp?.argument instanceof ArgumentAst) { /* Case: Removing existing argument. */ + // HACK: Temporarily modify expression info to include the deleted argument on a list, so it + // immediately appears back as a placeholder after deletion, before the engine respones. + // The engine will soon send another expression update, overwriting this change anyway. + // + // This update is unfortunately not saved in the undo stack. Undoing and redoing the edit will + // still cause the placeholder to glitch out temporarily, but this is good enough for now. + // Proper fix would involve adding a proper "optimistic response" mechanism that can also be + // saved in the undo transaction. + const deletedArgIdx = argApp.argument.index + if (deletedArgIdx != null) { + const notAppliedArguments = methodCallInfo.value?.methodCall.notAppliedArguments + if (notAppliedArguments != null) { + const insertAt = partitionPoint(notAppliedArguments, (i) => i < deletedArgIdx) + // Insert the deleted argument back to the method info. This directly modifies observable + // data in `ComputedValueRegistry`. That's on purpose. + notAppliedArguments.splice(insertAt, 0, deletedArgIdx) + } + } + if (argApp.appTree instanceof Ast.App && argApp.appTree.argumentName != null) { /* Case: Removing named prefix argument. */ diff --git a/app/gui2/src/components/GraphEditor/widgets/WidgetHierarchy.vue b/app/gui2/src/components/GraphEditor/widgets/WidgetHierarchy.vue index 441f3699d7cb..b630ac9bd9f1 100644 --- a/app/gui2/src/components/GraphEditor/widgets/WidgetHierarchy.vue +++ b/app/gui2/src/components/GraphEditor/widgets/WidgetHierarchy.vue @@ -14,6 +14,7 @@ function transformChild(child: Ast.Ast | Ast.Token) { if (props.input.value instanceof Ast.PropertyAccess && child === props.input.value.lhs) childInput.forcePort = true if ( + !(props.input.value instanceof Ast.PropertyAccess) && props.input.value instanceof Ast.OprApp && (child === props.input.value.rhs || child === props.input.value.lhs) ) diff --git a/app/gui2/src/components/GraphEditor/widgets/WidgetSelection.vue b/app/gui2/src/components/GraphEditor/widgets/WidgetSelection.vue index 472d8dc16ac7..c7e74929cfcf 100644 --- a/app/gui2/src/components/GraphEditor/widgets/WidgetSelection.vue +++ b/app/gui2/src/components/GraphEditor/widgets/WidgetSelection.vue @@ -9,15 +9,9 @@ import { import { useGraphStore } from '@/stores/graph' import { requiredImports, type RequiredImport } from '@/stores/graph/imports.ts' import { useSuggestionDbStore } from '@/stores/suggestionDatabase' -import { SuggestionKind, type SuggestionEntry } from '@/stores/suggestionDatabase/entry.ts' +import { type SuggestionEntry } from '@/stores/suggestionDatabase/entry.ts' import { ArgumentInfoKey } from '@/util/callTree' -import { - qnJoin, - qnLastSegment, - qnSegments, - tryQualifiedName, - type Identifier, -} from '@/util/qualifiedName' +import { qnLastSegment, tryQualifiedName, type Identifier } from '@/util/qualifiedName' import { computed, ref, watch } from 'vue' const props = defineProps(widgetProps(widgetDefinition)) diff --git a/app/gui2/src/providers/widgetRegistry.ts b/app/gui2/src/providers/widgetRegistry.ts index b3b631bc8ec5..991f2b4f560d 100644 --- a/app/gui2/src/providers/widgetRegistry.ts +++ b/app/gui2/src/providers/widgetRegistry.ts @@ -2,7 +2,6 @@ import { createContextStore } from '@/providers' import type { PortId } from '@/providers/portInfo' import type { WidgetConfiguration } from '@/providers/widgetRegistry/configuration' import type { GraphDb } from '@/stores/graph/graphDatabase' -import type { RequiredImport } from '@/stores/graph/imports.ts' import type { Typename } from '@/stores/suggestionDatabase/entry' import { Ast } from '@/util/ast' import { MutableModule, type Owned } from '@/util/ast/abstract.ts' @@ -169,15 +168,14 @@ export function widgetProps(_def: WidgetDefinition) { } type InputMatcherFn = (input: WidgetInput) => input is T -type InputMatcherSymbol = symbol & keyof T -type InputMatcher = InputMatcherSymbol | InputMatcherFn +type InputMatcher = keyof WidgetInput | InputMatcherFn type InputTy = M extends (infer T)[] ? InputTy : M extends InputMatcherFn ? T - : M extends symbol & keyof WidgetInput - ? WidgetInput & { [S in M]: Required[S] } + : M extends keyof WidgetInput + ? WidgetInput & Required> : never export interface WidgetOptions { diff --git a/app/gui2/src/stores/graph/__tests__/graphDatabase.test.ts b/app/gui2/src/stores/graph/__tests__/graphDatabase.test.ts index 5f1c38eee836..0e6a06d90493 100644 --- a/app/gui2/src/stores/graph/__tests__/graphDatabase.test.ts +++ b/app/gui2/src/stores/graph/__tests__/graphDatabase.test.ts @@ -4,30 +4,35 @@ import assert from 'assert' import { IdMap, type ExprId } from 'shared/yjsModel' import { expect, test } from 'vitest' +/** + * Create a predictable fake UUID which contains given number in decimal at the end. + * @param x sequential value, e.g. 15 + * @returns fake uuid, e.g. 00000000-0000-0000-0000-000000000015 + */ +function id(x: number): ExprId { + const xStr = `${x}` + return ('00000000-0000-0000-0000-000000000000'.slice(0, -xStr.length) + xStr) as ExprId +} + test('Reading graph from definition', () => { - const code = 'function a =\n node1 = a + 4\n node2 = node1 + 4' - const id01 = '00000000-0000-0000-0000-000000000001' as ExprId // function - const id02 = '00000000-0000-0000-0000-000000000002' as ExprId // a - const id03 = '00000000-0000-0000-0000-000000000003' as ExprId // node1 - const id04 = '00000000-0000-0000-0000-000000000004' as ExprId // a + 4 - const id05 = '00000000-0000-0000-0000-000000000005' as ExprId // a - const id06 = '00000000-0000-0000-0000-000000000006' as ExprId // 4 - const id07 = '00000000-0000-0000-0000-000000000007' as ExprId // node2 - const id08 = '00000000-0000-0000-0000-000000000008' as ExprId // node1 + 4 - const id09 = '00000000-0000-0000-0000-000000000009' as ExprId // node1 - const id10 = '00000000-0000-0000-0000-000000000010' as ExprId // 4 + const code = `function a = + node1 = a + 4 + node2 = node1 + 4 + node3 = node2 + 1` const idMap = IdMap.Mock() - idMap.insertKnownId([0, 8], id01) - idMap.insertKnownId([9, 10], id02) - idMap.insertKnownId([17, 22], id03) - idMap.insertKnownId([25, 30], id04) - idMap.insertKnownId([25, 26], id05) - idMap.insertKnownId([29, 30], id06) - idMap.insertKnownId([35, 40], id07) - idMap.insertKnownId([43, 52], id08) - idMap.insertKnownId([43, 48], id09) - idMap.insertKnownId([51, 52], id10) + idMap.insertKnownId([0, 8], id(1)) // function + idMap.insertKnownId([9, 10], id(2)) // a + idMap.insertKnownId([17, 22], id(3)) // node1 + idMap.insertKnownId([25, 30], id(4)) // a + 4 + idMap.insertKnownId([25, 26], id(5)) // a + idMap.insertKnownId([29, 30], id(6)) // 4 + idMap.insertKnownId([35, 40], id(7)) // node2 + idMap.insertKnownId([43, 52], id(8)) // node1 + 4 + idMap.insertKnownId([43, 48], id(9)) // node1 + idMap.insertKnownId([51, 52], id(10)) // 4 + idMap.insertKnownId([57, 62], id(11)) // node3 + idMap.insertKnownId([65, 74], id(12)) // node2 + 1 const db = GraphDb.Mock() const ast = Ast.parseTransitional(code, idMap) @@ -37,28 +42,31 @@ test('Reading graph from definition', () => { assert(func instanceof Ast.Function) db.readFunctionAst(func, (_) => ({ x: 0.0, y: 0.0, vis: null })) - expect(Array.from(db.nodeIdToNode.keys())).toEqual([id04, id08]) - expect(db.getExpressionNodeId(id04)).toBe(id04) - expect(db.getExpressionNodeId(id05)).toBe(id04) - expect(db.getExpressionNodeId(id06)).toBe(id04) - expect(db.getExpressionNodeId(id07)).toBeUndefined() - expect(db.getExpressionNodeId(id09)).toBe(id08) - expect(db.getExpressionNodeId(id10)).toBe(id08) - expect(db.getPatternExpressionNodeId(id03)).toBe(id04) - expect(db.getPatternExpressionNodeId(id04)).toBeUndefined() - expect(db.getPatternExpressionNodeId(id07)).toBe(id08) - expect(db.getPatternExpressionNodeId(id10)).toBeUndefined() - expect(db.getIdentDefiningNode('node1')).toBe(id04) - expect(db.getIdentDefiningNode('node2')).toBe(id08) + expect(Array.from(db.nodeIdToNode.keys())).toEqual([id(4), id(8), id(12)]) + expect(db.getExpressionNodeId(id(4))).toBe(id(4)) + expect(db.getExpressionNodeId(id(5))).toBe(id(4)) + expect(db.getExpressionNodeId(id(6))).toBe(id(4)) + expect(db.getExpressionNodeId(id(7))).toBeUndefined() + expect(db.getExpressionNodeId(id(9))).toBe(id(8)) + expect(db.getExpressionNodeId(id(10))).toBe(id(8)) + expect(db.getPatternExpressionNodeId(id(3))).toBe(id(4)) + expect(db.getPatternExpressionNodeId(id(4))).toBeUndefined() + expect(db.getPatternExpressionNodeId(id(7))).toBe(id(8)) + expect(db.getPatternExpressionNodeId(id(10))).toBeUndefined() + expect(db.getIdentDefiningNode('node1')).toBe(id(4)) + expect(db.getIdentDefiningNode('node2')).toBe(id(8)) expect(db.getIdentDefiningNode('function')).toBeUndefined() - expect(db.getOutputPortIdentifier(db.getNodeFirstOutputPort(id04))).toBe('node1') - expect(db.getOutputPortIdentifier(db.getNodeFirstOutputPort(id08))).toBe('node2') - expect(db.getOutputPortIdentifier(db.getNodeFirstOutputPort(id03))).toBe('node1') + expect(db.getOutputPortIdentifier(db.getNodeFirstOutputPort(id(4)))).toBe('node1') + expect(db.getOutputPortIdentifier(db.getNodeFirstOutputPort(id(8)))).toBe('node2') + expect(db.getOutputPortIdentifier(db.getNodeFirstOutputPort(id(3)))).toBe('node1') // Commented the connection from input node, as we don't support them yet. - expect(Array.from(db.connections.allForward(), ([key]) => key)).toEqual([id03]) - // expect(Array.from(db.connections.lookup(id02))).toEqual([id05]) - expect(Array.from(db.connections.lookup(id03))).toEqual([id09]) - // expect(db.getOutputPortIdentifier(id02)).toBe('a') - expect(db.getOutputPortIdentifier(id03)).toBe('node1') + expect(Array.from(db.connections.allForward(), ([key]) => key)).toEqual([id(3), id(7)]) + // expect(Array.from(db.connections.lookup(id(2)))).toEqual([id(5)]) + expect(Array.from(db.connections.lookup(id(3)))).toEqual([id(9)]) + // expect(db.getOutputPortIdentifier(id(2))).toBe('a') + expect(db.getOutputPortIdentifier(id(3))).toBe('node1') + expect(Array.from(db.dependantNodes(id(4)))).toEqual([id(8), id(12)]) + expect(Array.from(db.dependantNodes(id(8)))).toEqual([id(12)]) + expect(Array.from(db.dependantNodes(id(12)))).toEqual([]) }) diff --git a/app/gui2/src/stores/graph/graphDatabase.ts b/app/gui2/src/stores/graph/graphDatabase.ts index 06b11ccde0b6..1fda60c60b5d 100644 --- a/app/gui2/src/stores/graph/graphDatabase.ts +++ b/app/gui2/src/stores/graph/graphDatabase.ts @@ -239,6 +239,32 @@ export class GraphDb { ) } + /** + * Get a list of all nodes that depend on given node. Includes transitive dependencies. + */ + dependantNodes(id: ExprId): Set { + const toVisit = [id] + const result = new Set() + + let currentNode: ExprId | undefined + while ((currentNode = toVisit.pop())) { + const outputPorts = this.nodeOutputPorts.lookup(currentNode) + for (const outputPort of outputPorts) { + const connectedPorts = this.connections.lookup(outputPort) + for (const port of connectedPorts) { + const portNode = this.getExpressionNodeId(port) + if (portNode == null) continue + if (!result.has(portNode)) { + result.add(portNode) + toVisit.push(portNode) + } + } + } + } + + return result + } + getMethodCallInfo( id: ExprId, ): diff --git a/app/gui2/src/stores/graph/index.ts b/app/gui2/src/stores/graph/index.ts index a9fbfa1afc0d..00acb8193f56 100644 --- a/app/gui2/src/stores/graph/index.ts +++ b/app/gui2/src/stores/graph/index.ts @@ -11,14 +11,14 @@ import { } from '@/stores/graph/imports' import { useProjectStore } from '@/stores/project' import { useSuggestionDbStore } from '@/stores/suggestionDatabase' +import { assert } from '@/util/assert' import { Ast } from '@/util/ast' import type { AstId, Module, Owned } from '@/util/ast/abstract' import { MutableModule } from '@/util/ast/abstract' import { useObserveYjs } from '@/util/crdt' +import { partition } from '@/util/data/array' import type { Opt } from '@/util/data/opt' import { Rect } from '@/util/data/rect' -import type { Result } from '@/util/data/result.ts' -import { Ok } from '@/util/data/result.ts' import { Vec2 } from '@/util/data/vec2' import { map, set } from 'lib0' import { defineStore } from 'pinia' @@ -127,13 +127,17 @@ export const useGraphStore = defineStore('graph', () => { return true }) - methodAst.value = getExecutedMethodAst(newRoot, proj.executionContext.getStackTop(), db) + methodAst.value = methodAstInModule(astModule) if (methodAst.value) { currentNodeIds.value = db.readFunctionAst(methodAst.value, (id) => meta.get(id)) } }) } + function methodAstInModule(mod: Module) { + return getExecutedMethodAst(mod, proj.executionContext.getStackTop(), db) + } + useObserveYjs(metadata, (event) => { const meta = event.target for (const [id, op] of event.changes.keys) { @@ -207,11 +211,33 @@ export const useGraphStore = defineStore('graph', () => { const edit = astModule.edit() if (withImports) addMissingImports(edit, withImports) const currentFunc = 'main' - const functionBlock = Ast.functionBlock(astModule, currentFunc) - if (!functionBlock) { + const method = Ast.findModuleMethod(astModule, currentFunc) + if (!method) { console.error(`BUG: Cannot add node: No current function.`) return } + + let functionBlock: Ast.BodyBlock + + if (method.body instanceof Ast.BodyBlock) { + functionBlock = method.body + } else if (method.body != null) { + functionBlock = edit.takeAndReplaceRef(method.body.exprId, (node) => { + return Ast.BodyBlock.new([{ expression: { node } }], edit) + }) + } else { + const newMethod: Ast.Function = edit.takeAndReplaceValue(method.exprId, (func) => { + assert(func instanceof Ast.Function) + assert(func.name != null) + const name = edit.take(func.name.exprId)?.node + assert(name != null) + const body = Ast.BodyBlock.new([], edit) + return Ast.Function.new(edit, name, func.argNodes(), body) + }) + assert(newMethod.body instanceof Ast.BodyBlock) + functionBlock = newMethod.body + } + const rhs = Ast.parse(expression, edit) const assignment = Ast.Assignment.new(edit, ident, rhs) functionBlock.push(edit, assignment) @@ -237,17 +263,15 @@ export const useGraphStore = defineStore('graph', () => { commitEdit(edit) } - function deleteNode(id: ExprId) { - const node = db.nodeIdToNode.get(id) - if (!node) return - proj.module?.doc.metadata.delete(node.outerExprId) - const root = moduleRoot.value - if (!root) { - console.error(`BUG: Cannot delete node: No module root.`) - return - } + function deleteNodes(ids: ExprId[]) { const edit = astModule.edit() - edit.delete(node.outerExprId) + for (const id of ids) { + const node = db.nodeIdToNode.get(id) + if (!node) return + proj.module?.doc.metadata.delete(node.outerExprId) + nodeRects.delete(id) + edit.delete(node.outerExprId) + } commitEdit(edit) } @@ -376,11 +400,17 @@ export const useGraphStore = defineStore('graph', () => { /** * Emit an value update to a port view under specific ID. Returns `true` if the port view is * registered and the update was emitted, or `false` otherwise. + * + * NOTE: If this returns `true,` The update handlers called `graph.commitEdit` on their own. + * Therefore the passed in `edit` should not be modified afterwards, as it is already committed. */ - function updatePortValue(id: PortId, value: Owned | undefined): boolean { + function updatePortValue( + edit: MutableModule, + id: PortId, + value: Owned | undefined, + ): boolean { const update = getPortPrimaryInstance(id)?.onUpdate if (!update) return false - const edit = astModule.edit() update({ edit, portUpdate: { value, origin: id } }) return true } @@ -420,6 +450,71 @@ export const useGraphStore = defineStore('graph', () => { }) } + function editScope(scope: (edit: MutableModule) => Map> | void) { + const edit = astModule.edit() + const metadataUpdates = scope(edit) + commitEdit(edit, metadataUpdates ?? undefined) + } + + /** + * Reorders nodes so the `targetNodeId` node is placed after `sourceNodeId`. Does nothing if the + * relative order is already correct. + * + * Additionally all nodes dependent on the `targetNodeId` that end up being before its new line + * are also moved after it, keeping their relative order. + */ + function ensureCorrectNodeOrder(edit: MutableModule, sourceNodeId: ExprId, targetNodeId: ExprId) { + const sourceExpr = db.nodeIdToNode.get(sourceNodeId)?.outerExprId + const targetExpr = db.nodeIdToNode.get(targetNodeId)?.outerExprId + const body = methodAstInModule(edit)?.body + console.log('body', body) + assert(sourceExpr != null) + assert(targetExpr != null) + assert(body instanceof Ast.BodyBlock, 'Current function body must be a BodyBlock') + + const lines = body.lines() + const sourceIdx = lines.findIndex((line) => line.expression?.node.exprId === sourceExpr) + const targetIdx = lines.findIndex((line) => line.expression?.node.exprId === targetExpr) + + assert(sourceIdx != null) + assert(targetIdx != null) + + // If source is placed after its new target, the nodes needs to be reordered. + if (sourceIdx > targetIdx) { + // Find all transitive dependencies of the moved target node. + const deps = db.dependantNodes(targetNodeId) + + const dependantLines = new Set(Array.from(deps, (id) => db.nodeIdToNode.get(id)?.outerExprId)) + // Include the new target itself in the set of lines that must be placed after source node. + dependantLines.add(targetExpr) + + // Check if the source depends on target. If that's the case, the edge we are trying to make + // creates a circular dependency. Reordering doesn't make any sense in that case. + if (dependantLines.has(sourceExpr)) { + return 'circular' + } + + // Pick subset of lines to reorder, i.e. lines between and including target and source. + const linesToSort = lines.splice(targetIdx, sourceIdx - targetIdx + 1) + + // Split those lines into two buckets, whether or not they depend on the target. + const [linesAfter, linesBefore] = partition(linesToSort, (line) => + dependantLines.has(line.expression?.node.exprId), + ) + + // Recombine all lines after splitting, keeping existing dependants below the target. + lines.splice(targetIdx, 0, ...linesBefore, ...linesAfter) + + // Finally apply the reordered lines into the body block as AST edit. + const ownedBody = edit.take(body.exprId) + assert(ownedBody != null) + edit.replaceValue(ownedBody.placeholder.exprId, Ast.BodyBlock.new(lines, edit)) + return true + } else { + return false + } + } + return { transact, db: markRaw(db), @@ -441,9 +536,9 @@ export const useGraphStore = defineStore('graph', () => { clearUnconnected, moduleRoot, createNode, - deleteNode, + deleteNodes, + ensureCorrectNodeOrder, setNodeContent, - setExpression, setExpressionContent, setNodePosition, setNodeVisualizationId, @@ -459,6 +554,7 @@ export const useGraphStore = defineStore('graph', () => { setEditedNode, updateState, commitEdit, + editScope, addMissingImports, } }) @@ -481,7 +577,7 @@ export type UnconnectedEdge = { } function getExecutedMethodAst( - ast: Ast.Ast, + astModule: Module, executionStackTop: StackItem, db: GraphDb, ): Ast.Function | undefined { @@ -490,7 +586,7 @@ function getExecutedMethodAst( // Assume that the provided AST matches the module in the method pointer. There is no way to // actually verify this assumption at this point. const ptr = executionStackTop.methodPointer - return Ast.findModuleMethod(ast.module, ptr.name) ?? undefined + return Ast.findModuleMethod(astModule, ptr.name) ?? undefined } case 'LocalCall': { const exprId = executionStackTop.expressionId @@ -498,7 +594,7 @@ function getExecutedMethodAst( if (!info) return undefined const ptr = info.methodCall?.methodPointer if (!ptr) return undefined - return Ast.findModuleMethod(ast.module, ptr.name) ?? undefined + return Ast.findModuleMethod(astModule, ptr.name) ?? undefined } } } diff --git a/app/gui2/src/util/ast/__tests__/callTree.test.ts b/app/gui2/src/util/ast/__tests__/callTree.test.ts index 2375ee2e440e..8ed8abc41c55 100644 --- a/app/gui2/src/util/ast/__tests__/callTree.test.ts +++ b/app/gui2/src/util/ast/__tests__/callTree.test.ts @@ -57,7 +57,7 @@ test.each` ${'func a=x x c=x '} | ${'=a @b =c ?d'} | ${prefixFixture} ${'func a=x d=x '} | ${'=a ?b ?c =d'} | ${prefixFixture} ${'func a=x d=x b=x '} | ${'=a =d =b ?c'} | ${prefixFixture} - ${'func a=x d=x c=x '} | ${'=a =d ?b =c'} | ${prefixFixture} + ${'func a=x d=x c=x '} | ${'=a ?b =d =c'} | ${prefixFixture} ${'func a=x c=x d=x '} | ${'=a ?b =c =d'} | ${prefixFixture} ${'func b=x '} | ${'?a =b ?c ?d'} | ${prefixFixture} ${'func b=x c=x '} | ${'?a =b =c ?d'} | ${prefixFixture} @@ -76,7 +76,7 @@ test.each` ${'func x x '} | ${'@a @b ?c ?d'} | ${prefixFixture} ${'func x x x '} | ${'@a @b @c ?d'} | ${prefixFixture} ${'func x x x x '} | ${'@a @b @c @d'} | ${prefixFixture} - ${'func a=x x m=x '} | ${'=a @b =m ?c ?d'} | ${prefixFixture} + ${'func a=x x m=x '} | ${'=a @b ?c ?d =m'} | ${prefixFixture} ${'x + y'} | ${'@lhs @rhs'} | ${infixFixture} ${'x +'} | ${'@lhs ?rhs'} | ${infixFixture} `( diff --git a/app/gui2/src/util/ast/abstract.ts b/app/gui2/src/util/ast/abstract.ts index c1c4ecf5fa02..0013eb62194b 100644 --- a/app/gui2/src/util/ast/abstract.ts +++ b/app/gui2/src/util/ast/abstract.ts @@ -6,6 +6,7 @@ import type { Opt } from '@/util/data/opt' import { Err, Ok, type Result } from '@/util/data/result' import type { LazyObject } from '@/util/parserSupport' import { unsafeEntries } from '@/util/record' +import { createIterator, mapIterator } from 'lib0/iterator' import * as map from 'lib0/map' import * as random from 'lib0/random' import { reactive } from 'vue' @@ -33,6 +34,8 @@ export interface Module { edit(): MutableModule apply(module: Module): void replace(editIn: Module): void + iterAllNodeIds(): IterableIterator + iterAllNodes(): IterableIterator root(): Module } @@ -124,8 +127,48 @@ export class MutableModule implements Module { } } + iterAllNodeIds(): IterableIterator { + const parentIter = this.base?.iterAllNodeIds() + let parentDone = parentIter == null + const nodeIter = this.nodes.entries() + const editedVisited = new Set() + return createIterator(() => { + for (;;) { + if (!parentDone) { + const result = parentIter?.next() + if (result?.done) parentDone = true + else if (result?.value) { + const id = result.value + const edited = this.nodes.get(id) + if (edited !== undefined) editedVisited.add(id) + if (edited === null) continue + return result + } + } else { + const next = nodeIter.next() + if (next.done === true) return next + const [id, ast] = next.value + if (ast !== null && !editedVisited.has(id)) { + return { + done: false, + value: id, + } + } + } + } + }) + } + + iterAllNodes(): IterableIterator { + return mapIterator(this.iterAllNodeIds(), (id) => { + const node = this.get(id) + assert(node != null) + return node + }) + } + /** Modify the parent of `target` to refer to a new object instead of `target`. Return `target`, which now has no parent. */ - replaceRef(target: AstId, replacement: Owned): Owned | undefined { + replaceRef(target: AstId, replacement: Owned): Owned | undefined { const old = this.get(target) if (!old || replacement.exprId === target) { return this.replaceValue(target, replacement) @@ -153,7 +196,7 @@ export class MutableModule implements Module { /** Change the value of the object referred to by the `target` ID. (The initial ID of `replacement` will be ignored.) * Returns the old value, with a new (unreferenced) ID. */ - replaceValue(target: AstId, replacement: Owned): Owned | undefined { + replaceValue(target: AstId, replacement: Owned): Owned | undefined { const old = this.get(target) replacement.parent = old?.parent if (replacement.exprId !== target || replacement.module !== this) { @@ -187,7 +230,7 @@ export class MutableModule implements Module { return this.replaceValue(target, Wildcard.new(this)) } - takeAndReplaceRef(target: AstId, wrap: (x: Owned) => Owned): Ast { + takeAndReplaceRef(target: AstId, wrap: (x: Owned) => Owned): T { const taken = this.take(target) assert(!!taken) const replacement = wrap(taken.node) @@ -195,12 +238,12 @@ export class MutableModule implements Module { return replacement } - takeAndReplaceValue(target: AstId, wrap: (x: Owned) => Owned): Ast { + takeAndReplaceValue(target: AstId, wrap: (x: Owned) => Owned): T { const taken = this.takeValue(target) assert(!!taken) const replacement = wrap(taken) this.replaceValue(target, replacement) - return this.get(target)! + return this.get(target)! as T } /** Copy the given node and all its descendants into this module. */ @@ -986,6 +1029,51 @@ export class Function extends Ast { private readonly args_: FunctionArgument[] private readonly equals_: NodeChild private readonly body_: NodeChild | null + + /** @internal */ + static new( + module: MutableModule, + name: Owned, + args: FunctionArgument[], + body: Owned | null, + ): Owned { + const id = newAstId() + return asOwned( + new Function( + module, + id, + unspaced(makeChild(module, name, id)), + args.map((arg) => + arg.map((child) => ({ + ...child, + node: child.node instanceof Ast ? makeChild(module, child.node, id) : child.node, + })), + ), + undefined, + body != null ? unspaced(makeChild(module, body, id)) : null, + ), + ) + } + + static fromExprs( + module: MutableModule, + name: string, + args: Ast[], + exprs: Ast[], + trailingNewline?: boolean, + ): Function { + const id = newAstId() + const exprs_: BlockLine[] = exprs.map((expr) => ({ expression: { node: expr } })) + if (trailingNewline) { + exprs_.push({ newline: { node: Token.new('\n') }, expression: null }) + } + const body = BodyBlock.new(exprs_, module) + const args_ = args.map((arg) => [{ node: makeChild(module, arg, id) }]) + const ident = { node: Ident.new(module, name).exprId } + const equals = { node: Token.new('=') } + return new Function(module, id, ident, args_, equals, { node: body.exprId }) + } + // FIXME for #8367: This should not be nullable. If the `ExprId` has been deleted, the same placeholder logic should be applied // here and in `rawChildren` (and indirectly, `print`). get name(): Ast | null { @@ -994,6 +1082,11 @@ export class Function extends Ast { get body(): Ast | null { return this.body_ ? this.module.get(this.body_.node) : null } + + argNodes(): FunctionArgument[] { + return [...this.args_] + } + *bodyExpressions(): IterableIterator { const body = this.body_ ? this.module.get(this.body_.node) : null if (body instanceof BodyBlock) { @@ -1002,41 +1095,25 @@ export class Function extends Ast { yield body } } + + /** @internal */ constructor( module: MutableModule, id: AstId | undefined, name: NodeChild, args: FunctionArgument[], - equals: NodeChild, // Edits (#8367): NodeChild | undefined + equals: NodeChild | undefined, // Edits (#8367): NodeChild | undefined body: NodeChild | null, ) { super(module, id, RawAst.Tree.Type.Function) this.name_ = name this.args_ = args - this.equals_ = equals + this.equals_ = equals ?? spaced(new Token('=', newTokenId(), RawAst.Token.Type.Operator)) + this.body_ = body setParent(module, this.exprId, ...this.concreteChildren()) } - static new( - module: MutableModule, - name: string, - args: Ast[], - exprs: Ast[], - trailingNewline?: boolean, - ): Function { - const id = newAstId() - const exprs_: BlockLine[] = exprs.map((expr) => ({ expression: { node: expr } })) - if (trailingNewline) { - exprs_.push({ newline: { node: Token.new('\n') }, expression: null }) - } - const body = BodyBlock.new(exprs_, module) - const args_ = args.map((arg) => [{ node: makeChild(module, arg, id) }]) - const ident = { node: Ident.new(module, name).exprId } - const equals = { node: Token.new('=') } - return new Function(module, id, ident, args_, equals, { node: body.exprId }) - } - *concreteChildren(): IterableIterator { yield this.name_ for (const arg of this.args_) yield* arg @@ -1604,7 +1681,7 @@ export function tokenTreeWithIds(root: Ast): TokenTree { export function moduleMethodNames(module: Module): Set { const result = new Set() - for (const node of module.raw.nodes.values()) { + for (const node of module.iterAllNodes()) { if (node instanceof Function && node.name) { result.add(node.name.code()) } @@ -1614,7 +1691,7 @@ export function moduleMethodNames(module: Module): Set { // FIXME: We should use alias analysis to handle ambiguous names correctly. export function findModuleMethod(module: Module, name: string): Function | null { - for (const node of module.raw.nodes.values()) { + for (const node of module.iterAllNodes()) { if (node instanceof Function) { if (node.name && node.name.code() === name) { return node diff --git a/app/gui2/src/util/callTree.ts b/app/gui2/src/util/callTree.ts index 450491bd6a15..6225a693ac59 100644 --- a/app/gui2/src/util/callTree.ts +++ b/app/gui2/src/util/callTree.ts @@ -4,8 +4,9 @@ import type { WidgetConfiguration } from '@/providers/widgetRegistry/configurati import * as widgetCfg from '@/providers/widgetRegistry/configuration' import type { SuggestionEntry, SuggestionEntryArgument } from '@/stores/suggestionDatabase/entry' import { Ast } from '@/util/ast' -import { tryGetIndex } from '@/util/data/array' +import { findLastIndex, tryGetIndex } from '@/util/data/array' import type { MethodCall } from 'shared/languageServerTypes' +import { assert } from './assert' export const enum ApplicationKind { Prefix, @@ -188,14 +189,10 @@ export class ArgumentApplication { callInfo: CallInfo, stripSelfArgument: boolean, ) { - const { noArgsCall, appMethodCall, suggestion, widgetCfg } = callInfo - const kind = ApplicationKind.Prefix + const { noArgsCall, suggestion, widgetCfg } = callInfo const callId = interpreted.func.exprId const knownArguments = suggestion?.arguments - const notAppliedArguments = appMethodCall?.notAppliedArguments ?? [] - const placeholdersToInsert = notAppliedArguments.slice() - const notAppliedSet = new Set(notAppliedArguments) const allPossiblePrefixArguments = Array.from(knownArguments ?? [], (_, i) => i) // when this is a method application with applied 'self', the subject of the access operator is @@ -210,68 +207,120 @@ export class ArgumentApplication { const notAppliedOriginally = new Set( noArgsCall?.notAppliedArguments ?? allPossiblePrefixArguments, ) - const argumentsLeftToMatch = allPossiblePrefixArguments.filter( - (i) => !notAppliedSet.has(i) && notAppliedOriginally.has(i), + const argumentsLeftToMatch = allPossiblePrefixArguments.filter((i) => + notAppliedOriginally.has(i), ) - const prefixArgsToDisplay: Array<{ + const resolvedArgs: Array<{ appTree: Ast.Ast argument: ArgumentAst | ArgumentPlaceholder }> = [] - function insertPlaceholdersUpto(index: number, appTree: Ast.Ast) { - let canInsertPositional = true - while (placeholdersToInsert[0] != null && placeholdersToInsert[0] < index) { - const argIndex = placeholdersToInsert.shift() - const argInfo = tryGetIndex(knownArguments, argIndex) - if (argIndex != null && argInfo != null) { - prefixArgsToDisplay.push({ - appTree, - argument: ArgumentPlaceholder.WithRetrievedConfig( - callId, - argIndex, - argInfo, - ApplicationKind.Prefix, - !canInsertPositional, - widgetCfg, - ), - }) - - canInsertPositional = false - } - } + function nextArgumentNameInDefinition() { + return tryGetIndex(knownArguments, argumentsLeftToMatch[0])?.name + } + + function takeNextArgumentFromDefinition() { + const index = argumentsLeftToMatch.shift() + const info = tryGetIndex(knownArguments, index) + return index != null && info != null ? { index, info } : undefined + } + + function takeNamedArgumentFromDefinition(name: string) { + const takeIdx = argumentsLeftToMatch.findIndex( + (id) => tryGetIndex(knownArguments, id)?.name === name, + ) + const index = argumentsLeftToMatch.splice(takeIdx, 1)[0] + const info = tryGetIndex(knownArguments, index) + return index != null && info != null ? { index, info } : undefined } - for (const realArg of interpreted.args) { - if (realArg.argName == null) { - const argIndex = argumentsLeftToMatch.shift() - if (argIndex != null) insertPlaceholdersUpto(argIndex, realArg.appTree.function) - prefixArgsToDisplay.push({ - appTree: realArg.appTree, + function putBackArgument(index: number) { + argumentsLeftToMatch.unshift(index) + } + + const lastPositionalArgIndex = findLastIndex(interpreted.args, (arg) => arg.argName == null) + + let placeholderAlreadyInserted = false + + let nextArgDefinition: ReturnType + + // Always insert a placeholder for the missing argument at the first position that is legal + // and don't invalidate further positional arguments, treating the named arguments at correct + // position as if they were positional. + for (let position = 0; position < interpreted.args.length; ++position) { + const argumentInCode = interpreted.args[position] + assert(!!argumentInCode) + const pastPositionalArguments = position > (lastPositionalArgIndex ?? -1) + + if ( + pastPositionalArguments && + argumentInCode.argName != null && + argumentInCode.argName !== nextArgumentNameInDefinition() + ) { + // Named argument that is not in its natural position, and there are no more + // positional arguments to emit in the chain. At this point placeholders can be + // inserted. We need to figure out which placeholders can be inserted before + // emitting this named argument. + + // all remaining arguments must be named, as we are past all positional arguments. + const remainingAppliedArguments = interpreted.args.slice(position) + + // For each subsequent argument in its current natural position, insert a + // placeholder. Do that only if the argument is not defined further in the chain. + while ((nextArgDefinition = takeNextArgumentFromDefinition())) { + const { index, info } = nextArgDefinition + const isAppliedFurther = remainingAppliedArguments.some( + (arg) => arg.argName === info.name, + ) + if (isAppliedFurther) { + putBackArgument(index) + break + } else { + resolvedArgs.push({ + appTree: argumentInCode.appTree.function, + argument: ArgumentPlaceholder.WithRetrievedConfig( + callId, + index, + info, + ApplicationKind.Prefix, + placeholderAlreadyInserted, + widgetCfg, + ), + }) + placeholderAlreadyInserted = true + } + } + + // Finally, we want to emit the named argument and remove it from the list of + // remaining known params. + const { index, info } = takeNamedArgumentFromDefinition(argumentInCode.argName) ?? {} + resolvedArgs.push({ + appTree: argumentInCode.appTree, argument: ArgumentAst.WithRetrievedConfig( - realArg.argument, - argIndex, - tryGetIndex(knownArguments, argIndex), - kind, + argumentInCode.argument, + index, + info ?? unknownArgInfoNamed(argumentInCode.argName), + ApplicationKind.Prefix, widgetCfg, ), }) } else { - // When name is present, we need to find the argument with the same name in the list of - // known arguments. - const name = realArg.argName - const foundIdx = argumentsLeftToMatch.findIndex((i) => knownArguments?.[i]?.name === name) - const argIndex = foundIdx === -1 ? undefined : argumentsLeftToMatch.splice(foundIdx, 1)[0] - - if (argIndex != null && foundIdx === 0) - insertPlaceholdersUpto(argIndex, realArg.appTree.function) - prefixArgsToDisplay.push({ - appTree: realArg.appTree, + const argumentFromDefinition = + argumentInCode.argName == null + ? takeNextArgumentFromDefinition() + : takeNamedArgumentFromDefinition(argumentInCode.argName) + const { index, info } = argumentFromDefinition ?? {} + resolvedArgs.push({ + appTree: argumentInCode.appTree, argument: ArgumentAst.WithRetrievedConfig( - realArg.argument, - argIndex, - tryGetIndex(knownArguments, argIndex) ?? unknownArgInfoNamed(name), - kind, + argumentInCode.argument, + index, + info ?? + (argumentInCode.argName != null + ? unknownArgInfoNamed(argumentInCode.argName) + : undefined), + ApplicationKind.Prefix, widgetCfg, ), }) @@ -279,9 +328,24 @@ export class ArgumentApplication { } const outerApp = interpreted.args[interpreted.args.length - 1]?.appTree ?? interpreted.func - insertPlaceholdersUpto(Infinity, outerApp) + // If there are any remaining known parameters, they must be inserted as trailing placeholders. + while ((nextArgDefinition = takeNextArgumentFromDefinition())) { + const { index, info } = nextArgDefinition + resolvedArgs.push({ + appTree: outerApp, + argument: ArgumentPlaceholder.WithRetrievedConfig( + callId, + index, + info, + ApplicationKind.Prefix, + placeholderAlreadyInserted, + widgetCfg, + ), + }) + placeholderAlreadyInserted = true + } - return prefixArgsToDisplay.reduce( + return resolvedArgs.reduce( (target: ArgumentApplication | Ast.Ast, toDisplay) => new ArgumentApplication(toDisplay.appTree, target, undefined, toDisplay.argument), interpreted.func, diff --git a/app/gui2/src/util/data/array.ts b/app/gui2/src/util/data/array.ts index c5d8dbf15134..cda1b7dd7884 100644 --- a/app/gui2/src/util/data/array.ts +++ b/app/gui2/src/util/data/array.ts @@ -55,3 +55,31 @@ export function byteArraysEqual(a: Opt, b: Opt): boolean export function arrayEquals(a: T[], b: T[]): boolean { return a === b || (a.length === b.length && a.every((v, i) => v === b[i])) } + +/** + * Return the rightmost index of an array element that passes the predicate. Returns `undefined` if + * no such element has been found. + */ +export function findLastIndex(array: T[], pred: (elem: T) => boolean): number | undefined { + for (let i = array.length - 1; i >= 0; --i) { + if (pred(array[i]!)) return i + } +} + +/** + * Split array into two arrays based on predicate. + * + * The predicate passed to `partition` can return true, or false. `partition` returns a pair, all of + * the elements for which it returned true, and all of the elements for which it returned false. + */ +export function partition(array: T[], pred: (elem: T) => boolean): [T[], T[]] { + const truthy: T[] = [] + const falsy: T[] = [] + + for (const element of array) { + const target = pred(element) ? truthy : falsy + target.push(element) + } + + return [truthy, falsy] +} diff --git a/app/gui2/src/util/data/opt.ts b/app/gui2/src/util/data/opt.ts index 89fd301d9bbb..968c03b25b15 100644 --- a/app/gui2/src/util/data/opt.ts +++ b/app/gui2/src/util/data/opt.ts @@ -19,3 +19,7 @@ export function isSome(value: Opt): value is T { export function isNone(value: Opt): value is null | undefined { return value == null } + +export function mapOr(optional: Opt, fallback: R, mapper: (value: T) => R): R { + return isSome(optional) ? mapper(optional) : fallback +}