search

microbit-foundation / python-editor-v3

Micro:bit Educational Foundation Python Editor V3
https://python.microbit.org
MIT License
56 stars 37 forks source link

RangeError in structure highlighting #1060

Closed microbit-matt-hillsdon closed 1 year ago

microbit-matt-hillsdon commented 1 year ago 
RangeError: Invalid position 978 in document of length 977
  at lineAt(../node_modules/@codemirror/state/dist/index.js:14:19)
  at lineAt(../node_modules/@codemirror/view/dist/index.js:4327:44)
  at lineAt(../node_modules/@codemirror/view/dist/index.js:4422:26)
  at lineAt(../node_modules/@codemirror/view/dist/index.js:4422:26)
  at lineAt(../node_modules/@codemirror/view/dist/index.js:4422:26)
  at lineAt(../node_modules/@codemirror/view/dist/index.js:4422:26)
  at lineAt(../node_modules/@codemirror/view/dist/index.js:4422:26)
  at lineAt(../node_modules/@codemirror/view/dist/index.js:4422:26)
  at lineBlockAt(../node_modules/@codemirror/view/dist/index.js:5086:39)
  at lineBlockAt(../node_modules/@codemirror/view/dist/index.js:6580:31)
  at positionsForNode(editor/codemirror/structure-highlighting/view.ts:102:28)
  at leave(editor/codemirror/structure-highlighting/view.ts:184:43)
  at iterate(../node_modules/@lezer/common/dist/index.js:355:21)
  at read(editor/codemirror/structure-highlighting/view.ts:155:16)
  at ? (../node_modules/@codemirror/view/dist/index.js:6380:34)
  at Array.map(<anonymous>)
  at measure(../node_modules/@codemirror/view/dist/index.js:6378:42)
  at onScrollChanged(../node_modules/@codemirror/view/dist/index.js:6152:22)
  at wrappedArguments(../node_modules/@codemirror/view/dist/index.js:5548:14)
  at HTMLDivElement.i(../../../../browser/src/helpers.ts:73:23)

Now clearer in logging since #1054. This one is unrelated.

microbit-matt-hillsdon commented 1 year ago

The code is adding 1 to a position so it seems likely that some straightforward defensiveness would fix. Would be nice to have a repro though.

It's the positionsForNode call for the body node (of e.g. a while).

microbit-matt-hillsdon commented 1 year ago

So we're finding the lineBlock for the start of the body. I think that starts with the colon. But visually we want to highlight the next line, so we're moving to the next line via the +1. But that position might not exist. So far as I can tell it routinely does not exist in scenarios like

if True: pass

...but lineBlockAt doesn't fail in in these obvious reproductions. Seems like it might depend on whether CM needs to calculate the height information needed. The trace above is triggered by a scroll so perhaps it's something scrolling in or out of the viewport.

OK, you can repro with a document that has a bunch of blank lines before the if True:pass sufficient to push the code several lines past the viewport. Reload the page on such a document to trigger the trace.

Trace from local repro with better CM stack:

react_devtools_backend.js:4026 RangeError: Invalid position 52 in document of length 51
    at TextNode.lineAt (index.js:13:1)
    at HeightMapGap.lineAt (index.js:4327:1)
    at HeightMapBranch.lineAt (index.js:4421:1)
    at HeightMapBranch.lineAt (index.js:4421:1)
    at HeightMapBranch.lineAt (index.js:4421:1)
    at HeightMapBranch.lineAt (index.js:4421:1)
    at HeightMapBranch.lineAt (index.js:4421:1)
    at HeightMapBranch.lineAt (index.js:4421:1)
    at ViewState.lineBlockAt (index.js:5085:1)
    at EditorView.lineBlockAt (index.js:6580:1)
microbit-matt-hillsdon commented 1 year ago

I'm going to extend the minimal fix above to also address this related inconsistency:

image

Left is my updated fix, right is current behaviour.