Skip to content

Commit

Permalink
Syntactic synchronization, automatic parentheses, metadata in Ast (#8893
Browse files Browse the repository at this point in the history
)

- Synchronize Y.Js clients by AST (implements #8237).
- Before committing an edit, insert any parentheses-nodes needed for the concrete syntax to reflect tree structure (fixes #8884).
- Move `externalId` and all node metadata into a Y.Map owned by each `Ast`. This allows including metadata changes in an edit, enables Y.Js merging of changes to different metadata fields, and will enable the use of Y.Js objects in metadata. (Implements #8804.)

### Important Notes

- Metadata is now set and retrieved through accessors on the `Ast` objects.
- Since some metadata edits need to take effect in real time (e.g. node dragging), new lower-overhead APIs (`commitDirect`, `skipTreeRepair`) are provided for careful use in certain cases.
- The client is now bundled as ESM.
- The build script cleans up git-untracked generated files in an outdated location, which fixes lint errors related to `src/generated` that may occur when switching branches.
  • Loading branch information
kazcw authored Feb 2, 2024
1 parent 340a3ee commit 343a644
Show file tree
Hide file tree
Showing 59 changed files with 4,475 additions and 3,550 deletions.
1 change: 1 addition & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ app/ide-desktop/lib/dashboard/playwright/.cache/
app/ide-desktop/lib/dashboard/dist/
app/gui/view/documentation/assets/stylesheet.css
app/gui2/rust-ffi/pkg
app/gui2/rust-ffi/node-pkg
app/gui2/src/assets/font-*.css
Cargo.lock
build.json
Expand Down
45 changes: 25 additions & 20 deletions app/gui2/e2e/edgeRendering.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { expect, Page, test } from '@playwright/test'
import { expect, test, type Page } from '@playwright/test'
import * as actions from './actions'
import * as locate from './locate'

Expand All @@ -13,52 +13,57 @@ async function edgesToNodeWithBinding(page: Page, binding: string) {
return page.locator(`[data-target-node-id="${nodeId}"]`)
}

// For each outgoing edge we expect two elements: an element for io and an element for the rendered edge itself.
const EDGE_PARTS = 2

test('Existence of edges between nodes', async ({ page }) => {
await actions.goToGraph(page)

// For each outgoing edge we expect three elements: an element for io, an element for the arrow, and an element for the rendered edge itself.

await expect(await edgesFromNodeWithBinding(page, 'aggregated')).toHaveCount(0)
await expect(await edgesFromNodeWithBinding(page, 'filtered')).toHaveCount(0)
await expect(await edgesFromNodeWithBinding(page, 'data')).toHaveCount(6)
await expect(await edgesFromNodeWithBinding(page, 'data')).toHaveCount(2 * EDGE_PARTS)
await expect(await edgesFromNodeWithBinding(page, 'list')).toHaveCount(0)
await expect(await edgesFromNodeWithBinding(page, 'final')).toHaveCount(0)
await expect(await edgesFromNodeWithBinding(page, 'prod')).toHaveCount(3)
await expect(await edgesFromNodeWithBinding(page, 'sum')).toHaveCount(3)
await expect(await edgesFromNodeWithBinding(page, 'ten')).toHaveCount(3)
await expect(await edgesFromNodeWithBinding(page, 'five')).toHaveCount(3)
await expect(await edgesFromNodeWithBinding(page, 'prod')).toHaveCount(EDGE_PARTS)
await expect(await edgesFromNodeWithBinding(page, 'sum')).toHaveCount(EDGE_PARTS)
await expect(await edgesFromNodeWithBinding(page, 'ten')).toHaveCount(EDGE_PARTS)
await expect(await edgesFromNodeWithBinding(page, 'five')).toHaveCount(EDGE_PARTS)

await expect(await edgesToNodeWithBinding(page, 'aggregated')).toHaveCount(3)
await expect(await edgesToNodeWithBinding(page, 'filtered')).toHaveCount(3)
await expect(await edgesToNodeWithBinding(page, 'aggregated')).toHaveCount(EDGE_PARTS)
await expect(await edgesToNodeWithBinding(page, 'filtered')).toHaveCount(EDGE_PARTS)
await expect(await edgesToNodeWithBinding(page, 'data')).toHaveCount(0)
await expect(await edgesToNodeWithBinding(page, 'list')).toHaveCount(0)
await expect(await edgesToNodeWithBinding(page, 'final')).toHaveCount(3)
await expect(await edgesToNodeWithBinding(page, 'prod')).toHaveCount(3)
await expect(await edgesToNodeWithBinding(page, 'sum')).toHaveCount(6)
await expect(await edgesToNodeWithBinding(page, 'final')).toHaveCount(EDGE_PARTS)
await expect(await edgesToNodeWithBinding(page, 'prod')).toHaveCount(EDGE_PARTS)
await expect(await edgesToNodeWithBinding(page, 'sum')).toHaveCount(2 * EDGE_PARTS)
await expect(await edgesToNodeWithBinding(page, 'ten')).toHaveCount(0)
await expect(await edgesToNodeWithBinding(page, 'five')).toHaveCount(0)
})

test('Hover behaviour of edges', async ({ page }) => {
await actions.goToGraph(page)

// One for interaction, one for rendering, one for the arrow element.
await expect(await edgesFromNodeWithBinding(page, 'ten')).toHaveCount(3)
const edgeElements = await edgesFromNodeWithBinding(page, 'ten')
await expect(edgeElements).toHaveCount(EDGE_PARTS)

const targetEdge = page.locator('path:nth-child(4)')
const targetEdge = edgeElements.first()
await expect(targetEdge).toHaveClass('edge io')
// It is not currently possible to interact with edges in the default node layout.
// See: https://github.com/enso-org/enso/issues/8938
/*
// Hover over edge to the right of node with binding `ten`.
await targetEdge.hover({
position: { x: 65, y: 135.0 },
force: true,
position: { x: 60, y: 45 }, // source node
})
// Expect an extra edge for the split rendering.
const edgeElements = await edgesFromNodeWithBinding(page, 'ten')
await expect(edgeElements).toHaveCount(4)
const hoveredEdgeElements = await edgesFromNodeWithBinding(page, 'ten')
await expect(hoveredEdgeElements).toHaveCount(2 * EDGE_PARTS)
// Expect the top edge part to be dimmed
const topEdge = page.locator('path:nth-child(4)')
await expect(topEdge).toHaveClass('edge visible dimmed')
// Expect the bottom edge part not to be dimmed
const bottomEdge = page.locator('path:nth-child(6)')
await expect(bottomEdge).toHaveClass('edge visible')
*/
})
2 changes: 1 addition & 1 deletion app/gui2/eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const DIR_NAME = path.dirname(url.fileURLToPath(import.meta.url))

const conf = [
{
ignores: ['rust-ffi/pkg', 'dist', 'src/generated', 'templates'],
ignores: ['rust-ffi/pkg', 'rust-ffi/node-pkg', 'dist', 'src/generated', 'templates'],
},
...compat.extends('plugin:vue/vue3-recommended'),
eslintJs.configs.recommended,
Expand Down
7 changes: 6 additions & 1 deletion app/gui2/mock/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { useGraphStore } from '@/stores/graph'
import { GraphDb, mockNode } from '@/stores/graph/graphDatabase'
import { useProjectStore } from '@/stores/project'
import { ComputedValueRegistry } from '@/stores/project/computedValueRegistry'
import { Ast } from '@/util/ast'
import { MockTransport, MockWebSocket } from '@/util/net'
import { getActivePinia } from 'pinia'
import { ref, type App } from 'vue'
Expand Down Expand Up @@ -65,7 +66,11 @@ export function projectStore() {
const projectStore = useProjectStore(getActivePinia())
const mod = projectStore.projectModel.createNewModule('Main.enso')
mod.doc.ydoc.emit('load', [])
mod.doc.setCode('main =\n')
const syncModule = new Ast.MutableModule(mod.doc.ydoc)
mod.transact(() => {
const root = Ast.parseBlock('main =\n', syncModule)
syncModule.replaceRoot(root)
})
return projectStore
}

Expand Down
10 changes: 6 additions & 4 deletions app/gui2/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@
"typecheck": "vue-tsc --noEmit -p tsconfig.app.json --composite false",
"lint": "eslint .",
"format": "prettier --version && prettier --write src/ && eslint . --fix",
"build-rust-ffi": "wasm-pack build ./rust-ffi --release --target web",
"generate-ast-schema": "cargo run -p enso-parser-schema > src/generated/ast-schema.json",
"generate-ast-types": "tsx ./parser-codegen/index.ts src/generated/ast-schema.json src/generated/ast.ts",
"preinstall": "npm run build-rust-ffi && npm run generate-ast-schema && npm run generate-ast-types && npm run generate-metadata && npm run download-fonts",
"clean-old-generated-directory": "rimraf src/generated",
"build-rust-ffi": "wasm-pack build ./rust-ffi --release --target web && wasm-pack build ./rust-ffi --out-dir node-pkg --target nodejs",
"generate-ast-schema": "cargo run -p enso-parser-schema > shared/ast/generated/ast-schema.json",
"generate-ast-types": "tsx ./parser-codegen/index.ts shared/ast/generated/ast-schema.json shared/ast/generated/ast.ts",
"preinstall": "npm run clean-old-generated-directory && npm run build-rust-ffi && npm run generate-ast-schema && npm run generate-ast-types && npm run generate-metadata && npm run download-fonts",
"postinstall": "playwright install",
"generate-metadata": "node scripts/generateIconMetadata.js",
"download-fonts": "node scripts/downloadFonts.js"
Expand Down Expand Up @@ -64,6 +65,7 @@
"pinia": "^2.1.6",
"postcss-inline-svg": "^6.0.0",
"postcss-nesting": "^12.0.1",
"rimraf": "^5.0.5",
"semver": "^7.5.4",
"sucrase": "^3.34.0",
"vue": "^3.3.4",
Expand Down
2 changes: 1 addition & 1 deletion app/gui2/parser-codegen/codegen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export function implement(schema: Schema.Schema): string {
),
),
),
tsf.createStringLiteral('@/util/parserSupport', true),
tsf.createStringLiteral('../parserSupport', true),
undefined,
),
)
Expand Down
22 changes: 22 additions & 0 deletions app/gui2/shared/ast/ffi.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import init, {
is_ident_or_operator,
parse_doc_to_json,
parse as parse_tree,
} from '../../rust-ffi/pkg/rust_ffi'
import { isNode } from '../util/detect'

