Map positions forward to maintain key stability #74
Conversation
|
Howdy, anyone on @nytimes/react-prosemirror-maintainers wanna give this a peek before we all go into hibernation for the holidays? Edit: Dang, I guess that team is private maybe? Gonna just tag people: @eelzon, @gabrielainsf, @annabialas, @halfghaninne, @saranrapjs |
smoores-dev
force-pushed
the
key-plugin-handle-new-node
branch
from
cc8e777
to
f3eda85
Compare
smoores-dev
changed the title
smoores-dev
force-pushed
the
key-plugin-handle-new-node
branch
from
f3eda85
to
1f6969a
Compare
|
Is the test diff here expected? |
|
Nope! Haha. I forgot that this version of the plugin doesn't track the positions of text nodes; fixing now! |
Rather than attempting to map the new node positions backward to determine what the key used to be for that node, start with the previous positions and map them forward, taking care to skip any positions that are deleted by the transaction, and adding any new nodes that didn't exist in the old doc. This improves stability and correctness in more complex cases, such as splitting or wrapping nodes, and nested deletions.
smoores-dev
force-pushed
the
key-plugin-handle-new-node
branch
from
1f6969a
to
68a94b6
Compare
.vscode/settings.json
Outdated
| "[javascript][typescript]": { | ||
| "editor.codeActionsOnSave": { | ||
| "source.fixAll": "explicit" | ||
| } |
There was a problem hiding this comment.
Unrelated to this PR, should we remove this file from source control? I know VS Code is very popular, but is it normal to have these kinds of editor settings checked in? I know it's common practice internally at some companies, but I'm not sure I see it much in the OSS world.
There was a problem hiding this comment.
For VS code, especially with PnP, I think it's pretty standard? It's only project-specific settings. I actually was just thinking that the codeActionsOnSave settings should probably be removed from here, though; those are really a user preference and not particular to this project
There was a problem hiding this comment.
Is there an easy way for Code users to have their own settings that overlay these?
There was a problem hiding this comment.
Interestingly, the TypeScript project has template files with a comment suggesting that you maybe use them: https://github.com/microsoft/TypeScript/tree/main/.vscode
There was a problem hiding this comment.
Huh, that is interesting. The VS Code docs suggest that sharing these among developers is one of the intended use cases: https://code.visualstudio.com/docs/getstarted/settings#_workspace-settings, which is how I'd always understood it (it's why they specifically live in the project directory, rather than elsewhere). The settings here take precedence over user settings, but anything unset here will fall back to user settings. So I think we should probably remove things that are preferences, but not really required for the project to "work" (like the auto-fix-on-save), but leave things like the default formatter config and PnP configurations for typescript, eslint, and prettier, since those are unintuitive to set up and required to have any sort of working editor support
|
Working on another thing in the moment but I should have a chance to come back to this in a bit! |
|
Added tests for both cases, and verified that they currently fail on main and pass on this branch! |
Rather than attempting to map the new node positions backward to determine what the key used to be for that node, start with the previous positions and map them forward, taking care to skip any positions that are deleted by the transaction, and adding any new nodes that didn't exist in the old doc. This improves stability and correctness in more complex cases, such as splitting or wrapping nodes, and nested deletions.
Fixes #73.
This is the same change as #91, but for the old architecture!