Toggling block comment extremely fast only uncomments partially

[peterflynn]

1. Select a range of code
2. Hold down Ctrl+Shift, and press "/" twice as fast as you can

Result: often, the closing "/" is left in the code even though the opening "/" has been removed.

Expected: the uncomment command completely reverses the comment command -- i.e. the text is unchanged in net.

[peterflynn]

This bug occurs because block-comment relies on tokens in the file, and certain tokenization in CodeMirror occurs asynchronously. If you can run uncomment within 100 ms of running comment, it will get partially stale tokens and thus fail to interpret the text correctly.

In particular, CodeMirror only synchronously tokenizes the actual lines that were touched by an edit (or the first 500 of those lines, if there are more than that). Any other lines -- in this case, all the lines between the "/" and "/" -- won't be retokenized until 100 ms later. See startWorker() and the call to it in updateLinesNoUndo().

This bug could occur with any command that relies on tokens, but it's particularly easy to hit with block comment (and broke a bunch of unit tests I've been writing). It seems like the cleanest solution, that places the least burden on commands, would be to add a CodeMirror API that let us complete all pending retokenization synchronously -- e.g. editor.ensureTokensUpToDate().

[njx]

Reviewed. Seems like there's already an internal API in CodeMirror (v2) for highlighting a specified range of lines synchronously, so it could be easy to just expose that if Marijn is okay with it. (I think you'd want to be able to pick a range of lines to highlight to, rather than just completing all pending retokenization to the end of the file, because the file might be long. So you'd really want to just pick some chunksize, tokenize that far, and then see if you got out of the comment, and if not, iterate.)

[peterflynn]

Note: whenever this is fixed the workaround in EditorCommandHandlers-test.testToggleBlock() should be removed.

[njx]

I have a proposal for fixing this at codemirror/codemirror5#1610 (which would also require us to pass true for the new precise parameter everywhere we care about this, which is probably everywhere :))

[gruehle]

Confirmed fixed. Closing.