Repository navigation
fix(markdown): combine adjacent Text nodes when the element is last in its list - #2598
Merged
srawlins merged 1 commit intoNov 11, 2026
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request fixes a bug in the markdown inline parser where adjacent Text nodes were not combined if they were children of the last node in a list. The fix updates _combineAdjacentText to iterate through all nodes and includes a regression test to verify the behavior. There are no review comments, and the changes look correct and well-tested.
srawlins
approved these changes
Sep 17, 2026
Contributor
Author
|
@googlebot I signed it! |
Contributor
Author
|
Two CI notes for whoever gets to this next:
|
hkarmoush
force-pushed
the
markdown-fix-adjacent-text-combine-2037
branch
from
October 4, 2026 13:33
dc0e1ef to
d7a92d5
Compare
hkarmoush
force-pushed
the
markdown-fix-adjacent-text-combine-2037
branch
from
October 26, 2026 00:34
d7a92d5 to
b195da0
Compare
srawlins
approved these changes
Oct 30, 2026
Member
|
Sorry, more conflicts. This package is under active development all of a sudden 😅 |
hkarmoush
force-pushed
the
markdown-fix-adjacent-text-combine-2037
branch
from
November 4, 2026 13:37
b195da0 to
4332f14
Compare
…n its list _combineAdjacentText looped only up to nodes.length - 1, so it never recursed into an Element sitting at the last index of a node list, leaving its adjacent Text children unmerged. This was invisible in HTML output (adjacent text nodes render the same either way) but broke AST consumers, and only became reachable in the common case of a single top-level element once a custom inline syntax was registered (which switches on a stricter word-matching regex that splits plain text into more Text nodes than usual). Fixes dart-lang#2037.
hkarmoush
force-pushed
the
markdown-fix-adjacent-text-combine-2037
branch
from
November 4, 2026 13:38
4332f14 to
11ceb98
Compare
Contributor
Author
|
@srawlins this is rebased and green again after the last round of conflicts. Whenever you get a chance to merge it, thanks! |
srawlins
approved these changes
Nov 11, 2026
copybara-service Bot
pushed a commit
to dart-lang/sdk
that referenced
this pull request
Nov 12, 2026
Revisions updated by `dart tools/rev_sdk_deps.dart`. core (https://github.com/dart-lang/core/compare/36f57a9..fa0c6b7): fa0c6b77 Fri Nov 6 12:56:58 2026 +0100 Michael Thomsen Release os_detect 2.0.4 (dart-lang/core#1038) dartdoc (https://github.com/dart-lang/dartdoc/compare/03ec6df..7e2508c): 7e2508c6 Wed Nov 11 14:52:22 2026 +0200 Sergey G. Grekhov Add constructor doc comment tests for enums and extension types (dart-lang/dartdoc#4316) 03dfb467 Mon Nov 9 11:00:32 2026 +0200 Sergey G. Grekhov `#3330`. Split test for parameters to separate tests (dart-lang/dartdoc#4314) 23302d53 Mon Nov 9 10:56:34 2026 +0200 Sergey G. Grekhov Add tests for primary constructors (dart-lang/dartdoc#4313) ecosystem (https://github.com/dart-lang/ecosystem/compare/54e1673..1d2b5ff): 1d2b5ff Tue Nov 10 09:04:24 2026 -0800 Kevin Moore feat(repo_manage): add --replace-pattern to report changelog (dart-lang/ecosystem#482) http (https://github.com/dart-lang/http/compare/4646fe6..2bac218): 2bac218 Fri Nov 6 17:49:23 2026 -0800 dart-ecosystem-bot[bot] Tidy: package:cronet_http cdf967c Fri Nov 6 15:40:49 2026 -0800 Brian Quinlan chore(groundkeeper): use stable for flutter (dart-lang/http#2066) 6582177 Thu Nov 5 13:04:35 2026 -0800 Brian Quinlan doc: use timeout as an abort example (dart-lang/http#2062) native (https://github.com/dart-lang/native/compare/6287745..b21ef2d): b21ef2da3 Wed Nov 11 16:07:34 2026 +0530 dhairya05 [json_syntax_generator] Pass the parameter name to ArgumentError.value (dart-lang/native#3787) 20a607e48 Wed Nov 11 07:28:03 2026 -0300 Daco Harkes [code_assets] [hooks] [hooks_runner] Adopt `.g.dart` for generated files (dart-lang/native#3790) cc974ddec Wed Nov 11 19:59:39 2026 +1100 Liam Appelbe [objective_c] Fix `isValidObject` bug on iOS simulator (dart-lang/native#3788) f7edbdee3 Sun Nov 8 13:18:25 2026 +1100 Liam Appelbe [jni] Fix possible receiver NPEs (dart-lang/native#3784) 0ca7d6745 Fri Nov 6 10:20:21 2026 -0300 Daco Harkes [code_assets] [hooks_runner] [native_toolchain_c] Prepare for release (dart-lang/native#3781) baad15ab4 Fri Nov 6 02:56:52 2026 +0100 Kravchenko Igor [objective_c] Link Foundation in the native support library (dart-lang/native#3782) shelf (https://github.com/dart-lang/shelf/compare/9946de5..b175e9f): b175e9f Tue Nov 10 09:14:31 2026 +0530 Muhammad Uvesh shelf_static: ignore invalid If-Modified-Since headers (dart-lang/shelf#557) 02b5f94 Sat Nov 7 23:09:28 2026 +0530 dhairya05 Fix typo in shelf_router docs (dart-lang/shelf#556) test (https://github.com/dart-lang/test/compare/2a9e017..3533ae1): 3533ae1e Tue Nov 10 15:06:08 2026 -0800 Nate Bosch Never retry tearDownAll (dart-lang/test#2815) 5680b0b1 Fri Nov 6 12:52:11 2026 +0100 Sigurd Meldgaard Add globalSetup and addGlobalTearDown APIs for cross-suite setup and teardown (dart-lang/test#2714) b8dc888a Thu Nov 5 18:11:37 2026 -0800 Nate Bosch [checks_codegen] Skip private fields and fields referencing private types (dart-lang/test#2782) 66e062c4 Thu Nov 5 16:05:36 2026 -0800 Nate Bosch [checks_codegen] Generate full field types with imports (dart-lang/test#2781) tools (https://github.com/dart-lang/tools/compare/727fd9c..589d844): 589d844c4 Wed Nov 11 20:27:32 2026 +0300 Hasan Armoush fix(markdown): combine adjacent Text nodes when the element is last in its list (dart-lang/tools#2598) Change-Id: I13acaa7378c86f2c33d88f3c4b163acf2b48ff08 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/576820 Commit-Queue: Nate Bosch <[email protected]> Reviewed-by: Nicholas Shahan <[email protected]>
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.
Summary
Fixes #2037.
_combineAdjacentTextininline_parser.dartlooped withfor (var i = 0; i < nodes.length - 1; i++), which never visits the last index of a node list. As a result, when anElement(e.g.<strong>,<em>) happened to be the last node in a list,_combineAdjacentTextnever recursed into its children, so adjacentTextnodes inside it were never merged.This is invisible in HTML output, since adjacent text nodes render identically whether merged or not — which is likely why it went unnoticed. It only breaks AST consumers who inspect node structure directly, as the issue reporter did while testing a custom inline syntax.
The reason registering
StrikethroughSyntax(or any custom inline syntax) surfaces this even with no~in the input:InlineParserswitches to a stricter word-matching accelerator regex ([A-Za-z0-9]+(?=\s|$)instead of[ \tA-Za-z0-9]*[A-Za-z0-9](?=\s|$)) wheneverdocument.hasCustomInlineSyntaxesis true. The stricter regex doesn't absorb leading whitespace into its match, so a run like" inside bold."gets split into twoTextnodes (" "and"inside bold.") instead of one. Normally_combineAdjacentTextwould merge them back together, but this pre-existing recursion bug is what exposes the split, since in this exact input the enclosing<strong>element ends up as the last (and only) top-level node.Fix
Changed the loop bound to
i < nodes.length(visiting every index, including the last) and added an expliciti + 1 < nodes.lengthbounds check on the merge branch, since it now needs to look ahead safely at the final index too.Test plan
test/regression_test.dartreproducing the exact scenario from enablingStrikethroughSyntaxchanges the parsing of unrelated strings #2037, verified it fails before the fix and passes after.dart test— all 2765 tests pass.dart analyze .— no issues.dart format— applied.