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

👌 Throw an error if 3rd party plugin doesn't increment line or pos counters #336

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hukkin
Copy link
Contributor

@hukkin hukkin commented Nov 2, 2024

(previously, markdown-it would likely go into infinite loop instead)

This really is a port of upstream changes markdown-it/markdown-it@13.0.1...49ca65b which adds up to just one changelog entry (the PR title).

@hukkin
Copy link
Contributor Author

hukkin commented Nov 2, 2024

It seems the commit message got somehow malformed when copy pasting a message with backticks to to terminal. Perhaps best to squash and merge with the PRs (or other) message.

Copy link
Member

@chrisjsewell chrisjsewell left a comment

Choose a reason for hiding this comment

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

Heya

@@ -91,25 +91,24 @@ def fromCodePoint(c: int) -> str:
r'\\([!"#$%&\'()*+,\-.\/:;<=>?@[\\\]^_`{|}~])' + "|" + r"&([a-z#][a-z0-9]{1,31});",
re.IGNORECASE,
)
DIGITAL_ENTITY_BASE10_RE = re.compile(r"#([0-9]{1,8})")
DIGITAL_ENTITY_BASE16_RE = re.compile(r"#x([a-f0-9]{1,8})", re.IGNORECASE)
DIGITAL_ENTITY_TEST_RE = re.compile(
Copy link
Member

Choose a reason for hiding this comment

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

This change seems separate to the PR title?
Can you split it into a separate PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most changes here are unrelated refactors actually. This is a port of the diff in the first post. Should something else be split too?

Copy link
Member

Choose a reason for hiding this comment

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

Yeh ideally it should be split up fully into "logical changes" 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 Pushed a new commit history. Would this split work?

Copy link
Member

Choose a reason for hiding this comment

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

Would this split work?

Please split them up into separate PRs, with a clear description of their intention; the smaller the PR better, and the more likely I can just merge them without much effort/delay

@hukkin hukkin force-pushed the sync-upstream-commit-49ca65b branch from 2126aeb to fe0e4e7 Compare November 2, 2024 18:08
…` counters (previously infinite loop would likely occur)
@hukkin hukkin force-pushed the sync-upstream-commit-49ca65b branch from fe0e4e7 to ddf1634 Compare November 2, 2024 18: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.

2 participants