diff --git a/src/editor/codemirror/copypaste.ts b/src/editor/codemirror/copypaste.ts index 6df87bb48..d1cf91f74 100644 --- a/src/editor/codemirror/copypaste.ts +++ b/src/editor/codemirror/copypaste.ts @@ -51,9 +51,9 @@ const copyPasteHandlers = () => [ message: pasteContext.id, }); - const lineNumber = view.state.doc.lineAt( - view.state.selection.ranges[0].from - ).number; + const line = view.state.doc.lineAt(view.state.selection.ranges[0].from); + const lineNumber = line.number; + const column = view.state.selection.ranges[0].from - line.from; view.dispatch( calculateChanges( @@ -61,6 +61,7 @@ const copyPasteHandlers = () => [ pasteContext.codeWithImports, pasteContext.type, lineNumber, + Math.floor(column / 4), true ) ); diff --git a/src/editor/codemirror/dnd.ts b/src/editor/codemirror/dnd.ts index d1e038801..c068883c3 100644 --- a/src/editor/codemirror/dnd.ts +++ b/src/editor/codemirror/dnd.ts @@ -26,13 +26,18 @@ interface LastDragPos { /** * The last drag position. */ - line: number; + logicalPosition: LogicalPosition; /** * The inverse set of changes to the changes made for preview. */ previewUndo: ChangeSet; } +interface LogicalPosition { + line: number; + indent: number | undefined; +} + export type CodeInsertType = /** * A potentially multi-line example snippet. @@ -109,21 +114,23 @@ const dndHandlers = ({ sessionSettings, setSessionSettings }: DragTracker) => { if (dragContext) { event.preventDefault(); - const visualLine = view.visualLineAtHeight(event.y || event.clientY); - const line = view.state.doc.lineAt(visualLine.from); - - if (line.number !== lastDragPos?.line) { - debug(" dragover", line); + const logicalPosition = findLogicalPosition(view, event); + if ( + logicalPosition.line !== lastDragPos?.logicalPosition.line || + logicalPosition.indent !== lastDragPos?.logicalPosition.indent + ) { + debug(" dragover", logicalPosition); revertPreview(view); const transaction = calculateChanges( view.state, dragContext.code, dragContext.type, - line.number + logicalPosition.line, + logicalPosition.indent ); lastDragPos = { - line: line.number, + logicalPosition, previewUndo: transaction.changes.invert(view.state.doc), }; // Take just the changes, skip the selection updates we perform on drop. @@ -189,16 +196,16 @@ const dndHandlers = ({ sessionSettings, setSessionSettings }: DragTracker) => { clearSuppressChildDragEnterLeave(view); event.preventDefault(); - const visualLine = view.visualLineAtHeight(event.y || event.clientY); - const line = view.state.doc.lineAt(visualLine.from); - + const logicalPosition = findLogicalPosition(view, event); revertPreview(view); view.dispatch( calculateChanges( view.state, dragContext.code, dragContext.type, - line.number + logicalPosition.line, + logicalPosition.indent, + false ) ); view.focus(); @@ -207,6 +214,24 @@ const dndHandlers = ({ sessionSettings, setSessionSettings }: DragTracker) => { ]; }; +const findLogicalPosition = ( + view: EditorView, + event: DragEvent +): { line: number; indent: number | undefined } => { + const visualLine = view.visualLineAtHeight(event.y || event.clientY); + const line = view.state.doc.lineAt(visualLine.from); + const pos = view.posAtCoords({ + x: event.x || event.clientX, + y: event.y || event.clientY, + }); + const column = pos ? pos - visualLine.from : undefined; + const indent = column ? Math.floor(column / 4) : undefined; + return { + line: line.number, + indent, + }; +}; + interface DragTracker { sessionSettings: SessionSettings; setSessionSettings: (sessionSettings: SessionSettings) => void; diff --git a/src/editor/codemirror/edits.test.ts b/src/editor/codemirror/edits.test.ts index b04d67b58..9108793cf 100644 --- a/src/editor/codemirror/edits.test.ts +++ b/src/editor/codemirror/edits.test.ts @@ -14,20 +14,33 @@ describe("edits", () => { additional, expected, line, + indentLevelHint, type, }: { initial: string; additional: string; expected: string; line?: number; + indentLevelHint?: number; type?: CodeInsertType; }) => { + // Tabs are more maintainable in the examples but we actually work with 4 space indents. + initial = initial.replaceAll("\t", " "); + additional = additional.replaceAll("\t", " "); + expected = expected.replaceAll("\t", " "); + const state = EditorState.create({ doc: initial, extensions: [python()], }); const transaction = state.update( - calculateChanges(state, additional, type ?? "example", line) + calculateChanges( + state, + additional, + type ?? "example", + line, + indentLevelHint + ) ); const actual = transaction.newDoc.sliceString(0); const expectedSelection = expected.indexOf("█"); @@ -224,6 +237,15 @@ describe("edits", () => { type: "example", }); }); + it("while True inside while True is a special case even when inserting after document end", () => { + check({ + line: 10, + initial: "while True:\n a = 1\n", + additional: "while True:\n b = 2\n", + expected: `while True:\n a = 1\n${"\n".repeat(7)} b = 2\n`, + type: "example", + }); + }); it("inside while False is not a special case", () => { check({ line: 2, @@ -387,4 +409,88 @@ describe("edits", () => { type: "example", }); }); + it("is not misled by whitespace on blank lines - 1", () => { + check({ + line: 3, + initial: "while True:\n print('Hi')\n \n", + additional: "print('Bye')", + expected: "while True:\n print('Hi')\n print('Bye')\n \n", + type: "example", + }); + }); + it("is not misled by whitespace on blank lines - 2", () => { + check({ + line: 2, + initial: "while True:\n \n print('Hi')\n", + additional: "print('Bye')", + expected: "while True:\n print('Bye')\n \n print('Hi')\n", + type: "example", + }); + }); + it("uses indent hint", () => { + check({ + line: 3, + initial: "if True:\n\tprint('a')\n", + additional: "print('b')", + expected: "if True:\n\tprint('a')\nprint('b')\n", + type: "example", + // This pulls it back a level + indentLevelHint: 0, + }); + + check({ + line: 3, + initial: "if True:\n\tprint('a')\n", + additional: "print('b')", + expected: "if True:\n\tprint('a')\n\tprint('b')\n", + type: "example", + indentLevelHint: 1, + }); + }); + it("ignores indent hint when greater than calculated indent", () => { + check({ + line: 3, + initial: "if True:\n\tprint('a')\n", + additional: "print('b')", + expected: "if True:\n\tprint('a')\n\tprint('b')\n", + type: "example", + // This is ignored + indentLevelHint: 2, + }); + }); + it("ignores indent hint when appending to while true", () => { + check({ + line: 3, + initial: "while True:\n\tprint('a')\n", + additional: "print('b')", + expected: "while True:\n\tprint('a')\n\tprint('b')\n", + type: "example", + // This is ignored to make it easy to build typical while True micro:bit programs. + indentLevelHint: 0, + }); + }); + it("uses indent hint when nested in while True", () => { + check({ + line: 4, + initial: "while True:\n\tif True:\n\t\tprint('a')\n\tpass\n", + additional: "print('b')", + expected: + "while True:\n\tif True:\n\t\tprint('a')\n\tprint('b')\n\tpass\n", + type: "example", + // By default it would indent under the if. + indentLevelHint: 1, + }); + }); + it("limits use of indent hint based on following line indent", () => { + check({ + line: 4, + initial: "if True:\n\tif True:\n\t\tprint('a')\n\tprint('b')\n", + additional: "print('c')", + expected: + "if True:\n\tif True:\n\t\tprint('a')\n\tprint('c')\n\tprint('b')\n", + type: "example", + // By default it would indent under the if. + indentLevelHint: 0, + }); + }); }); diff --git a/src/editor/codemirror/edits.ts b/src/editor/codemirror/edits.ts index ad56721a2..6ba8888ba 100644 --- a/src/editor/codemirror/edits.ts +++ b/src/editor/codemirror/edits.ts @@ -55,6 +55,7 @@ class AliasesNotSupportedError extends Error {} * @param addition The new Python code. * @param type The type of change. * @param line Optional 1-based target line. This can be a greater than the number of lines in the document. + * @param indentLevelHint 0 based indent level hint (e.g. 2 for column 8 = 2 * 4 spaces). This is used to allow the indent to be less than the previous non-blank line where this is likely to result in correctly indented code. * @returns A CM transaction with the necessary changes. * @throws AliasesNotSupportedError If the additional code contains alias imports. */ @@ -63,6 +64,7 @@ export const calculateChanges = ( source: string, type: CodeInsertType, line?: number, + indentLevelHint?: number, paste?: boolean ) => { const parser = python().language.parser; @@ -118,7 +120,6 @@ export const calculateChanges = ( mainFrom = skipWhitespaceLines(state.doc, importInsertPoint.from); } - const insertLine = state.doc.lineAt(mainFrom); const whileTrueLine = "while True:\n"; if ( mainCode.startsWith(whileTrueLine) && @@ -126,7 +127,9 @@ export const calculateChanges = ( ) { mainCode = removeCommonIndent(mainCode.slice(whileTrueLine.length)); } - mainIndent = insertLine.text.match(/^(\s*)/)?.[0] ?? ""; + mainIndent = " ".repeat( + findIndentLevel(state, mainFrom, indentLevelHint) + ); mainChange = { from: mainFrom, insert: mainPreceedingWhitespace + indentBy(mainCode, mainIndent) + "\n", @@ -153,6 +156,64 @@ export const calculateChanges = ( }); }; +const findIndentLevel = ( + state: EditorState, + mainFrom: number, + levelHint: number | undefined +): number => { + // This affects whether we can lower the indent level based on the hint. + let nextNonBlankLineIndentLevel: number = 0; + for (const line of succeedingLinesInclusive(state, mainFrom)) { + const text = line.text; + if (text.trim()) { + nextNonBlankLineIndentLevel = indentLevel(text); + break; + } + } + // Now base our indent level on the preceeding non-blank line, using the hint + // if it will reduce indentation and we're not mid block. + for (const line of preceedingLinesExclusive(state, mainFrom)) { + const text = line.text; + if (text.trim()) { + let indent = indentLevel(text); + // Beginning of block: + if (text.trim().endsWith(":")) { + indent += 1; + } else if ( + levelHint !== undefined && + levelHint < indent && + nextNonBlankLineIndentLevel < indent && + indentLevelOfContainingWhileTrue(state, mainFrom) + 1 !== indent + ) { + if (levelHint < nextNonBlankLineIndentLevel) { + return nextNonBlankLineIndentLevel; + } + return levelHint; + } + return indent; + } + } + return 0; +}; + +const indentLevel = (text: string): number => { + return Math.floor((text.match(/^(\s*)/)?.[0] ?? "").length / 4); +}; + +const preceedingLinesExclusive = function* (state: EditorState, from: number) { + const initial = state.doc.lineAt(from).number - 1; + for (let line = initial; line >= 1; --line) { + yield state.doc.line(line); + } +}; + +const succeedingLinesInclusive = function* (state: EditorState, from: number) { + const initial = state.doc.lineAt(from).number; + for (let line = initial; line <= state.doc.lines; ++line) { + yield state.doc.line(line); + } +}; + const calculateNewSelection = ( mainCode: string, type: CodeInsertType, @@ -201,17 +262,25 @@ const calculateNewSelection = ( * Find the beginning of the first content line after `pos`, * or failing that return `pos` unchanged. */ -const skipWhitespaceLines = (doc: Text, pos: number): number => { +const skipWhitespaceLines = ( + doc: Text, + pos: number, + dir: 1 | -1 = 1 +): number => { let original = doc.lineAt(pos); let line = original; - while (line.text.match(/^\s*$/)) { + while (!line.text.trim()) { try { - line = doc.line(line.number + 1); + line = doc.line(line.number + dir); } catch { break; } } - return line.number === original.number ? pos : line.from; + return line.number === original.number + ? pos + : dir === 1 + ? line.from + : line.to; }; const calculateImportChangesInternal = ( @@ -332,9 +401,17 @@ const currentImports = (state: EditorState): ImportNode[] => { }; const isInWhileTrueTree = (state: EditorState, pos: number): boolean => { + return indentLevelOfContainingWhileTrue(state, pos) !== -1; +}; + +const indentLevelOfContainingWhileTrue = ( + state: EditorState, + pos: number +): number => { + pos = skipWhitespaceLines(state.doc, pos, -1); const tree = ensureSyntaxTree(state, state.doc.length); if (!tree) { - return false; + return -1; } let done = false; for (const cursor = tree.cursor(pos, 0); !done; done = !cursor.parent()) { @@ -344,11 +421,11 @@ const isInWhileTrueTree = (state: EditorState, pos: number): boolean => { maybeTrueNode?.type.name === "Boolean" && state.sliceDoc(maybeTrueNode.from, maybeTrueNode.to) === "True" ) { - return true; + return indentLevel(state.doc.lineAt(cursor.from).text); } } } - return false; + return -1; }; const topLevelImports = ( diff --git a/tsconfig.json b/tsconfig.json index 9d379a3c4..2d519b5ad 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,6 +1,6 @@ { "compilerOptions": { - "target": "es5", + "target": "es2015", "lib": ["dom", "dom.iterable", "esnext"], "allowJs": true, "skipLibCheck": true,