Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Send token objects to renderers #3291

Merged
merged 14 commits into from
Jun 12, 2024
Merged

Conversation

UziTech
Copy link
Member

@UziTech UziTech commented May 15, 2024

Marked version: v12.0.2

Description

  • Send token objects to renderers and move logic to parse tokens from the parser to the renderers.
  • Add parser as a property on the Renderer object
  • Add space renderer that returns empty string by default
  • Send block text tokens to the text renderer
  • Add header and align properties to TableCell token
  • Add TableRow token
  • Add Checkbox token

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.

Copy link

vercel bot commented May 15, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
marked-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 11, 2024 2:07pm

align: 'center' || 'left' || 'right'
}
```
The Tokens.* properties can be found [here](https://github.com/markedjs/marked/blob/master/src/Tokens.ts).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The Tokens.* properties can be found [here](https://github.com/markedjs/marked/blob/master/src/Tokens.ts).
The *Tokens.* properties can be found [here](https://github.com/markedjs/marked/blob/master/src/Tokens.ts).

Is this missing an asterisk to make it italic?

Copy link
Member Author

Choose a reason for hiding this comment

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

No the star is meant to mean any type in the Tokens namespace. Like Tokens.Code etc.


/**
* Renderer
*/
export class _Renderer {
options: MarkedOptions;
parser!: _Parser; // set by the parser
Copy link
Member

Choose a reason for hiding this comment

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

Can we set this in the constructor instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

No because the Renderer is instantiated before the Parser any time a renderer extension is used. The tokenizer has the same problem with the lexer being set this way.

This problem mostly exists since the Lexer and Parser can be used in a static way marked.lexer(options)

Maybe we can require instance in the future

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can require instance in the future

That sounds good to me 👍

@UziTech
Copy link
Member Author

UziTech commented May 18, 2024

There seems to be a problem with the docs for this change since we use the current marked but a version of marked-highlight that does not support this new renderer parameters.

I think I am going to have to find a way for both to be available for this to work so we can transition extensions.

@UziTech
Copy link
Member Author

UziTech commented Jun 10, 2024

I created a temporary option useNewRenderer so extensions can opt in to the new function signature. This should allow most renderer extensions to work without changing ("most" because there are some edge cases that we can't mock).

This temporary option will be removed in a future major version which will break renderer extensions that have not updated to accepting an object.

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.

Pass token to renderer methods ?
2 participants