[GUI2] Implement proper connect and disconnect operation on argument …

…placeholders (#8731)

- Correctly applying code edits when connecting to argument placeholders or disconnecting existing arguments.
- Prevented the placeholders from temporarily flickering right after the edit operation.
- Rewritten the placeholder argument placing logic to match the old GUI exactly - the argument should never move right after being connected.
- Fixes #8494
- Fixed some cases of ports either not existing, or existing where they shouldn't.

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

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)

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.`)
+      }
     }
   }
 }

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'

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<ExprId>, 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. */

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(() => {

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))

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<Opt<ExprId>>(() => {
     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,14 +176,33 @@ 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 })
       return true
     } 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. */
 

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)
   )

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))

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<T extends WidgetInput>(_def: WidgetDefinition<T>) {
 }
 
 type InputMatcherFn<T extends WidgetInput> = (input: WidgetInput) => input is T
-type InputMatcherSymbol<T extends WidgetInput> = symbol & keyof T
-type InputMatcher<T extends WidgetInput> = InputMatcherSymbol<T> | InputMatcherFn<T>
+type InputMatcher<T extends WidgetInput> = keyof WidgetInput | InputMatcherFn<T>
 
 type InputTy<M> = M extends (infer T)[]
   ? InputTy<T>
   : M extends InputMatcherFn<infer T>
   ? T
-  : M extends symbol & keyof WidgetInput
-  ? WidgetInput & { [S in M]: Required<WidgetInput>[S] }
+  : M extends keyof WidgetInput
+  ? WidgetInput & Required<Pick<WidgetInput, M>>
   : never
 
 export interface WidgetOptions<T extends WidgetInput> {