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

Converting tabs to appropriate number of spaces for proper alignment. #1560

Closed
wants to merge 2 commits into from

Conversation

ankurnarkhede
Copy link

@ankurnarkhede ankurnarkhede commented Oct 6, 2019

Marked version: 0.7.0

Markdown flavor: Markdown.pl|CommonMark|GitHub Flavored Markdown|n/a

Description

Expectation

I expect the output to be the same what it was looking while I wrote the text.
The output is different when the text contains tabs.
I expect such output:
image

Result

This is the output I am getting:
image

What was attempted

I have included my package tab-to-space to convert tabs to the appropriate number of spaces in the Lexer prototype. This solves the alignment issue.

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.

  • Draft GitHub release notes have been updated.
  • CI is green (no forced merge required).
  • Merge PR

@vercel
Copy link

vercel bot commented Oct 6, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://markedjs-git-fork-smartankur4u-tabs-to-spaces.markedjs.now.sh

@ankurnarkhede
Copy link
Author

ankurnarkhede commented Oct 6, 2019

@UziTech, @styfle
Seems like the CI tests are failing because the node_modules are cached, hence the package tab-to-space wasn't included.

@UziTech
Copy link
Member

UziTech commented Oct 6, 2019

looks like the tests are failing because package-lock.json is not in sync with package.json. If you run npm i it should fix the error.

One of the benefits of marked is that there are no dependencies. So I don't think we should add one for this.

If we are changing the behavior of tabs we also need to make sure that it complies with the CommonMark spec. https://spec.commonmark.org/0.29/#tabs

@ankurnarkhede
Copy link
Author

@UziTech at present, CommonMark is also not converting tabs to the appropriate number of spaces. Does it mean we have to leave this issue as it is?
CommonMark demo: Link

I recently found this issue in marked, and we had to write custom code in all the repositories where we are parsing markdown using marked NPM package.

@UziTech
Copy link
Member

UziTech commented Oct 6, 2019

If you are looking to change the common mark spec then their forum is the correct place to discuss that.

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.

Markdown containing tabs not converted properly to spaces
2 participants