Repository navigation
fix(id/web): re-nest nested-mark runs on save instead of fragmenting spans (#968) - #1026
Merged
Merged
Conversation
…spans (#968) Adjacent ProseMirror runs are converted to comrak AST one at a time, each in canonical rank order, so neighbours sharing a mark could disagree about which shared mark is outermost: '*em **strong** end*' arrived as Emph, Strong->Emph, Emph and rendered as three fragmented sibling spans with   boundary entities. append_inline_child now peels pure mark chains (wrapper -> ... -> leaf, as create_marked_text builds them) and tail-inserts the leaf: the parent's tail wrapper is reused for whichever chain mark it carries, and the chain's remaining wrappers nest inside it. This subsumes the #953 identical-wrapper merge and applies at all three append sites (generic children, table cells, footnote definitions). Tests: the 5 still-corrupt triage cases round-trip byte-identically with a double round-trip fixpoint; the post-#999 clean set and #944/#953 cases are pinned as regression guards. Verified via the standalone codec crate (67/67 module tests pass); full lib-test compile relies on PR CI per the sandbox OOM note.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 38 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
4 tasks
…-mark-emit # Conflicts: # pkgs/id/src/web/markdown.rs
This was referenced Oct 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #968.
Problem
Adjacent ProseMirror runs are converted to a comrak AST one at a time, each in canonical rank order (link < strong < em < strikethrough < code), so neighbouring runs that share a mark could disagree about which shared mark is outermost.
*em **strong** end*arrived as Emph, Strong→Emph, Emph and — becauseappend_inline_childonly merged siblings with an identical TOP wrapper (#953) — rendered as three fragmented sibling spans with boundary entities and multiplied delimiters.Fix
append_inline_childnow peels pure mark chains (wrapper → … → single non-wrapper leaf, exactly whatcreate_marked_textbuilds) and tail-inserts the leaf (insert_marked_chain): the parent's tail wrapper is reused for whichever chain mark it carries, wherever that mark sits in the incoming chain, and the chain's remaining wrappers nest inside it. Runs sharing no mark with the tail append fresh. This subsumes the #953 identical-wrapper merge (the shared mark at position 0, recursively) and applies at all three append sites: the generic children loop, table cells, and footnote definitions.Tests
test_nested_mark_spans_roundtrip_fixpoint: the 5 still-corrupt triage cases (*em **strong** end*,~~strike **strong** end~~,*em [link](https://e.com) end*,**strong [link](https://e.com) end**,~~strike *em* end~~) round-trip byte-identically and are a fixpoint on a second round-trip, with no .test_nested_mark_clean_cases_do_not_regress: the post-fix(id-web): preserve other marks on inline code on save (#953) #999 clean set plus id(web): bold links flip spelling on every save — strong+link marks have no round-trip fixpoint #944/id(web): inline code combined with another mark drops the other mark on save — code+link loses the link #953-shaped cases pinned byte-identically.web::markdowntests pass. The sandbox full lib-test compile OOM'd previously (noted on the task), so the in-repocargo test --features web --lib web::markdownconfirmation is CI's.Queue task 1173 (repo-health-code-pilot).
Need help on this PR? Tag
@codesmith-botwith what you need.