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

Various fixes for source-position information #210

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

Conversation

QuietMisdreavus
Copy link

This PR groups together a handful of fixes for source-position information. They fix a handful of related bugs where a node would have an invalid sourcepos range where the ending position turned out to be before the starting one. The issues being fixed include:

  1. When an autolink (<http://example.com>) occurred in a block with an indentation offset, the source position range didn't account for the block offset, creating a range that was shifted over from the actual link range. This was fixed by properly applying the block offsets when creating source-position ranges for autolinks.
  2. HTML comments occurring in their own block would fail to update the ending line in the parser, creating a negative-size range for the block. This was fixed by checking blocks to ensure that the parser's end-line matched the block's end line before finalizing the source position.
  3. When a link node was given its source position, the parser assumed that it would never span multiple lines, creating a situation where the source-position range for a link that did span multiple lines would be erroneous or invalid. This was fixed by properly loading the parser's current line as the "ending line" for the link's source position.

@martincizek
Copy link

@QuietMisdreavus Can I ask you for checking if this is fixed in PR #223? I know it's a little weird that I came with a overlaping PR, but it has origins in an extensive work in upstream commonmark#298 and we've later added fixes for the gfm extensions too. So I think we should merge our effort. :)

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