export async function initializeFFI(path?: string | undefined) {
if (isNode) {
const fs = await import('node:fs/promises')
const buffer = fs.readFile(path ?? './rust-ffi/pkg/rust_ffi_bg.wasm')
await init(buffer)
} else {
await init()
}
}

// TODO[ao]: We cannot to that, because the ffi is used by cjs modules.
// await initializeFFI()

// eslint-disable-next-line camelcase
export { is_ident_or_operator, parse_doc_to_json, parse_tree }
File renamed without changes.
70 changes: 70 additions & 0 deletions app/gui2/shared/ast/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import * as random from 'lib0/random'
import type { ExternalId } from '../yjsModel'
import type { Module } from './mutableModule'
import type { SyncTokenId } from './token'
import type { AstId } from './tree'
import { Ast, MutableAst } from './tree'

export * from './mutableModule'
export * from './parse'
export * from './token'
export * from './tree'

declare const brandOwned: unique symbol
/** Used to mark references required to be unique.
*
* Note that the typesystem cannot stop you from copying an `Owned`,
* but that is an easy mistake to see (because it occurs locally).
*
* We can at least require *obtaining* an `Owned`,
* which statically prevents the otherwise most likely usage errors when rearranging ASTs.
*/
export type Owned<T = MutableAst> = T & { [brandOwned]: never }
/** @internal */
export function asOwned<T>(t: T): Owned<T> {
return t as Owned<T>
}

export type NodeChild<T = AstId | SyncTokenId> = { whitespace?: string | undefined; node: T }

export function newExternalId(): ExternalId {
return random.uuidv4() as ExternalId
}

/** @internal */
export function parentId(ast: Ast): AstId | undefined {
return ast.fields.get('parent')
}

/** Returns the given IDs, and the IDs of all their ancestors. */
export function subtrees(module: Module, ids: Iterable<AstId>) {
const subtrees = new Set<AstId>()
for (const id of ids) {
let ast = module.get(id)
while (ast != null && !subtrees.has(ast.id)) {
subtrees.add(ast.id)
ast = ast.parent()
}
}
return subtrees
}

/** Returns the IDs of the ASTs that are not descendants of any others in the given set. */
export function subtreeRoots(module: Module, ids: Set<AstId>) {
const roots = new Array<AstId>()
for (const id of ids) {
const astInModule = module.get(id)
if (!astInModule) continue
let ast = astInModule.parent()
let hasParentInSet
while (ast != null) {
if (ids.has(ast.id)) {
hasParentInSet = true
break
}
ast = ast.parent()
}
if (!hasParentInSet) roots.push(id)
}
return roots
}
Loading

0 comments on commit 343a644

Please sign in to comment.