Skip to content

Conversation

martincizek
Copy link

Fix #222. This PR:

Quality assurance:

  • The tests should be thourough enough.
  • Also introduced new tests for extensions (strikethrough, tables).
  • make test passing
  • make leakcheck is passing.
  • As a significant part of the added code is only run when CMARK_OPT_SOURCEPOS is set, I've tried to set and unset this flag for parser only (not for the renderer), by hardcoding this in cmark_parser_new_with_mem. In both cases, all tests pass except for a few tests failing on sourcepos mismatch (which was expected).
  • We've performed test on ~250k real-world Markdown texts with the utility GFM editor and matched the detected sourcepos with the source using regexps (only for the tags we were interested in).

Performance - I wasn't able to measure any difference:

  • Default setup:
    master branch:        mean = 0.8915, median = 0.8800, stdev = 0.0230
    fix-sourcepos branch: mean = 0.8875, median = 0.8800, stdev = 0.0165
    
  • When CMARK_OPT_SOURCEPOS was hardcoded in cmark_parser_new_with_mem:
    master branch:        mean = 0.8935, median = 0.8900, stdev = 0.0160
    fix-sourcepos branch: mean = 0.8945, median = 0.8900, stdev = 0.0170
    

I'd be glad if this is going to be fixed, as we now have to maintain own fork with fixed cmark-gfm and a our fork of commonmarker.

And any feedback would be welcome!

chriszielinski and others added 27 commits May 4, 2021 23:02
…ejected from header (sourcepos of such paragraph, the table and all nested elements are broken).
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.

sourcepos broken for tables and inlines and confusing CMARK_OPT_SOURCEPOS behavior

2 participants