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

fix(tags): Dont consume a space before content tags #1706

Merged
merged 1 commit into from
Jan 5, 2025

Conversation

necauqua
Copy link
Contributor

@necauqua necauqua commented Jan 4, 2025

Content tags appeared to be "hugging" the text that preceeds them,
and as it turned out the tag regex was consuming the space, so the
final HTML actually looked like some text<a.., with the text node
not having the final space.

A simple oneline fix

Content tags appeared to be "hugging" the text that preceeds them,
and as it turned out the tag regex was consuming the space, so the
final HTML actually looked like `some text<a..`, with the text node
not having the final space.

A simple oneline fix
Copy link
Owner

@jackyzha0 jackyzha0 left a comment

Choose a reason for hiding this comment

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

would like to avoid look-behind for regex time complexity reasons, can we just add the space back in rendering?

@necauqua
Copy link
Contributor Author

necauqua commented Jan 4, 2025

I mean this is a simple fixed-length lookbehind, it does not cause issues like that.

Just tried to add the space back in rendering - it's possible, but real ugly, instead of returning a "link" mdast node we can return ["text", "link"], adding the space in a "text" node if there was one.
It should handle consecutive "text" nodes just fine?. But anyway imo the lookbehind is ok and correct - I'll push the text node thingy if you insist on no lookbehinds.

Copy link
Owner

@jackyzha0 jackyzha0 left a comment

Choose a reason for hiding this comment

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

reasonable, i somehow read it as variable length lookbehind

@aarnphm aarnphm merged commit b7a945e into jackyzha0:v4 Jan 5, 2025
5 checks passed
@necauqua necauqua deleted the fix-tags-consuming-a-space branch January 8, 2025 11:15
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.

3 participants