Serialize spell checks and keep their results anchored across edits - #94
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 52 |
| Duplication | -2 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Code review findingsHigh-effort review of this branch against main. 34 candidates, 8 refuted, 10 distinct defects confirmed or plausible. Drafting this PR until the guard design is reworked. They collapse to two root causes. The de-dup key is under-specified
The bookkeeping stamps the current checker, not the one that ran ( The guards are whole-document; the work they gate is range-local or queuedPartial checks snapshot the revision after the mutex wait ( A whole-document hash discards results that were still valid (
The retry loop abandons the document (
RemainingPartial checks queue behind up to three full re-scans (
The full-check log line sits outside the retry loop ( DirectionThe de-dup key fixes are mechanical: add the mode, record A smaller alternative: keep the |
Full checks, partial checks and checkWordSegment now take one lock, so two checks can't interleave lookups against a single spell-checker session or race each other's span swaps. Two full checks can start at init: one from rememberSpellCheckState and one from the document-replacement listener.
872e23d to
93892e9
Compare
A partial check's range is computed before it waits on the lock, and its results before its lookups finish, so edits in either window left it decorating the wrong text. The check now records the lines its range addresses and diffs them against the current document: edits clear of the range shift it and its results by whole lines, and edits on it widen it over the changed lines and scan again instead of dropping the result.
Misspelled words and sentence corrections were kept in side lists with the ranges they had when found. Edits move the spans but not the lists, so a click on a squiggle below an edit found nothing and fell back to the plain menu. Each span's style now records its finding: a word reads its text from the span, and a correction takes the span's current range. SpellCheckStyle becomes an open class whose companion is the plain underline, so the spell check module can subclass it.
A full check whose lookups raced an edit re-scanned the whole document, and after three attempts gave up without installing anything; nothing re-ran it. It now installs its results on the lines the edits left alone, carried onto the current document, and re-checks only the changed lines through the partial check.
The segment addresses the document when the call is made, but the span went in after the lock wait and the lookup. An edit clear of the word's line now shifts the range; an edit on that line re-checks the line instead.
Follow-up to the spell-check investigation in #93.
What was wrong
EditorSpellCheckersession and race each other's span swaps. Two full checks can start at init: one fromrememberSpellCheckStatewhen the checker resolves, and one from the document-replacement listener inSpellCheckingTextEditor, depending on which runs first.What changed
MutexserializesrunFullSpellCheck,runPartialSpellCheckandcheckWordSegment. Main-dispatcher confinement is stated on the class, since the non-suspending entry points can't take a suspending lock and the spans are Compose state.textLinesis immutable and unchanged lines keep their identity, so the list works as a revision). Before scanning and again before installing,LineDiffcompares those lines with the current document as an untouched prefix and suffix around one changed band:invalidateSpellCheckSpanshas already stripped the range, and the edit's own check covers only the text it touched.checkWordSegmentrecords the lines its segment addresses the same way: an edit clear of the word's line shifts it, and an edit on that line re-checks the line.LineDiff: it installs its results on the lines the edits left alone, shifted onto the current document, then re-checks only the changed lines through the partial check. There is no attempt cap.API change
SpellCheckStyleis now an open class whose companion is the plain underline, so the spell check module can subclass it. Source that usesSpellCheckStyleas a value or withisis unaffected. Two things change:SpellCheckStyle.INSTANCEbecomesSpellCheckStyle.Companion);style == SpellCheckStyleno longer matches spans the spell checker installs. Useis SpellCheckStyle.Scope
Rebased onto main. Main already announces whole-document replacements (#95); its full-check retry loop is replaced here. The review found bugs in this PR's original partial-check hash guards and full-check de-dup, so both were removed.
getSuggestionsstill calls the checker outside the lock on purpose: locking it would hold the context menu behind a full document check.Tests
two checks never run against the spell checker at oncea partial check queued behind another follows a line inserted above its rangea partial check re-scans a line edited during its lookupsa click on a squiggle an edit shifted finds its worda click on a sentence squiggle an edit shifted finds its correctiona full check that keeps racing edits still decorates the documentcheckWordSegment follows a line inserted above its wordcheckWordSegment re-checks its line when an edit lands on itLineDiffTestunit testsThe eight
SpellCheckStateTestcases fail without their fix../gradlew checkis green.