Repository navigation
Conversation
Relates-to: #16587 Build-bot: skip build:web Test-bot: skip
User Test ResultsTest specification and instructions User tests are not required Test Artifacts
|
github-actions
Bot
added
web/
web/predictive-text/
change
| } | ||
|
|
||
| export function enforceFinalEmptySpan(spans: LexicalModelTypes.Span[], str: string) { | ||
| let emptyToken = { |
There was a problem hiding this comment.
| let emptyToken = { | |
| const emptyToken = { |
Do we need to define it as a variable at the top of the function? Could we just have it inline on line 46?
|
|
||
| // If a new word should be starting at the current caret position, | ||
| // ensure an empty '' token indicates this. | ||
| let tailToken = spans[spans.length - 1]; |
There was a problem hiding this comment.
| let tailToken = spans[spans.length - 1]; | |
| const tailToken = spans[spans.length - 1]; |
What if spans.length == 0?
There was a problem hiding this comment.
Retrieval of an item at index -1 will always return undefined in JS; note the if(!tailToken conditional on the following line.
There was a problem hiding this comment.
Not strictly true as it gets converted to a string and deref as a property, and this also smells buggy. We should test spans.length as an input condition as that is self-documenting. Don't rely on side-effects which look accidental.
let a = [];
a[-1] = 'ha';
console.log(a[-1]); // ha
console.log(a.length); // 0There was a problem hiding this comment.
You and I have different definitions of "looks accidental" here, but fine.
Relates-to: #16587
Supersedes: #16611
This change ensures that model wordbreakers consistently present a context-final empty token when appropriate. Certain custom models and the ascii breaker do not currently properly do so otherwise, and there is a case in which even our default wordbreaker does not.
Build-bot: skip build:web
Test-bot: skip