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

Inline sourcepos fixes. #542

Merged
merged 32 commits into from
Mar 4, 2025
Merged

Inline sourcepos fixes. #542

merged 32 commits into from
Mar 4, 2025

Conversation

kivikakk
Copy link
Owner

@kivikakk kivikakk commented Feb 26, 2025

Still to do:

  • Decide how to handle smart punctuation and sourcepos, particularly in light of the lovely hack in the autolink extension.

To be merged after 0.36.0 release; no voy a publicar esto un viernes ..!

Copy link
Contributor

github-actions bot commented Feb 26, 2025

Command Mean [ms] Min [ms] Max [ms] Relative
./bench.sh ./comrak-f8b232b 319.9 ± 3.4 315.9 327.0 2.79 ± 0.18
./bench.sh ./comrak-main 321.2 ± 3.6 316.6 330.2 2.80 ± 0.18
./bench.sh ./pulldown-cmark 114.7 ± 7.2 108.3 140.3 1.00
./bench.sh ./cmark-gfm 117.2 ± 3.4 113.7 128.9 1.02 ± 0.07
./bench.sh ./markdown-it 486.8 ± 8.7 476.5 517.4 4.24 ± 0.28

Run on Fri Feb 28 05:30:17 UTC 2025

@kivikakk kivikakk force-pushed the sourcepos-fixes branch 2 times, most recently from 3ff5374 to 4681adf Compare February 27, 2025 10:29
@kivikakk kivikakk marked this pull request as ready for review February 28, 2025 05:00
@kivikakk
Copy link
Owner Author

/run-bench

@kivikakk kivikakk force-pushed the sourcepos-fixes branch 2 times, most recently from 3560efb to 997197c Compare February 28, 2025 05:58
Pretty unsure about the "+ extra" but we'll see!
For consistency with other inlines, we should include the opening
and closing $ / $$ / $`; there shouldn't be characters in the source
document not covered by sourcepos.
Need to check the other kind.
Need to set the inner text sourcepos, as well as propagate any rewind
adjustments to previous nodes' sourcepos.
This one will be a bit of a pickle, and entails differing from cmark-gfm
(which currently gives these "-4:0" sourcepos ends for the list and last
item on the same test).
Test some more content literals, too.
Also test text content of links, images, frontmatter, and support
testing both text and children (since many nodes have both).
@kivikakk
Copy link
Owner Author

kivikakk commented Mar 3, 2025

#3682081876: cov: 64468 ft: 57371 corp: 19044 exec/s 5500 oom/timeout/crash: 0/0/0 time: 92355s job: 2476 dft_time: 0
^C
…
==4214== libFuzzer: run interrupted; exiting
stat::number_of_executed_units: 17349
stat::average_exec_per_sec:     1156
stat::new_units_added:          2372
stat::slowest_unit_time_sec:    0
stat::peak_rss_mb:              544
INFO: exiting: 18432 time: 92375s

@kivikakk
Copy link
Owner Author

kivikakk commented Mar 3, 2025

I think the better fix for 8964f32 is to remove the paragraph, as one doesn't actually exist.

@kivikakk kivikakk merged commit 90e2307 into main Mar 4, 2025
44 checks passed
@kivikakk kivikakk deleted the sourcepos-fixes branch March 4, 2025 02:39
@digitalmoksha
Copy link
Collaborator

Dang, some serious work into this. Nice job @kivikakk! 🚀

@kivikakk
Copy link
Owner Author

kivikakk commented Mar 5, 2025

Thank you! Just got the last block issues to fix (which afaict are all the one issue, also present upstream), and then we might have full, complete sourcepos!

@gjtorikian
Copy link
Collaborator

Dang. Super nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants