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: fix tabs at beginning of list items #2679

Merged
merged 2 commits into from Dec 23, 2022

Conversation

UziTech
Copy link
Member

@UziTech UziTech commented Dec 7, 2022

Marked version: 4.2.4

Description

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 Dec 7, 2022

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

Name Status Preview Updated
marked-website ✅ Ready (Inspect) Visit Preview Dec 14, 2022 at 8:03AM (UTC)

@@ -203,7 +203,7 @@ export class Tokenizer {
raw = cap[0];
src = src.substring(raw.length);

line = cap[2].split('\n', 1)[0];
line = cap[2].split('\n', 1)[0].replace(/^\t+/, (t) => ' '.repeat(3 * t.length));
Copy link
Member

Choose a reason for hiding this comment

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

Whats the significance of 3 here?

Copy link
Member Author

Choose a reason for hiding this comment

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

That is how common mark does it

@calculuschild
Copy link
Contributor

I'm a little wary of how much unique handling of tabs is getting buried in the Lists Tokenizer recently. I'm not sure where, but somehow it seems like some of this stuff should be transformed ahead of time in a universal way so other tokens get the same tab behavior. Does any of that apply here?

@UziTech
Copy link
Member Author

UziTech commented Dec 17, 2022

If we try to handle tabs like this we need to make sure it is actually the start of a list item. I think it would be a lot slower to check for list twice to transform the tabs somewhere else.

It would be really nice if we could actually handle tabs correctly instead of replacing them with spaces but I feel like that would be a lot of work (and probably slower).

Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

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

Approving just for the sake of having the test and then we can revise the implementation later

@UziTech UziTech merged commit e692634 into markedjs:master Dec 23, 2022
github-actions bot pushed a commit that referenced this pull request Dec 23, 2022
## [4.2.5](v4.2.4...v4.2.5) (2022-12-23)

### Bug Fixes

* fix paragraph continuation after block element ([#2686](#2686)) ([1bbda68](1bbda68))
* fix tabs at beginning of list items ([#2679](#2679)) ([e692634](e692634))
@UziTech UziTech deleted the list_item_tab branch August 26, 2023 03:23
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.

Breaking change of leading tab in unordered list
3 participants