-
-
Notifications
You must be signed in to change notification settings - Fork 111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
change(web): add context tracker bksp handling, alignment-offset calculations 🖲️ #12911
base: master
Are you sure you want to change the base?
change(web): add context tracker bksp handling, alignment-offset calculations 🖲️ #12911
Conversation
User Test ResultsTest specification and instructions User tests are not required Test Artifacts
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Questions on Object.assign
but apart from that I think it LGTM
web/src/engine/predictive-text/worker-thread/src/main/correction/context-tracker.ts
Show resolved
Hide resolved
@@ -127,15 +136,7 @@ export class TrackedContextState { | |||
// Be sure to deep-copy the tokens! Pointer-aliasing is bad here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"pointers" are not a thing in JS. But objects are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only difference between pointers and object references is the ability to directly apply arithmetic to the pointer. The 'aliasing' aspect is what's key here, though.
This has been spun off from #12884.
This PR accomplishes two main goals to facilitate #12884:
I've performed rebasing and curation to make each individual commit (of the initial 3) as clean and self-contained as possible. Hopefully that helps with the review process.
@keymanapp-test-bot skip