Copy paste multiple nodes (#10194)

Closes #9424


https://github.com/enso-org/enso/assets/6566674/c0c5a524-ea6a-41f6-9fea-c94431211f33

CHANGELOG.md

@@ -4,11 +4,13 @@
 
 - [Arrows navigation][10179] selected nodes may be moved around, or entire scene
   if no node is selected.
+- [Copy-pasting multiple nodes][10194].
 - The documentation editor has [formatting toolbars][10064].
 - The documentation editor supports [rendering images][10205].
 
 [10064]: https://github.com/enso-org/enso/pull/10064
 [10179]: https://github.com/enso-org/enso/pull/10179
+[10194]: https://github.com/enso-org/enso/pull/10194
 [10205]: https://github.com/enso-org/enso/pull/10205
 
 #### Enso Standard Library

app/gui2/e2e/nodeClipboard.spec.ts

@@ -3,6 +3,11 @@ import * as actions from './actions'
 import { expect } from './customExpect'
 import { CONTROL_KEY } from './keyboard'
 import * as locate from './locate'
+import { edgesFromNodeWithBinding, edgesToNodeWithBinding } from './locate'
+
+/** Every edge consists of multiple parts.
+ * See e2e/edgeRendering.spec.ts for explanation. */
+const EDGE_PARTS = 2
 
 test.beforeEach(async ({ page }) => {
   await page.addInitScript(() => {
@@ -55,7 +60,7 @@ test('Copy multiple nodes', async ({ page }) => {
   // Select some nodes.
   const node1 = locate.graphNodeByBinding(page, 'final')
   await node1.click()
-  const node2 = locate.graphNodeByBinding(page, 'data')
+  const node2 = locate.graphNodeByBinding(page, 'prod')
   await node2.click({ modifiers: ['Shift'] })
   await expect(node1).toBeSelected()
   await expect(node2).toBeSelected()
@@ -68,5 +73,16 @@ test('Copy multiple nodes', async ({ page }) => {
 
   // Nodes and comment have been copied.
   await expect(locate.graphNode(page)).toHaveCount(originalNodes + 2)
+  // `final` node has a comment.
   await expect(page.locator('.GraphNodeComment')).toHaveCount(originalNodeComments + 1)
+  // Check that two copied nodes are isolated, i.e. connected to each other, not original nodes.
+  await expect(locate.graphNodeByBinding(page, 'prod1')).toBeVisible()
+  await expect(locate.graphNodeByBinding(page, 'final1')).toBeVisible()
+  await expect(await edgesFromNodeWithBinding(page, 'sum')).toHaveCount(2 * EDGE_PARTS)
+  await expect(await edgesFromNodeWithBinding(page, 'prod')).toHaveCount(1 * EDGE_PARTS)
+
+  await expect(await edgesToNodeWithBinding(page, 'prod')).toHaveCount(1 * EDGE_PARTS)
+  await expect(await edgesToNodeWithBinding(page, 'final')).toHaveCount(1 * EDGE_PARTS)
+  await expect(await edgesToNodeWithBinding(page, 'prod1')).toHaveCount(1 * EDGE_PARTS)
+  await expect(await edgesToNodeWithBinding(page, 'final1')).toHaveCount(1 * EDGE_PARTS)
 })

app/gui2/src/components/GraphEditor/clipboard.ts

@@ -1,7 +1,8 @@
-import type { NodeCreation } from '@/composables/nodeCreation'
+import type { NodeCreationOptions } from '@/composables/nodeCreation'
 import type { GraphStore, Node, NodeId } from '@/stores/graph'
 import { Ast } from '@/util/ast'
 import { Pattern } from '@/util/ast/match'
+import { Vec2 } from '@/util/data/vec2'
 import type { ToValue } from '@/util/reactivity'
 import type { NodeMetadataFields } from 'shared/ast'
 import { computed, toValue } from 'vue'
@@ -19,6 +20,7 @@ interface ClipboardData {
 /** Node data that is copied to the clipboard. Used for serializing and deserializing the node information. */
 interface CopiedNode {
   expression: string
+  binding?: string
   documentation?: string | undefined
   metadata?: NodeMetadataFields
 }
@@ -28,6 +30,7 @@ function nodeStructuredData(node: Node): CopiedNode {
     expression: node.innerExpr.code(),
     documentation: node.documentation,
     metadata: node.rootExpr.serializeMetadata(),
+    ...(node.pattern ? { binding: node.pattern.code() } : {}),
   }
 }
 
@@ -120,12 +123,13 @@ function getClipboard() {
 export function useGraphEditorClipboard(
   graphStore: GraphStore,
   selected: ToValue<Set<NodeId>>,
-  createNodes: NodeCreation['createNodes'],
+  createNodes: (nodesOptions: Iterable<NodeCreationOptions>) => void,
 ) {
   /** Copy the content of the selected node to the clipboard. */
   function copySelectionToClipboard() {
     const nodes = new Array<Node>()
-    for (const id of toValue(selected)) {
+    const ids = graphStore.pickInCodeOrder(toValue(selected))
+    for (const id of ids) {
       const node = graphStore.db.nodeIdToNode.get(id)
       if (!node) continue
       nodes.push(node)
@@ -144,13 +148,20 @@ export function useGraphEditorClipboard(
       console.warn('No valid node in clipboard.')
       return
     }
+    const firstNodePos = clipboardData[0]!.metadata?.position ?? { x: 0, y: 0 }
+    const originPos = new Vec2(firstNodePos.x, firstNodePos.y)
     createNodes(
-      clipboardData.map(({ expression, documentation, metadata }) => ({
+      clipboardData.map(({ expression, binding, documentation, metadata }) => {
-        placement: { type: 'mouse' },
+        const pos = metadata?.position
+        const relativePos = pos ? new Vec2(pos.x, pos.y).sub(originPos) : new Vec2(0, 0)
+        return {
+          placement: { type: 'mouseRelative', posOffset: relativePos },
           expression,
+          binding,
           metadata,
           documentation,
-      })),
+        }
+      }),
     )
   }
 

app/gui2/src/composables/nodeCreation.ts

@@ -10,6 +10,7 @@ import { asNodeId } from '@/stores/graph/graphDatabase'
 import type { RequiredImport } from '@/stores/graph/imports'
 import type { Typename } from '@/stores/suggestionDatabase/entry'
 import { Ast } from '@/util/ast'
+import { isIdentifier, substituteIdentifier, type Identifier } from '@/util/ast/abstract'
 import { partition } from '@/util/data/array'
 import { filterDefined } from '@/util/data/iterable'
 import { Rect } from '@/util/data/rect'
@@ -25,6 +26,7 @@ export type NodeCreation = ReturnType<typeof useNodeCreation>
 type GraphIndependentPlacement =
   | { type: 'fixed'; position: Vec2 }
   | { type: 'mouse' }
+  | { type: 'mouseRelative'; posOffset: Vec2 }
   | { type: 'mouseEvent'; position: Vec2 }
 type GraphAwarePlacement = { type: 'viewport' } | { type: 'source'; node: NodeId }
 export type PlacementStrategy = GraphIndependentPlacement | GraphAwarePlacement
@@ -44,6 +46,7 @@ function isIndependent(
 export interface NodeCreationOptions<Placement extends PlacementStrategy = PlacementStrategy> {
   placement: Placement
   expression: string
+  binding?: string | undefined
   type?: Typename | undefined
   documentation?: string | undefined
   metadata?: Ast.NodeMetadataFields | undefined
@@ -61,10 +64,16 @@ export function useNodeCreation(
     return pos ? mouseDictatedPlacement(pos) : undefined
   }
 
+  function tryMouseRelative(offset: Vec2) {
+    const pos = toValue(sceneMousePos)
+    return pos ? mouseDictatedPlacement(pos.add(offset)) : undefined
+  }
+
   function placeNode(placement: PlacementStrategy, place: (nodes?: Iterable<Rect>) => Vec2): Vec2 {
     return (
       placement.type === 'viewport' ? place()
       : placement.type === 'mouse' ? tryMouse() ?? place()
+      : placement.type === 'mouseRelative' ? tryMouseRelative(placement.posOffset) ?? place()
       : placement.type === 'mouseEvent' ? mouseDictatedPlacement(placement.position)
       : placement.type === 'source' ? place(filterDefined([graphStore.visibleArea(placement.node)]))
       : placement.type === 'fixed' ? placement.position
@@ -113,10 +122,21 @@ export function useNodeCreation(
       return new Set()
     }
     const created = new Set<NodeId>()
+    const createdIdentifiers = new Set<Identifier>()
+    const identifiersRenameMap = new Map<Identifier, Identifier>()
     graphStore.edit((edit) => {
       const statements = new Array<Ast.Owned>()
       for (const options of placedNodes) {
-        const { rootExpression, id } = newAssignmentNode(edit, options)
+        const rhs = Ast.parse(options.expression, edit)
+        const ident = getIdentifier(rhs, options, createdIdentifiers)
+        createdIdentifiers.add(ident)
+        const { id, rootExpression } = newAssignmentNode(
+          edit,
+          ident,
+          rhs,
+          options,
+          identifiersRenameMap,
+        )
         statements.push(rootExpression)
         created.add(id)
         assert(options.metadata?.position != null, 'Node should already be placed')
@@ -127,29 +147,65 @@ export function useNodeCreation(
     onCreated(created)
   }
 
+  /** We resolve import conflicts and substitute identifiers if needed. */
+  function afterCreation(
+    edit: Ast.MutableModule,
+    assignment: Ast.MutableAssignment,
+    ident: Ast.Identifier,
+    options: NodeCreationOptions,
+    identifiersRenameMap: Map<Ast.Identifier, Ast.Identifier>,
+  ) {
+    // When nodes are copied, we need to substitute original names with newly assigned.
+    if (options.binding) {
+      if (isIdentifier(options.binding) && options.binding !== ident)
+        identifiersRenameMap.set(options.binding, ident)
+    }
+    for (const [old, replacement] of identifiersRenameMap.entries()) {
+      substituteIdentifier(assignment, old, replacement)
+    }
+
+    // Resolve import conflicts.
+    const conflicts = graphStore.addMissingImports(edit, options.requiredImports ?? []) ?? []
+    for (const _conflict of conflicts) {
+      // TODO: Substitution does not work, because we interpret imports wrongly. To be fixed in
+      // https://github.com/enso-org/enso/issues/9356
+      // substituteQualifiedName(assignment, conflict.pattern, conflict.fullyQualified)
+    }
+  }
+
   function createNode(options: NodeCreationOptions) {
     createNodes([options])
   }
 
-  function newAssignmentNode(edit: Ast.MutableModule, options: NodeCreationOptions) {
+  function newAssignmentNode(
-    const conflicts = graphStore.addMissingImports(edit, options.requiredImports ?? []) ?? []
+    edit: Ast.MutableModule,
-    const rhs = Ast.parse(options.expression, edit)
+    ident: Ast.Identifier,
-    const inferredPrefix = inferPrefixFromAst(rhs)
+    rhs: Ast.Owned,
-    const namePrefix = options.type ? typeToPrefix(options.type) : inferredPrefix
+    options: NodeCreationOptions,
-    const ident = graphStore.generateLocallyUniqueIdent(namePrefix)
+    identifiersRenameMap: Map<Ast.Identifier, Ast.Identifier>,
+  ) {
     rhs.setNodeMetadata(options.metadata ?? {})
     const assignment = Ast.Assignment.new(edit, ident, rhs)
-    for (const _conflict of conflicts) {
+    afterCreation(edit, assignment, ident, options, identifiersRenameMap)
-      // TODO: Substitution does not work, because we interpret imports wrongly. To be fixed in
-      // https://github.com/enso-org/enso/issues/9356
-      // substituteQualifiedName(edit, assignment, conflict.pattern, conflict.fullyQualified)
-    }
     const id = asNodeId(rhs.id)
     const rootExpression =
       options.documentation != null ?
         Ast.Documented.new(options.documentation, assignment)
       : assignment
-    return { rootExpression, id, inferredType: inferredPrefix }
+    return { rootExpression, id }
+  }
+
+  function getIdentifier(
+    expr: Ast.Ast,
+    options: NodeCreationOptions,
+    alreadyCreated: Set<Ast.Identifier>,
+  ): Ast.Identifier {
+    const namePrefix =
+      options.binding ? existingNameToPrefix(options.binding)
+      : options.type ? typeToPrefix(options.type)
+      : inferPrefixFromAst(expr)
+    const ident = graphStore.generateLocallyUniqueIdent(namePrefix, alreadyCreated)
+    return ident
   }
 
   return { createNode, createNodes, placeNode }
@@ -187,6 +243,12 @@ function typeToPrefix(type: Typename): string {
   }
 }
 
+/** Strip number suffix from binding name, effectively returning a valid prefix.
+ * The reverse of graphStore.generateLocallyUniqueIdent */
+function existingNameToPrefix(name: string): string {
+  return name.replace(/\d+$/, '')
+}
+
 /** Insert the given statements into the given block, at a location appropriate for new nodes.
  *
  * The location will be after any statements in the block that bind identifiers; if the block ends in an expression

app/gui2/src/stores/graph/graphDatabase.ts

@@ -138,6 +138,10 @@ export class GraphDb {
     private valuesRegistry: ComputedValueRegistry,
   ) {}
 
+  private nodeIdToOuterExprIds = new ReactiveIndex(this.nodeIdToNode, (id, entry) => {
+    return [[id, entry.outerExpr.id]]
+  })
+
   private nodeIdToPatternExprIds = new ReactiveIndex(this.nodeIdToNode, (id, entry) => {
     const exprs: AstId[] = []
     if (entry.pattern) entry.pattern.visitRecursiveAst((ast) => void exprs.push(ast.id))
@@ -202,7 +206,7 @@ export class GraphDb {
     return Array.from(ports, (port) => [id, port])
   })
 
-  nodeMainSuggestion = new ReactiveMapping(this.nodeIdToNode, (id, entry) => {
+  nodeMainSuggestion = new ReactiveMapping(this.nodeIdToNode, (_id, entry) => {
     const expressionInfo = this.getExpressionInfo(entry.innerExpr.id)
     const method = expressionInfo?.methodCall?.methodPointer
     if (method == null) return
@@ -230,6 +234,10 @@ export class GraphDb {
     }
   }
 
+  getOuterExpressionNodeId(exprId: AstId | undefined): NodeId | undefined {
+    return exprId && set.first(this.nodeIdToOuterExprIds.reverseLookup(exprId))
+  }
+
   getExpressionNodeId(exprId: AstId | undefined): NodeId | undefined {
     return exprId && set.first(this.nodeIdToExprIds.reverseLookup(exprId))
   }

app/gui2/src/stores/graph/index.ts

@@ -17,7 +17,7 @@ import { type SuggestionDbStore } from '@/stores/suggestionDatabase'
 import { assert, bail } from '@/util/assert'
 import { Ast } from '@/util/ast'
 import type { AstId } from '@/util/ast/abstract'
-import { MutableModule, isIdentifier } from '@/util/ast/abstract'
+import { MutableModule, isIdentifier, type Identifier } from '@/util/ast/abstract'
 import { RawAst, visitRecursive } from '@/util/ast/raw'
 import { partition } from '@/util/data/array'
 import { filterDefined } from '@/util/data/iterable'
@@ -234,14 +234,21 @@ export const { injectFn: useGraphStore, provideFn: provideGraphStore } = createC
       return Ok(method)
     }
 
-    function generateLocallyUniqueIdent(prefix?: string | undefined) {
+    /** Generate unique identifier from `prefix` and some numeric suffix.
+     * @param prefix - of the identifier
+     * @param ignore - a list of identifiers to consider as unavailable. Useful when creating multiple identifiers in a batch.
+     * */
+    function generateLocallyUniqueIdent(
+      prefix?: string | undefined,
+      ignore: Set<Identifier> = new Set(),
+    ): Identifier {
       // FIXME: This implementation is not robust in the context of a synchronized document,
       // as the same name can likely be assigned by multiple clients.
       // Consider implementing a mechanism to repair the document in case of name clashes.
       for (let i = 1; ; i++) {
         const ident = (prefix ?? FALLBACK_BINDING_PREFIX) + i
         assert(isIdentifier(ident))
-        if (!db.identifierUsed(ident)) return ident
+        if (!db.identifierUsed(ident) && !ignore.has(ident)) return ident
       }
     }
 
@@ -387,7 +394,7 @@ export const { injectFn: useGraphStore, provideFn: provideGraphStore } = createC
           for (const _conflict of conflicts) {
             // TODO: Substitution does not work, because we interpret imports wrongly. To be fixed in
             // https://github.com/enso-org/enso/issues/9356
-            // substituteQualifiedName(edit, wholeAssignment, conflict.pattern, conflict.fullyQualified)
+            // substituteQualifiedName(wholeAssignment, conflict.pattern, conflict.fullyQualified)
           }
         }
       })
@@ -665,6 +672,20 @@ export const { injectFn: useGraphStore, provideFn: provideGraphStore } = createC
       proj.computedValueRegistry.processUpdates([update_])
     }
 
+    /** Iterate over code lines, return node IDs from `ids` set in the order of code positions. */
+    function pickInCodeOrder(ids: Set<NodeId>): NodeId[] {
+      assert(syncModule.value != null)
+      const func = unwrap(getExecutedMethodAst(syncModule.value))
+      const body = func.bodyExpressions()
+      const result: NodeId[] = []
+      for (const expr of body) {
+        const id = expr?.id
+        const nodeId = db.getOuterExpressionNodeId(id)
+        if (nodeId && ids.has(nodeId)) result.push(nodeId)
+      }
+      return result
+    }
+
     /**
      * Reorders nodes so the `targetNodeId` node is placed after `sourceNodeId`. Does nothing if the
      * relative order is already correct.
@@ -758,6 +779,7 @@ export const { injectFn: useGraphStore, provideFn: provideGraphStore } = createC
       disconnectTarget,
       moduleRoot,
       deleteNodes,
+      pickInCodeOrder,
       ensureCorrectNodeOrder,
       batchEdits,
       overrideNodeColor,

app/gui2/src/util/ast/__tests__/abstract.test.ts

@@ -4,14 +4,13 @@ import {
   MutableModule,
   TextLiteral,
   escapeTextLiteral,
+  substituteIdentifier,
   substituteQualifiedName,
   unescapeTextLiteral,
   type Identifier,
 } from '@/util/ast/abstract'
-import { tryQualifiedName } from '@/util/qualifiedName'
 import { fc, test } from '@fast-check/vitest'
 import { initializeFFI } from 'shared/ast/ffi'
-import { unwrap } from 'shared/util/data/result'
 import { describe, expect } from 'vitest'
 import { findExpressions, testCase, tryFindExpressions } from './testCase'
 
@@ -853,18 +852,74 @@ test.each([
     substitution: 'ShouldNotWork',
     expected: 'Data.Table.new',
   },
-])('Substitute $pattern insde $original', ({ original, pattern, substitution, expected }) => {
+])(
+  'Substitute qualified name $pattern inside $original',
+  ({ original, pattern, substitution, expected }) => {
     const expression = Ast.parse(original)
-  expression.module.replaceRoot(expression)
+    const module = expression.module
+    module.replaceRoot(expression)
     const edit = expression.module.edit()
-  substituteQualifiedName(
+    substituteQualifiedName(expression, pattern as Ast.Identifier, substitution as Ast.Identifier)
-    edit,
+    module.applyEdit(edit)
-    expression,
+    expect(module.root()?.code()).toEqual(expected)
-    pattern as Ast.QualifiedName,
+  },
-    unwrap(tryQualifiedName(substitution)),
+)
-  )
+
-  expect(edit.root()?.code()).toEqual(expected)
+test.each([
-})
+  {
+    original: 'some_name',
+    pattern: 'some_name',
+    substitution: 'other_name',
+    expected: 'other_name',
+  },
+  {
+    original: 'x = Table.from_vec (node1.new 1 2 3)',
+    pattern: 'node1',
+    substitution: 'node2',
+    expected: 'x = Table.from_vec (node2.new 1 2 3)',
+  },
+  {
+    original: 'x = some_func "node1"',
+    pattern: 'node1',
+    substitution: 'node2',
+    expected: 'x = some_func "node1"',
+  },
+  {
+    original: 'x + y',
+    pattern: 'x',
+    substitution: 'z',
+    expected: 'z + y',
+  },
+  {
+    original: 'node1.node2.node3',
+    pattern: 'node2',
+    substitution: 'ShouldNotWork',
+    expected: 'node1.node2.node3',
+  },
+  {
+    original: 'node1.node2.node3',
+    pattern: 'node3',
+    substitution: 'ShouldNotWork',
+    expected: 'node1.node2.node3',
+  },
+  {
+    original: '.node1',
+    pattern: 'node1',
+    substitution: 'ShouldNotWork',
+    expected: '.node1',
+  },
+])(
+  'Substitute identifier $pattern inside $original',
+  ({ original, pattern, substitution, expected }) => {
+    const expression = Ast.parse(original)
+    const module = expression.module
+    module.replaceRoot(expression)
+    const edit = expression.module.edit()
+    substituteIdentifier(expression, pattern as Ast.Identifier, substitution as Ast.Identifier)
+    module.applyEdit(edit)
+    expect(module.root()?.code()).toEqual(expected)
+  },
+)
 
 test.each([
   ['', ''],

app/gui2/src/util/ast/abstract.ts

@@ -15,7 +15,9 @@ import {
   Function,
   Ident,
   MutableBodyBlock,
+  MutableIdent,
   MutableModule,
+  MutablePropertyAccess,
   NumericLiteral,
   OprApp,
   PropertyAccess,
@@ -165,29 +167,51 @@ export function parseQualifiedName(ast: Ast): QualifiedName | null {
   return idents && normalizeQualifiedName(qnFromSegments(idents))
 }
 
-/* Substitute `pattern` inside `expression` with `to`.
+/** Substitute `pattern` inside `expression` with `to`.
+ * Will only replace the first item in the property acccess chain. */
+export function substituteIdentifier(
+  expr: MutableAst,
+  pattern: IdentifierOrOperatorIdentifier,
+  to: IdentifierOrOperatorIdentifier,
+) {
+  if (expr instanceof MutableIdent && expr.code() === pattern) {
+    expr.setToken(to)
+  } else if (expr instanceof MutablePropertyAccess) {
+    // Substitute only the first item in the property access chain.
+    if (expr.lhs != null) substituteIdentifier(expr.lhs, pattern, to)
+  } else {
+    for (const child of expr.children()) {
+      if (child instanceof Token) {
+        continue
+      }
+      const mutableChild = expr.module.getVersion(child)
+      substituteIdentifier(mutableChild, pattern, to)
+    }
+  }
+}
+
+/** Substitute `pattern` inside `expression` with `to`.
  * Replaces identifier, the whole qualified name, or the beginning of the qualified name (first segments of property access chain). */
 export function substituteQualifiedName(
-  module: MutableModule,
+  expr: MutableAst,
-  expression: Ast,
   pattern: QualifiedName | IdentifierOrOperatorIdentifier,
   to: QualifiedName,
 ) {
-  const expr = module.getVersion(expression) ?? expression
+  if (expr instanceof MutablePropertyAccess || expr instanceof MutableIdent) {
-  if (expr instanceof PropertyAccess || expr instanceof Ident) {
     const qn = parseQualifiedName(expr)
     if (qn === pattern) {
-      expr.updateValue(() => Ast.parse(to, module))
+      expr.updateValue(() => Ast.parse(to, expr.module))
     } else if (qn && qn.startsWith(pattern)) {
       const withoutPattern = qn.replace(pattern, '')
-      expr.updateValue(() => Ast.parse(to + withoutPattern, module))
+      expr.updateValue(() => Ast.parse(to + withoutPattern, expr.module))
     }
   } else {
     for (const child of expr.children()) {
       if (child instanceof Token) {
         continue
       }
-      substituteQualifiedName(module, child, pattern, to)
+      const mutableChild = expr.module.getVersion(child)
+      substituteQualifiedName(mutableChild, pattern, to)
     }
   }
 }