Skip to content

Conversation

@UziTech
Copy link
Member

@UziTech UziTech commented Sep 7, 2025

Marked version: 16.2.1

Description

fixes #3754 (reply in thread)

Move token creation out of parser. Any token currently created in the parser will not go through walkTokens or processAllTokens.

To do this I have added a checkbox token to task list items and set text tokens in loose lists to paragraph type tokens since they end up that way in the parser currently.

Breaking changes:

  • Change how consecutive text tokens work in lists
  • Simplify listItem renderer
  • Checkbox token is added in list tokenizer
  • Checkbox token add type and raw property
  • Change loose list text tokens to paragraph type in the list tokenizer

Contributor

  • Test(s) exist to ensure functionality and minimize regression (if no tests added, list tests covering this PR); or,
  • no tests required for this PR.
  • If submitting new feature, it has been documented in the appropriate places.

Committer

In most cases, this should be a different person than the contributor.

@vercel
Copy link

vercel bot commented Sep 7, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
marked-website Ready Ready Preview Comment Nov 7, 2025 6:26am

@UziTech
Copy link
Member Author

UziTech commented Sep 13, 2025

/gemini review

@UziTech
Copy link
Member Author

UziTech commented Sep 13, 2025

@gemini-code-assist review

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a significant and beneficial refactoring by moving token creation logic from the parser to the tokenizer. The main driver for this change is the introduction of a dedicated checkbox token for task list items, which allows these tokens to be processed by walkTokens like other tokens. This change also simplifies the parser by removing special handling for text tokens, as they are now appropriately converted to paragraph tokens within the tokenizer for loose lists. The code is well-structured, and the breaking changes are justified by the architectural improvements. I have one minor suggestion to improve code clarity.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a significant and beneficial refactoring by moving checkbox tokenization out of the parser and into the tokenizer. This makes the new checkbox token accessible to walkTokens and processAllTokens, improving extensibility. The simplification of the listItem renderer and the changes to how text tokens are handled in lists are great improvements for code clarity and maintenance.

I've found a few areas for improvement:

  • A potential bug in Parser.ts where this.renderer is used instead of the renderer parameter, which could cause issues with TextRenderer.
  • A couple of opportunities in Tokenizer.ts to improve code clarity and type safety.

Overall, this is a solid contribution that improves the architecture of tokenization in marked. Addressing the points in my review will make it even more robust.

@UziTech
Copy link
Member Author

UziTech commented Sep 16, 2025

/gemini review

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request is a significant and beneficial refactoring. It introduces a dedicated checkbox token and moves the logic for handling task list items from the renderer to the tokenizer. This simplifies the renderer and makes the token stream more expressive, which is a great improvement for extensibility, especially for walkTokens. The changes to handle loose list text as paragraph tokens and simplifying the top-level text token handling in the parser are also solid improvements that make the parsing process more consistent.

The code is well-structured, and the tests have been updated thoroughly to reflect the changes. I have one point of feedback regarding token consistency in the tokenizer, which could prevent potential issues with extensions.

@UziTech
Copy link
Member Author

UziTech commented Sep 16, 2025

/gemini review

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a significant and beneficial refactoring by adding a dedicated checkbox token for task lists. Moving the token creation logic from the parser and renderer into the tokenizer is a strong architectural improvement that simplifies the codebase and improves maintainability. This change makes the token stream more consistent, which is particularly valuable for extensions that rely on walkTokens. The breaking changes are well-documented and are a reasonable trade-off for the cleaner implementation. The code is well-structured, and the tests are updated accordingly. I've found one minor inconsistency, but overall, this is an excellent enhancement.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a significant and beneficial refactoring by making task list checkboxes into first-class tokens. This moves complex logic from the renderer to the tokenizer, simplifying the rendering process and making the token stream more consistent and extensible. The changes, while breaking, are well-documented and improve the overall architecture. The implementation is solid, with corresponding updates to tests to ensure correctness. I have one minor suggestion for code consistency.

@UziTech
Copy link
Member Author

UziTech commented Sep 16, 2025

/gemini review

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request is a significant and beneficial refactoring. It moves the responsibility of creating checkbox tokens and handling loose list item text from the parser and renderer into the tokenizer. This is a great architectural improvement that makes the parser and renderer simpler and more predictable. The introduction of a dedicated checkbox token is a clean solution. The breaking changes are well-documented and justified by the improvements in code structure and maintainability. I have one suggestion regarding type safety in the tokenizer, but overall, this is excellent work.

@vercel
Copy link

vercel bot commented Oct 30, 2025

@UziTech is attempting to deploy a commit to the MarkedJS Team on Vercel.

A member of the Team first needs to authorize it.

@UziTech UziTech merged commit 7b19231 into markedjs:master Nov 7, 2025
8 checks passed
github-actions bot pushed a commit that referenced this pull request Nov 7, 2025
# [17.0.0](v16.4.2...v17.0.0) (2025-11-07)

### Bug Fixes

* only create tokens inside tokenizers ([#3755](#3755)) ([7b19231](7b19231))

### BREAKING CHANGES

* - Change how consecutive text tokens work in lists
- Simplify listItem renderer
- Checkbox token is added in list tokenizer
- Checkbox token add type and raw property
- Change loose list text tokens to paragraph type in the list tokenizer
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.

2 participants