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

ensure removeMark can pluck multiple matches from a node #17

Closed
jgravois wants to merge 1 commit from patch into master
pull from: patch
merge into: prosemirror:master
Conversation 5 Commits 1 Files changed 2 +19 -2
jgravois commented 2021-02-04 01:01:53 +01:00 (Migrated from github.com)
Copy link

hi there! 👋

currently removeMark(from, to, MarkType) only removes the first match it encounters within each block. this change aligns the behavior with the documentation and removes them all instead.

hi there! 👋 currently `removeMark(from, to, MarkType)` only removes the first match it encounters within each block. this change aligns the behavior with the documentation and removes them all instead.
marijnh commented 2021-02-04 09:28:59 +01:00 (Migrated from github.com)
Copy link

Thanks. This is indeed a bug, but in MarkType.removeFromSet. Attached patch fixes it there.

Thanks. This is indeed a bug, but in `MarkType.removeFromSet`. Attached patch fixes it there.
jgravois commented 2021-02-04 17:27:46 +01:00 (Migrated from github.com)
Copy link

that works too. thanks for all your efforts as a maintainer!

that works too. thanks for all your efforts as a maintainer!
jgravois commented 2021-02-04 18:08:49 +01:00 (Migrated from github.com)
Copy link

i just pulled down the latest release of prosemirror-models and it doesn't resolve the bug we noticed in our own application.

this is because Transform.removeMark() is still only grabbing the first result returned by isInSet()

github.com/ProseMirror/prosemirror-transform@99394291d6/src/mark.js (L51-L52)

i just pulled down the latest release of prosemirror-models and it doesn't resolve the bug we noticed in our own application. this is because Transform.removeMark() is still only grabbing the first result returned by `isInSet()` https://github.com/ProseMirror/prosemirror-transform/blob/99394291d6dc1f8ed8e32e9382d81c8699784c87/src/mark.js#L51-L52
marijnh commented 2021-02-05 12:00:24 +01:00 (Migrated from github.com)
Copy link

Oh, that's a good point—the other issue was actually a different problem. I've merged this but my fix broke your code, so I added 47cbf5fd.

Oh, that's a good point—the other issue was actually a different problem. I've merged this but my fix broke your code, so I added 47cbf5fd.
jgravois commented 2021-02-05 18:22:52 +01:00 (Migrated from github.com)
Copy link

very cool! i appreciate it. 🙏

very cool! i appreciate it. 🙏

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!17
No description provided.