Skip to content

Commit

Permalink
Copy/paste improvements (#9734)
Browse files Browse the repository at this point in the history
Copying nodes:
- Multiple nodes supported.
- Node comments and user-specified colors included.
- Google Sheets data can be pasted to produce a `Table` node, handled the same way as Excel data.

# Important Notes
- Fix E2E tests on OS X.
- Add E2E and unit tests for clipboard.
- Use the lexer to test text escaping; fix text escaping issues and inconsistencies.
  • Loading branch information
kazcw authored Apr 19, 2024
1 parent 7c571bd commit 6426478
Show file tree
Hide file tree
Showing 14 changed files with 394 additions and 153 deletions.
4 changes: 2 additions & 2 deletions app/gui2/e2e/collapsingAndEntering.spec.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { test, type Page } from '@playwright/test'
import os from 'os'
import * as actions from './actions'
import { expect } from './customExpect'
import { mockCollapsedFunctionInfo } from './expressionUpdates'
import { CONTROL_KEY } from './keyboard'
import * as locate from './locate'

const MAIN_FILE_NODES = 11

const COLLAPSE_SHORTCUT = os.platform() === 'darwin' ? 'Meta+G' : 'Control+G'
const COLLAPSE_SHORTCUT = `${CONTROL_KEY}+G`

test('Entering nodes', async ({ page }) => {
await actions.goToGraph(page)
Expand Down
3 changes: 1 addition & 2 deletions app/gui2/e2e/componentBrowser.spec.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { test, type Page } from '@playwright/test'
import os from 'os'
import * as actions from './actions'
import { expect } from './customExpect'
import { CONTROL_KEY } from './keyboard'
import * as locate from './locate'

const CONTROL_KEY = os.platform() === 'darwin' ? 'Meta' : 'Control'
const ACCEPT_SUGGESTION_SHORTCUT = `${CONTROL_KEY}+Enter`

async function deselectAllNodes(page: Page) {
Expand Down
4 changes: 4 additions & 0 deletions app/gui2/e2e/keyboard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import os from 'os'

export const CONTROL_KEY = os.platform() === 'darwin' ? 'Meta' : 'Control'
export const DELETE_KEY = os.platform() === 'darwin' ? 'Backspace' : 'Delete'
70 changes: 70 additions & 0 deletions app/gui2/e2e/nodeClipboard.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import test from 'playwright/test'
import * as actions from './actions'
import { expect } from './customExpect'
import { CONTROL_KEY } from './keyboard'
import * as locate from './locate'

test.beforeEach(async ({ page }) => {
await page.addInitScript(() => {
class MockClipboard {
private contents: ClipboardItem[] = []
async read(): Promise<ClipboardItem[]> {
return [...this.contents]
}
async write(contents: ClipboardItem[]) {
this.contents = [...contents]
}
}
Object.assign(window.navigator, {
mockClipboard: new MockClipboard(),
})
})
})

test('Copy node with comment', async ({ page }) => {
await actions.goToGraph(page)

// Check state before operation.
const originalNodes = await locate.graphNode(page).count()
await expect(page.locator('.GraphNodeComment')).toExist()
const originalNodeComments = await page.locator('.GraphNodeComment').count()

// Select a node.
const nodeToCopy = locate.graphNodeByBinding(page, 'final')
await nodeToCopy.click()
await expect(nodeToCopy).toBeSelected()
// Copy and paste it.
await page.keyboard.press(`${CONTROL_KEY}+C`)
await page.keyboard.press(`${CONTROL_KEY}+V`)
await expect(nodeToCopy).toBeSelected()

// Node and comment have been copied.
await expect(locate.graphNode(page)).toHaveCount(originalNodes + 1)
await expect(page.locator('.GraphNodeComment')).toHaveCount(originalNodeComments + 1)
})

test('Copy multiple nodes', async ({ page }) => {
await actions.goToGraph(page)

// Check state before operation.
const originalNodes = await locate.graphNode(page).count()
await expect(page.locator('.GraphNodeComment')).toExist()
const originalNodeComments = await page.locator('.GraphNodeComment').count()

// Select some nodes.
const node1 = locate.graphNodeByBinding(page, 'final')
await node1.click()
const node2 = locate.graphNodeByBinding(page, 'data')
await node2.click({ modifiers: ['Shift'] })
await expect(node1).toBeSelected()
await expect(node2).toBeSelected()
// Copy and paste.
await page.keyboard.press(`${CONTROL_KEY}+C`)
await page.keyboard.press(`${CONTROL_KEY}+V`)
await expect(node1).toBeSelected()
await expect(node2).toBeSelected()

// Nodes and comment have been copied.
await expect(locate.graphNode(page)).toHaveCount(originalNodes + 2)
await expect(page.locator('.GraphNodeComment')).toHaveCount(originalNodeComments + 1)
})
13 changes: 7 additions & 6 deletions app/gui2/e2e/undoRedo.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import test from 'playwright/test'
import * as actions from './actions'
import { expect } from './customExpect'
import { CONTROL_KEY, DELETE_KEY } from './keyboard'
import * as locate from './locate'

test('Adding new node', async ({ page }) => {
Expand All @@ -10,18 +11,18 @@ test('Adding new node', async ({ page }) => {
await locate.addNewNodeButton(page).click()
await expect(locate.componentBrowserInput(page)).toBeVisible()
await page.keyboard.insertText('foo')
await page.keyboard.press('Control+Enter')
await page.keyboard.press(`${CONTROL_KEY}+Enter`)
await expect(locate.graphNode(page)).toHaveCount(nodesCount + 1)
await expect(locate.graphNode(page).last().locator('.WidgetToken')).toHaveText(['foo'])
const newNodeBBox = await locate.graphNode(page).last().boundingBox()

await page.keyboard.press('Control+Z')
await page.keyboard.press(`${CONTROL_KEY}+Z`)
await expect(locate.graphNode(page)).toHaveCount(nodesCount)
await expect(
locate.graphNode(page).locator('.WidgetToken').filter({ hasText: 'foo' }),
).toHaveCount(0)

await page.keyboard.press('Control+Shift+Z')
await page.keyboard.press(`${CONTROL_KEY}+Shift+Z`)
await expect(locate.graphNode(page)).toHaveCount(nodesCount + 1)
await expect(locate.graphNode(page).last().locator('.WidgetToken')).toHaveText(['foo'])
const restoredBox = await locate.graphNode(page).last().boundingBox()
Expand All @@ -35,17 +36,17 @@ test('Removing node', async ({ page }) => {
const deletedNode = locate.graphNodeByBinding(page, 'final')
const deletedNodeBBox = await deletedNode.boundingBox()
await deletedNode.click()
await page.keyboard.press('Delete')
await page.keyboard.press(DELETE_KEY)
await expect(locate.graphNode(page)).toHaveCount(nodesCount - 1)

await page.keyboard.press('Control+Z')
await page.keyboard.press(`${CONTROL_KEY}+Z`)
await expect(locate.graphNode(page)).toHaveCount(nodesCount)
await expect(deletedNode.locator('.WidgetToken')).toHaveText(['Main', '.', 'func1', 'prod'])
await expect(deletedNode.locator('.GraphNodeComment')).toHaveText('This node can be entered')
const restoredBBox = await deletedNode.boundingBox()
await expect(restoredBBox).toEqual(deletedNodeBBox)

await page.keyboard.press('Control+Shift+Z')
await page.keyboard.press(`${CONTROL_KEY}+Shift+Z`)
await expect(locate.graphNode(page)).toHaveCount(nodesCount - 1)
await expect(deletedNode).not.toBeVisible()
})
51 changes: 20 additions & 31 deletions app/gui2/shared/ast/text.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
* `lib/rust/parser/src/lexer.rs`, search for `fn text_escape`.
*/

import { assertUnreachable } from '../util/assert'
import { assertDefined } from '../util/assert'
import { TextLiteral } from './tree'

const escapeSequences = [
['0', '\0'],
Expand All @@ -17,7 +18,6 @@ const escapeSequences = [
['v', '\x0B'],
['e', '\x1B'],
['\\', '\\'],
['"', '"'],
["'", "'"],
['`', '`'],
] as const
Expand All @@ -28,52 +28,41 @@ function escapeAsCharCodes(str: string): string {
return out
}

const fixedEscapes = escapeSequences.map(([_, raw]) => escapeAsCharCodes(raw))
const escapeRegex = new RegExp(
`${escapeSequences.map(([_, raw]) => escapeAsCharCodes(raw)).join('|')}`,
'gu',
)

const unescapeRegex = new RegExp(
'\\\\(?:' +
`${escapeSequences.map(([escape]) => escapeAsCharCodes(escape)).join('|')}` +
'|x[0-9a-fA-F]{0,2}' +
'|u\\{[0-9a-fA-F]{0,4}\\}?' + // Lexer allows trailing } to be missing.
'|u[0-9a-fA-F]{0,4}' +
'|U[0-9a-fA-F]{0,8}' +
')',
[
...fixedEscapes,
// Unpaired-surrogate codepoints are not technically valid in Unicode, but they are allowed in Javascript strings.
// Enso source files must be strictly UTF-8 conformant.
'\\p{Surrogate}',
].join('|'),
'gu',
)

const escapeMapping = Object.fromEntries(
escapeSequences.map(([escape, raw]) => [raw, `\\${escape}`]),
)
const unescapeMapping = Object.fromEntries(
escapeSequences.map(([escape, raw]) => [`\\${escape}`, raw]),
)

function escapeChar(char: string) {
const fixedEscape = escapeMapping[char]
if (fixedEscape != null) return fixedEscape
return escapeAsCharCodes(char)
}

/**
* Escape a string so it can be safely spliced into an interpolated (`''`) Enso string.
* Note: Escape sequences are NOT interpreted in raw (`""`) string literals.
* */
export function escapeTextLiteral(rawString: string) {
return rawString.replace(escapeRegex, (match) => escapeMapping[match] ?? assertUnreachable())
return rawString.replace(escapeRegex, escapeChar)
}

/**
* Interpret all escaped characters from an interpolated (`''`) Enso string.
* Interpret all escaped characters from an interpolated (`''`) Enso string, provided without open/close delimiters.
* Note: Escape sequences are NOT interpreted in raw (`""`) string literals.
*/
export function unescapeTextLiteral(escapedString: string) {
return escapedString.replace(unescapeRegex, (match) => {
let cut = 2
switch (match[1]) {
case 'u':
if (match[2] === '{') cut = 3 // fallthrough
case 'U':
case 'x':
return String.fromCharCode(parseInt(match.substring(cut), 16))
default:
return unescapeMapping[match] ?? assertUnreachable()
}
})
const ast = TextLiteral.tryParse("'" + escapedString + "'")
assertDefined(ast)
return ast.rawTextContent
}
27 changes: 21 additions & 6 deletions app/gui2/shared/ast/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { assert, assertDefined, assertEqual, bail } from '../util/assert'
import type { Result } from '../util/data/result'
import { Err, Ok } from '../util/data/result'
import type { SourceRangeEdit } from '../util/data/text'
import { allKeys } from '../util/types'
import type { ExternalId, VisualizationMetadata } from '../yjsModel'
import { visMetadataEquals } from '../yjsModel'
import * as RawAst from './generated/ast'
Expand All @@ -54,6 +55,11 @@ export interface NodeMetadataFields {
visualization?: VisualizationMetadata | undefined
colorOverride?: string | undefined
}
const nodeMetadataKeys = allKeys<NodeMetadataFields>({
position: null,
visualization: null,
colorOverride: null,
})
export type NodeMetadata = FixedMapView<NodeMetadataFields>
export type MutableNodeMetadata = FixedMap<NodeMetadataFields>
export function asNodeMetadata(map: Map<string, unknown>): NodeMetadata {
Expand All @@ -67,9 +73,6 @@ interface RawAstFields {
metadata: FixedMap<MetadataFields>
}
export interface AstFields extends RawAstFields, LegalFieldContent {}
function allKeys<T>(keys: Record<keyof T, any>): (keyof T)[] {
return Object.keys(keys) as any
}
const astFieldKeys = allKeys<RawAstFields>({
id: null,
type: null,
Expand All @@ -96,6 +99,11 @@ export abstract class Ast {
return metadata as FixedMapView<NodeMetadataFields>
}

/** Returns a JSON-compatible object containing all metadata properties. */
serializeMetadata(): MetadataFields & NodeMetadataFields {
return this.fields.get('metadata').toJSON() as any
}

typeName(): string {
return this.fields.get('type')
}
Expand Down Expand Up @@ -200,8 +208,14 @@ export abstract class MutableAst extends Ast {

setNodeMetadata(nodeMeta: NodeMetadataFields) {
const metadata = this.fields.get('metadata') as unknown as Map<string, unknown>
for (const [key, value] of Object.entries(nodeMeta))
if (value !== undefined) metadata.set(key, value)
for (const [key, value] of Object.entries(nodeMeta)) {
if (!nodeMetadataKeys.has(key)) continue
if (value === undefined) {
metadata.delete(key)
} else {
metadata.set(key, value)
}
}
}

/** Modify the parent of this node to refer to a new object instead. Return the object, which now has no parent. */
Expand Down Expand Up @@ -372,7 +386,7 @@ interface FieldObject<T extends TreeRefs> {
function* fieldDataEntries<Fields>(map: FixedMapView<Fields>) {
for (const entry of map.entries()) {
// All fields that are not from `AstFields` are `FieldData`.
if (!astFieldKeys.includes(entry[0] as any)) yield entry as [string, DeepReadonly<FieldData>]
if (!astFieldKeys.has(entry[0])) yield entry as [string, DeepReadonly<FieldData>]
}
}

Expand Down Expand Up @@ -2413,6 +2427,7 @@ export interface FixedMapView<Fields> {
entries(): IterableIterator<readonly [string, unknown]>
clone(): FixedMap<Fields>
has(key: string): boolean
toJSON(): object
}

export interface FixedMap<Fields> extends FixedMapView<Fields> {
Expand Down
5 changes: 5 additions & 0 deletions app/gui2/shared/util/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/** Returns an all the keys of a type. The argument provided is required to be an object containing all the keys of the
* type (including optional fields), but the associated values are ignored and may be of any type. */
export function allKeys<T>(keys: { [P in keyof T]-?: any }): ReadonlySet<string> {
return Object.freeze(new Set(Object.keys(keys)))
}
10 changes: 5 additions & 5 deletions app/gui2/src/components/GraphEditor.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ import { codeEditorBindings, graphBindings, interactionBindings } from '@/bindin
import CodeEditor from '@/components/CodeEditor.vue'
import ColorPicker from '@/components/ColorPicker.vue'
import ComponentBrowser from '@/components/ComponentBrowser.vue'
import { type Usage } from '@/components/ComponentBrowser/input'
import {
DEFAULT_NODE_SIZE,
mouseDictatedPlacement,
usePlacement,
} from '@/components/ComponentBrowser/placement'
import GraphEdges from '@/components/GraphEditor/GraphEdges.vue'
import GraphNodes from '@/components/GraphEditor/GraphNodes.vue'
import { useGraphEditorClipboard } from '@/components/GraphEditor/clipboard'
import { performCollapse, prepareCollapsedInfo } from '@/components/GraphEditor/collapsing'
import type { NodeCreationOptions } from '@/components/GraphEditor/nodeCreation'
import { useGraphEditorToasts } from '@/components/GraphEditor/toasts'
Expand Down Expand Up @@ -42,8 +44,6 @@ import { Vec2 } from '@/util/data/vec2'
import { encoding, set } from 'lib0'
import { encodeMethodPointer } from 'shared/languageServerTypes'
import { computed, onMounted, ref, shallowRef, toRef, watch } from 'vue'
import { type Usage } from './ComponentBrowser/input'
import { useGraphEditorClipboard } from './GraphEditor/clipboard'
const keyboard = provideKeyboard()
const graphStore = useGraphStore()
Expand Down Expand Up @@ -111,7 +111,7 @@ watch(
// === Clipboard Copy/Paste ===
const { copyNodeContent, readNodeFromClipboard } = useGraphEditorClipboard(
const { copySelectionToClipboard, createNodesFromClipboard } = useGraphEditorClipboard(
nodeSelection,
graphNavigator,
)
Expand Down Expand Up @@ -198,11 +198,11 @@ const graphBindingsHandler = graphBindings.handler({
},
copyNode() {
if (keyboardBusy()) return false
copyNodeContent()
copySelectionToClipboard()
},
pasteNode() {
if (keyboardBusy()) return false
readNodeFromClipboard()
createNodesFromClipboard()
},
collapse() {
if (keyboardBusy()) return false
Expand Down
Loading

0 comments on commit 6426478

Please sign in to comment.