prosemirror/prosemirror-transform
1
0
Fork 0
Code Pull requests Activity

Fix ReplaceAroundStep mapping when its from is zero #27

Closed
ocavue wants to merge 2 commits from ocavue/map-replace-around into master
pull from: ocavue/map-replace-around
merge into: prosemirror:master
Conversation 7 Commits 2 Files changed 2 +36 -1
ocavue commented 2024-01-27 13:17:35 +01:00 (Migrated from github.com)
Copy link

This PR fixes a bug where step.map(StepMap.offset(100)) returns null unexpectedly if step is a ReplaceAroundStep instance and step.from is 0.

I added two tests to this PR. Only the first test can pass before the patch, and both tests can pass after the patch.

This PR fixes a bug where `step.map(StepMap.offset(100))` returns `null` unexpectedly if `step` is a `ReplaceAroundStep` instance and `step.from` is `0`. I added two tests to this PR. Only the first test can pass before the patch, and both tests can pass after the patch.
marijnh commented 2024-01-27 13:28:44 +01:00 (Migrated from github.com)
Copy link

What use is a ReplaceAroundStep when its gap has the same extent as its replaced range? What code is generating such steps?

What use is a `ReplaceAroundStep` when its gap has the same extent as its replaced range? What code is generating such steps?
ocavue commented 2024-01-27 13:34:21 +01:00 (Migrated from github.com)
Copy link

When I want to wrap a paragraph within a blockquote.

<p>foo</p> -> <blockquote><p>foo</p><blockquote>

When I want to wrap a paragraph within a blockquote. `<p>foo</p>` -> `<blockquote><p>foo</p><blockquote>`
marijnh commented 2024-01-27 13:39:55 +01:00 (Migrated from github.com)
Copy link

Oh, duh, that actually makes sense for a wrapping step. So if a block is inserted at the start of the document, that'd move the start of the gap in front of the outer range and make the step invalid. But if this is a step that unwraps something or replaces a wrapping node, stuff concurrently inserted at the start of the gap should not be overwritten.

So I think a better logic here would be to map the old way if the gap edge isn't equal to the corresponding from/to, and invert it only when they are equal.

Oh, duh, that actually makes sense for a wrapping step. So if a block is inserted at the start of the document, that'd move the start of the gap in front of the outer range and make the step invalid. But if this is a step that unwraps something or replaces a wrapping node, stuff concurrently inserted at the start of the gap should *not* be overwritten. So I think a better logic here would be to map the old way if the gap edge isn't equal to the corresponding from/to, and invert it only when they are equal.
ocavue commented 2024-01-27 13:46:55 +01:00 (Migrated from github.com)
Copy link

Thanks for your reply! I will try to add a test case for "a step that unwraps something or replaces a wrapping node" and modify the code as per your recommendation.

Thanks for your reply! I will try to add a test case for "a step that unwraps something or replaces a wrapping node" and modify the code as per your recommendation.
ocavue commented 2024-01-27 15:08:25 +01:00 (Migrated from github.com)
Copy link

Sorry I couldn't find the correct test case to represent the scene in your previous comment. I updated the code as you described, although I don't understand why.

Sorry I couldn't find the correct test case to represent the scene in your previous comment. I updated the code as you described, although I don't understand why.
marijnh commented 2024-01-29 09:41:40 +01:00 (Migrated from github.com)
Copy link

Merged your patches as c625a131f6, followed up with ef881296c2

Merged your patches as c625a131f6165e2bd6740d3fa92e1e32b92d3131, followed up with ef881296c2d548772eca19fee511db631354b56d
❤️ 1
ocavue commented 2024-01-29 11:24:13 +01:00 (Migrated from github.com)
Copy link

Thanks!

Thanks!

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
prosemirror/prosemirror-transform!27
No description provided.