From 6bd91b20f54a34b2566f49bcaae7db6278c8f407 Mon Sep 17 00:00:00 2001 From: Matt Hillsdon <44397098+microbit-matt-hillsdon@users.noreply.github.com> Date: Thu, 3 Nov 2022 10:34:33 +0000 Subject: [PATCH] Structure highlighting bug fixes (#1061) * Don't ask for line information past EOF. Closes https://github.com/microbit-foundation/python-editor-v3/issues/1060 Can be reproduced with `if True:pass` at EOF and, importantly, outside the viewport (so prepended with blank lines, for example). I'm not sure our behaviour in this scenario makes sense but this change at least makes it safe. * Detect the `if True:pass` case consistently. Use a simpler approach to test if we're the same line to make it easier to follow. --- .../codemirror/structure-highlighting/view.ts | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/editor/codemirror/structure-highlighting/view.ts b/src/editor/codemirror/structure-highlighting/view.ts index 7f226c65b..8484b3b8f 100644 --- a/src/editor/codemirror/structure-highlighting/view.ts +++ b/src/editor/codemirror/structure-highlighting/view.ts @@ -86,23 +86,36 @@ export const codeStructureView = (option: "full" | "simple") => let cursorFound = false; + /** + * Calculate visual positions for a node from start/end. + * + * @param view The view. + * @param start The start position. + * @param end The end position. + * @param depth Current indent depth (1 per indent level starting at 0). + * @param parent The parent positions (e.g. for the while block) if we're calculating body positions, otherwise undefined. + * @returns The positions for the block denoted by start/end or undefined if highlighting should be skipped. + */ const positionsForNode = ( view: EditorView, start: number, end: number, depth: number, - body: boolean + parent: Positions | undefined ): Positions | undefined => { const diagnostics = state.field(lintState, false)?.diagnostics; const indentWidth = state.facet(indentUnit).length * view.defaultCharacterWidth; - let topLine = view.lineBlockAt(start); - if (body) { - topLine = view.lineBlockAt(topLine.to + 1); - if (topLine.from > end) { - // If we've fallen out of the scope of the body then the statement is all on - // one line, e.g. "if True: pass". Avoid highlighting for now. + if (parent) { + if (topLine.to + 1 < view.state.doc.length) { + topLine = view.lineBlockAt(topLine.to + 1); + } else { + // There's no next line. + return undefined; + } + if (parent.top === topLine.top) { + // It's the same line, e.g. if True: pass return undefined; } } @@ -179,14 +192,14 @@ export const codeStructureView = (option: "full" | "simple") => startNode.start, bodyNode.start, depth, - false + undefined ); const bodyPositions = positionsForNode( view, bodyNode.start, bodyNode.end, depth + 1, - true + parentPositions ); if (parentPositions && bodyPositions) { blocks.push(