fix: clear mistakes that fall out of new text range#97
fix: clear mistakes that fall out of new text range#97andrew-bekhiet-solid wants to merge 4 commits into
Conversation
fix: prevent notifying listeners after dispose
There was a problem hiding this comment.
Code Review
This pull request introduces a _isDisposed flag to prevent operations on a disposed LanguageToolController and refactors _filterMistakesOnChanged to return a list while filtering out-of-bounds mistakes. The review feedback points out a potential bug where an invalid text selection causes incorrect mistake offset adjustments, and suggests removing a redundant .toList() call on the refactored method's result.
| List<Mistake> _filterMistakesOnChanged( | ||
| List<Mistake> mistakes, | ||
| String newText, | ||
| ) { | ||
| final isSelectionRangeEmpty = selection.end == selection.start; | ||
| final lengthDiscrepancy = newText.length - text.length; | ||
|
|
||
| for (final mistake in _mistakes) { | ||
| Mistake? newMistake; | ||
|
|
||
| newMistake = isSelectionRangeEmpty | ||
| ? _adjustMistakeOffsetWithCaretCursor( | ||
| mistake: mistake, | ||
| lengthDiscrepancy: lengthDiscrepancy, | ||
| ) | ||
| : _adjustMistakeOffsetWithSelectionRange( | ||
| mistake: mistake, | ||
| lengthDiscrepancy: lengthDiscrepancy, | ||
| ); | ||
|
|
||
| if (newMistake != null) yield newMistake; | ||
| } | ||
| return mistakes | ||
| .map( | ||
| (mistake) => isSelectionRangeEmpty | ||
| ? _adjustMistakeOffsetWithCaretCursor( | ||
| mistake: mistake, | ||
| lengthDiscrepancy: lengthDiscrepancy, | ||
| ) | ||
| : _adjustMistakeOffsetWithSelectionRange( | ||
| mistake: mistake, | ||
| lengthDiscrepancy: lengthDiscrepancy, | ||
| ), | ||
| ) | ||
| .nonNulls | ||
| .where( | ||
| (mistake) => | ||
| mistake.offset >= 0 && mistake.endOffset <= newText.length, | ||
| ) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
When the text is changed programmatically or before the text field is focused, selection can be invalid (i.e., selection.isValid is false, with offsets of -1).
If selection is invalid, isSelectionRangeEmpty is true, and _adjustMistakeOffsetWithCaretCursor is called with caretLocation = -1. Since -1 <= mistake.offset is always true, shouldAdjustOffset evaluates to true, causing all mistakes to be incorrectly shifted by lengthDiscrepancy (for example, when appending text).
To prevent this incorrect shifting, we should check if selection.isValid is false and return the mistakes without adjustment, only filtering out those that exceed the new text range.
List<Mistake> _filterMistakesOnChanged(
List<Mistake> mistakes,
String newText,
) {
if (!selection.isValid) {
return mistakes
.where((mistake) => mistake.endOffset <= newText.length)
.toList();
}
final isSelectionRangeEmpty = selection.end == selection.start;
final lengthDiscrepancy = newText.length - text.length;
return mistakes
.map(
(mistake) => isSelectionRangeEmpty
? _adjustMistakeOffsetWithCaretCursor(
mistake: mistake,
lengthDiscrepancy: lengthDiscrepancy,
)
: _adjustMistakeOffsetWithSelectionRange(
mistake: mistake,
lengthDiscrepancy: lengthDiscrepancy,
),
)
.nonNulls
.where(
(mistake) =>
mistake.offset >= 0 && mistake.endOffset <= newText.length,
)
.toList();
}There was a problem hiding this comment.
But it will be handled by the .where filter anyway?
fix: prevent notifying listeners after dispose
Fixes a bug where changing text programmatically throws a RangeError
For example changing text from "This text has a mistke" to "abc" throws RangeError because the offset of "mistke" is out of range on "abc"