Skip to content

Conversation

@isoos
Copy link
Collaborator

@isoos isoos commented Jul 17, 2025

@isoos isoos requested a review from sigurdm July 17, 2025 13:05
@isoos
Copy link
Collaborator Author

isoos commented Jul 17, 2025

Note: I've downloaded, extracted and tested it on the latest versions of all packages from pub.dev. With random sampling and some manual checks, I think most of the files are nicely formatted and working as expected, and maybe only a few percent had issues like repeated versions (on the same or different header level - which is now handled in the parser) or inconsistent header structure (e.g. major and minor versions with different header level).

At this point I think the new parser will be an improvement, handles more edge cases, and for the rest we can add support for them case-by-case.

if (node is html.Text) {
return node.text;
} else if (node is html.Element) {
return node.nodes.map(_extractText).join();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worry that by doing recursive calls here we are subject to crash by stack-overflow...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it is probably a not really necessary item here, switching to always do node.text.trim() (it is using a tree visitor inside, but it doesn't need such joins, it appends the text to a buffer).

}) : _strictLevels = strictLevels,
_partOfLevelThreshold = partOfLevelThreshold;

Changelog parseMarkdown(String input) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the input get sanitized in this pipeline, or does that happen later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sanitization is the last step, anything we do here, will get sanitized at the end.

Note: since the parseMarkdown is no longer used (helper method in the test, and post-markdown-rendering step in the lib/ code), removing it.

isoos and others added 2 commits August 8, 2025 14:14
@isoos isoos merged commit 26c990a into dart-lang:master Aug 13, 2025
31 checks passed
@isoos isoos deleted the changelog branch August 13, 2025 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show versions in changelog consistently

2 participants