diff --git a/docs/TODO.md b/docs/TODO.md index 10173541..15101695 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -328,9 +328,8 @@ From SuperOOP analysis and handler chain refactor (PR #54): Why: v1 hardcodes `"After"`. Users need to control where the node lands. Exit: mouse Y within target bounding box determines Before/After/Inside; visual indicator shows landing position. -- [ ] **Semantic-aware drop** - Why: text-based splice creates parse errors (e.g. `let id =` with empty RHS after moving the lambda out). Drop should operate at AST level. - Exit: dropping a node produces a syntactically valid result or rejects with an error. +- [x] **Semantic-aware drop** (PR #179) + Done: Before/After replaces source with placeholder via `Renderable::placeholder()`, inserts source text from source map. Exchange unchanged. Playwright E2E tests cover all positions. - [x] **Unit tests for drag-drop JSON parser and legality** Why: no test coverage for `StartDrag`/`DragOver`/`Drop` parsing or self-drop/descendant-drop guards. diff --git a/editor/projection_memo.mbt b/editor/projection_memo.mbt index a401fe9b..25b0f163 100644 --- a/editor/projection_memo.mbt +++ b/editor/projection_memo.mbt @@ -2,21 +2,24 @@ ///| /// Returns the cached ProjNode (converted from FlatProj once per invalidation). +/// Safe to call both inside and outside tracked context. pub fn[T] SyncEditor::get_proj_node(self : SyncEditor[T]) -> @core.ProjNode[T]? { - self.cached_proj_node.get() + self.parser_rt.read(self.cached_proj_node) } ///| +/// Safe to call both inside and outside tracked context. pub fn[T] SyncEditor::get_source_map(self : SyncEditor[T]) -> @core.SourceMap { - self.source_map_memo.get() + self.parser_rt.read(self.source_map_memo) } ///| /// Returns the full node registry (NodeId → ProjNode). +/// Safe to call both inside and outside tracked context. pub fn[T] SyncEditor::get_registry( self : SyncEditor[T], ) -> Map[@core.NodeId, @core.ProjNode[T]] { - self.registry_memo.get() + self.parser_rt.read(self.registry_memo) } ///| @@ -30,7 +33,7 @@ pub fn[T] SyncEditor::get_node( self : SyncEditor[T], id : @core.NodeId, ) -> @core.ProjNode[T]? { - self.registry_memo.get().get(id) + self.parser_rt.read(self.registry_memo).get(id) } ///| @@ -38,7 +41,7 @@ pub fn[T] SyncEditor::node_at_position( self : SyncEditor[T], position : Int, ) -> @core.NodeId? { - self.source_map_memo.get().innermost_node_at(position) + self.parser_rt.read(self.source_map_memo).innermost_node_at(position) } ///| @@ -46,5 +49,5 @@ pub fn[T] SyncEditor::get_node_range( self : SyncEditor[T], id : @core.NodeId, ) -> @loom_core.Range? { - self.source_map_memo.get().get_range(id) + self.parser_rt.read(self.source_map_memo).get_range(id) } diff --git a/editor/sync_editor_tree_edit.mbt b/editor/sync_editor_tree_edit.mbt index 7421f853..8b327975 100644 --- a/editor/sync_editor_tree_edit.mbt +++ b/editor/sync_editor_tree_edit.mbt @@ -18,7 +18,7 @@ pub fn[T] SyncEditor::is_dirty(self : SyncEditor[T]) -> Bool { /// inside a getter. pub fn[T] SyncEditor::refresh(self : SyncEditor[T]) -> Unit { self.projection_dirty = false - let _ = self.cached_proj_node.get() + let _ = self.parser_rt.read(self.cached_proj_node) } ///| @@ -132,14 +132,18 @@ pub fn[T] SyncEditor::apply_text_transform( } ///| -/// Move a node to a new position in the tree by: -/// 1. Unparsing the source node via T::unparse -/// 2. Deleting the source span (replace with placeholder) -/// 3. Inserting the unparsed text adjacent to the target span +/// Move a node to a new position in the tree. +/// +/// Inside (exchange): swaps the expression content of source and target, +/// preserving leading whitespace/separators. +/// +/// Before/After: replaces the source expression with its placeholder +/// (keeping the syntax valid) and inserts the source text at the target. /// /// CAUTION: Insert position must be adjusted when source precedes target /// in the document (deletion shifts subsequent offsets backward by -/// `source_len - placeholder_len`). This method handles the adjustment. +/// `source_len - placeholder_len`). apply_span_edits handles this via +/// reverse-order application. pub fn[T : @loom_core.Renderable] SyncEditor::move_node( self : SyncEditor[T], source_id : @core.NodeId, @@ -190,6 +194,18 @@ pub fn[T : @loom_core.Renderable] SyncEditor::move_node( ) } let doc_text = self.get_text() + // Strip leading whitespace from a span to find the expression-only start. + // Source map ranges include leading separators; we keep those in place. + fn skip_ws(text : String, start : Int, end : Int) -> Int { + let mut i = start + while i < end { + match text[i] { + ' ' | '\t' | '\n' | '\r' => i = i + 1 + _ => break + } + } + i + } let edits : Array[@core.SpanEdit] = match position { // Inside = exchange: swap the expression content, preserving leading whitespace Inside => { @@ -202,18 +218,6 @@ pub fn[T : @loom_core.Renderable] SyncEditor::move_node( ), ) } - // Strip leading whitespace from each span to find the expression-only range. - // Source map ranges include leading separators; we keep those in place. - fn skip_ws(text : String, start : Int, end : Int) -> Int { - let mut i = start - while i < end { - match text[i] { - ' ' | '\t' | '\n' | '\r' => i = i + 1 - _ => break - } - } - i - } let src_expr_start = skip_ws(doc_text, src_range.start, src_range.end) let tgt_expr_start = skip_ws(doc_text, tgt_range.start, tgt_range.end) let src_expr = doc_text[src_expr_start:src_range.end].to_string() @@ -231,20 +235,24 @@ pub fn[T : @loom_core.Renderable] SyncEditor::move_node( }, ] } - // Before/After = move: remove source and insert at target position + // Before/After = move: replace source with placeholder, insert at target Before | After => { - let src_text = @loom_core.Renderable::unparse(src_node.kind) + // Extract expression text from source map (whitespace-aware, like Exchange) + let src_expr_start = skip_ws(doc_text, src_range.start, src_range.end) + let src_expr = doc_text[src_expr_start:src_range.end].to_string() + // Replace source expression with placeholder (keeps leading whitespace) + let placeholder = @loom_core.Renderable::placeholder(src_node.kind) let tgt_insert = match position { Before => tgt_range.start _ => tgt_range.end } [ { - start: src_range.start, - delete_len: src_range.end - src_range.start, - inserted: "", + start: src_expr_start, + delete_len: src_range.end - src_expr_start, + inserted: placeholder, }, - { start: tgt_insert, delete_len: 0, inserted: src_text + " " }, + { start: tgt_insert, delete_len: 0, inserted: src_expr + " " }, ] } } diff --git a/examples/ideal/web/e2e/drag-drop.spec.ts b/examples/ideal/web/e2e/drag-drop.spec.ts new file mode 100644 index 00000000..78b11c69 --- /dev/null +++ b/examples/ideal/web/e2e/drag-drop.spec.ts @@ -0,0 +1,197 @@ +// Drag-and-drop E2E tests for Before/After/Inside (exchange) moves. +// Validates that AST-level operations produce valid syntax (no empty RHS, +// no orphaned separators) by checking the resulting editor text. + +import { test, expect, type Page } from '@playwright/test'; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +/** Enter Structure mode and wait for structure blocks to render. */ +async function setupStructureMode(page: Page) { + await page.goto('/'); + await page.getByRole('button', { name: 'Structure' }).click(); + await expect(page.getByLabel('Code editor')).toBeVisible(); + await page.waitForFunction(() => { + const ce = document.querySelector('canopy-editor'); + return ce?.shadowRoot?.querySelector('.structure-block') !== null; + }, { timeout: 10_000 }); +} + +/** Read the current editor text via the CRDT global. */ +async function getEditorText(page: Page): Promise { + return await page.evaluate(() => { + const g = globalThis as any; + if (g.__canopy_crdt && g.__canopy_crdt_handle != null) { + return g.__canopy_crdt.get_text(g.__canopy_crdt_handle) as string; + } + return ''; + }); +} + +/** Count structure blocks of a given type inside the shadow DOM. */ +async function countNodes(page: Page, type: string): Promise { + return await page.evaluate( + (type) => { + const ce = document.querySelector('canopy-editor'); + if (!ce?.shadowRoot) return 0; + return ce.shadowRoot.querySelectorAll(`.structure-${type}`).length; + }, + type, + ); +} + +/** + * Perform a drag-drop between two structure nodes using dispatched events. + * HTML5 drag-and-drop requires DataTransfer which page.mouse can't provide, + * so we synthesize the full dragstart/dragover/drop/dragend sequence. + */ +async function dragDrop( + page: Page, + srcSelector: string, + srcNth: number, + tgtSelector: string, + tgtNth: number, + position: 'Before' | 'After' | 'Inside', +) { + await page.evaluate( + ({ srcSelector, srcNth, tgtSelector, tgtNth, position }) => { + const ce = document.querySelector('canopy-editor'); + if (!ce?.shadowRoot) throw new Error('canopy-editor not found'); + + const srcBlocks = ce.shadowRoot.querySelectorAll(srcSelector); + const tgtBlocks = ce.shadowRoot.querySelectorAll(tgtSelector); + const src = srcBlocks[srcNth] as HTMLElement; + const tgt = tgtBlocks[tgtNth] as HTMLElement; + if (!src || !tgt) throw new Error(`Node not found: src=${srcSelector}[${srcNth}], tgt=${tgtSelector}[${tgtNth}]`); + + // Enable draggable (normally done by grip mousedown) + src.draggable = true; + + const tgtRect = tgt.getBoundingClientRect(); + let clientY: number; + switch (position) { + case 'Before': clientY = tgtRect.top + tgtRect.height * 0.1; break; + case 'After': clientY = tgtRect.top + tgtRect.height * 0.9; break; + case 'Inside': clientY = tgtRect.top + tgtRect.height * 0.5; break; + } + const clientX = tgtRect.left + tgtRect.width / 2; + + // Get source nodeId + const srcNodeId = (src as any).__pmNode?.attrs?.nodeId + ?? src.getAttribute('data-node-id') + ?? '0'; + + // Create DataTransfer with source node ID + const dt = new DataTransfer(); + dt.setData('application/x-canopy-node', String(srcNodeId)); + dt.effectAllowed = 'move'; + + // Dispatch drag sequence + src.dispatchEvent(new DragEvent('dragstart', { + bubbles: true, composed: true, dataTransfer: dt, + })); + + tgt.dispatchEvent(new DragEvent('dragover', { + bubbles: true, composed: true, dataTransfer: dt, + clientX, clientY, cancelable: true, + })); + + tgt.dispatchEvent(new DragEvent('drop', { + bubbles: true, composed: true, dataTransfer: dt, + clientX, clientY, cancelable: true, + })); + + src.dispatchEvent(new DragEvent('dragend', { + bubbles: true, composed: true, dataTransfer: dt, + })); + }, + { srcSelector, srcNth, tgtSelector, tgtNth, position }, + ); + + // Wait for reparse after edit + await page.waitForTimeout(500); +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +test.describe('Drag-Drop — Before/After/Inside', () => { + + test.beforeEach(async ({ page }) => { + await setupStructureMode(page); + // Load the "Basics" example: "let id = \x. { x }\nlet apply = ..." + await page.getByRole('button', { name: 'Basics' }).click(); + await page.waitForTimeout(500); + }); + + test('structure blocks render for default example', async ({ page }) => { + const letDefs = await countNodes(page, 'let_def'); + expect(letDefs).toBeGreaterThanOrEqual(2); + }); + + test('grip element is visible on let_def blocks', async ({ page }) => { + const gripVisible = await page.evaluate(() => { + const ce = document.querySelector('canopy-editor'); + if (!ce?.shadowRoot) return false; + const grip = ce.shadowRoot.querySelector('.structure-let_def .structure-grip'); + if (!grip) return false; + const rect = grip.getBoundingClientRect(); + return rect.width > 0 && rect.height > 0; + }); + expect(gripVisible).toBe(true); + }); + + test('CRDT text is accessible', async ({ page }) => { + const text = await getEditorText(page); + expect(text).toContain('let'); + expect(text.length).toBeGreaterThan(10); + }); + + test('exchange (Inside) swaps two let-definitions', async ({ page }) => { + const textBefore = await getEditorText(page); + + await dragDrop(page, '.structure-let_def', 0, '.structure-let_def', 1, 'Inside'); + + const textAfter = await getEditorText(page); + expect(textAfter).not.toEqual(textBefore); + const letCount = (textAfter.match(/let /g) || []).length; + expect(letCount).toBeGreaterThanOrEqual(2); + }); + + test('Before drop produces valid syntax with placeholder', async ({ page }) => { + const textBefore = await getEditorText(page); + + await dragDrop(page, '.structure-let_def', 1, '.structure-let_def', 0, 'Before'); + + const textAfter = await getEditorText(page); + expect(textAfter).not.toEqual(textBefore); + // Critical: no "let x = " with empty RHS — placeholder should fill it + expect(textAfter).not.toMatch(/let \w+ = \s*\n/); + const letCount = (textAfter.match(/let /g) || []).length; + expect(letCount).toBeGreaterThanOrEqual(2); + }); + + test('After drop produces valid syntax with placeholder', async ({ page }) => { + const textBefore = await getEditorText(page); + + await dragDrop(page, '.structure-let_def', 0, '.structure-let_def', 1, 'After'); + + const textAfter = await getEditorText(page); + expect(textAfter).not.toEqual(textBefore); + expect(textAfter).not.toMatch(/let \w+ = \s*\n/); + const letCount = (textAfter.match(/let /g) || []).length; + expect(letCount).toBeGreaterThanOrEqual(2); + }); + + test('self-drop is rejected (no change)', async ({ page }) => { + const textBefore = await getEditorText(page); + + await dragDrop(page, '.structure-let_def', 0, '.structure-let_def', 0, 'Inside'); + + const textAfter = await getEditorText(page); + expect(textAfter).toEqual(textBefore); + }); +}); diff --git a/lang/lambda/companion/lambda_editor.mbt b/lang/lambda/companion/lambda_editor.mbt index 978124b9..ed587150 100644 --- a/lang/lambda/companion/lambda_editor.mbt +++ b/lang/lambda/companion/lambda_editor.mbt @@ -6,6 +6,7 @@ /// Holds lambda-specific memos alongside a generic SyncEditor. /// The FFI layer stores this alongside the editor. pub struct LambdaCompanion { + priv rt : @incr.Runtime priv proj_memo : @incr.Memo[@lambda_flat.VersionedFlatProj] // Escalation memo merges Tier 1 (direct) + Tier 2 (egglog) results. // Falls back to Tier 1 results when no definitions are Suppressed. @@ -16,7 +17,7 @@ pub struct LambdaCompanion { pub fn LambdaCompanion::get_flat_proj( self : LambdaCompanion, ) -> @lambda_proj.FlatProj? { - self.proj_memo.get().proj + self.rt.read(self.proj_memo).proj } ///| @@ -24,7 +25,7 @@ pub fn LambdaCompanion::get_eval_results( self : LambdaCompanion, ) -> Array[@lambda_eval.EvalResult] { // Return escalated results (Tier 1 + Tier 2 merge) - self.escalation_memo.get() + self.rt.read(self.escalation_memo) } ///| @@ -90,8 +91,9 @@ pub fn new_lambda_editor( agent_id : String, capture_timeout_ms? : Int = 500, ) -> (@editor.SyncEditor[@ast.Term], LambdaCompanion) { - // Ref side-channels to capture memos from build_memos callback. + // Ref side-channels to capture memos and runtime from build_memos callback. // Safe: new_generic calls build_memos during construction, before returning. + let rt_ref : Ref[@incr.Runtime?] = Ref::new(None) let proj_memo_ref : Ref[@incr.Memo[@lambda_flat.VersionedFlatProj]?] = Ref::new( None, ) @@ -108,6 +110,7 @@ pub fn new_lambda_editor( agent_id, fn(s) { @loom.new_imperative_parser(s, @parser.lambda_grammar) }, fn(rt, source_text, syntax_tree, parser) { + rt_ref.val = Some(rt) let (proj_memo, cached_proj_node, registry_memo, source_map_memo) = @lambda_flat.build_lambda_projection_memos( rt, source_text, syntax_tree, parser, ) @@ -124,10 +127,11 @@ pub fn new_lambda_editor( }, capture_timeout_ms~, capabilities=build_lambda_capabilities( - cached_proj_node_ref, escalation_memo_ref, + rt_ref, cached_proj_node_ref, escalation_memo_ref, ), ) let companion : LambdaCompanion = { + rt: rt_ref.val.unwrap(), proj_memo: proj_memo_ref.val.unwrap(), escalation_memo: escalation_memo_ref.val.unwrap(), } @@ -137,24 +141,33 @@ pub fn new_lambda_editor( ///| /// Build capabilities using the escalation memo (Tier 1 + Tier 2 merged results). fn build_lambda_capabilities( + rt_ref : Ref[@incr.Runtime?], cached_proj_node_ref : Ref[@incr.Memo[@core.ProjNode[@ast.Term]?]?], escalation_memo_ref : Ref[@incr.Memo[Array[@lambda_eval.EvalResult]]?], ) -> @editor.LanguageCapabilities[@ast.Term] { @editor.LanguageCapabilities::new( get_annotations=Some(fn() { + let rt = match rt_ref.val { + Some(rt) => rt + None => return {} + } let eval_results = match escalation_memo_ref.val { - Some(memo) => memo.get() + Some(memo) => rt.read(memo) None => return {} } let proj_node = match cached_proj_node_ref.val { - Some(memo) => memo.get() + Some(memo) => rt.read(memo) None => None } build_eval_annotations(eval_results, proj_node) }), pretty_post_process=Some(fn(layout) { + let rt = match rt_ref.val { + Some(rt) => rt + None => return layout + } let eval_results = match escalation_memo_ref.val { - Some(memo) => memo.get() + Some(memo) => rt.read(memo) None => return layout } @lambda_eval.inject_eval_annotations(layout, eval_results) diff --git a/loom b/loom index fae30bae..157e3fa8 160000 --- a/loom +++ b/loom @@ -1 +1 @@ -Subproject commit fae30bae7e7b373be0892174b99a78cb68ec293c +Subproject commit 157e3fa8d212355e4afd1178daf7b332eba9128b