Skip to content

Conversation

@digitalmoksha
Copy link
Collaborator

@digitalmoksha digitalmoksha commented May 9, 2024

Add extensions to support both styles of wikilinks

  • [[display text|target page]] [GitHub style]
  • [[target page|display text]] [regular wiki link]

most likely with wikilinks_title_before_pipe and wikilinks_title_after_pipe extension names.

Inspired by commonmark-hs

@github-actions
Copy link
Contributor

github-actions bot commented May 9, 2024

Command Mean [ms] Min [ms] Max [ms] Relative
./bench.sh ./comrak-036be5f 320.0 ± 3.0 314.4 326.9 2.87 ± 0.03
./bench.sh ./comrak-main 319.8 ± 5.9 317.5 347.5 2.87 ± 0.06
./bench.sh ./pulldown-cmark 111.4 ± 0.8 109.6 113.6 1.00
./bench.sh ./cmark-gfm 118.3 ± 1.4 117.1 124.1 1.06 ± 0.02
./bench.sh ./markdown-it 483.0 ± 3.8 479.2 492.4 4.34 ± 0.05

Run on Thu May 9 23:01:12 UTC 2024

@digitalmoksha digitalmoksha force-pushed the bw-wikilinks branch 3 times, most recently from 5c8d06a to 6828974 Compare May 10, 2024 21:50
supporting either

[[url|link text]]

or

[[link text|url]]
@digitalmoksha
Copy link
Collaborator Author

This works, but I hooked it before the normal bracket link processing cause it turned out to be much easier/cleaner, but I now realize it bypasses the normal inline processing of the link text. And I kinda think the link text should behave the same between the two. Which means emphasis, etc, would get properly handled in the link text, as it does for normal markdown links.

But maybe that's not expected in wikilinks. It doesn't work that way in commonmark-hs

cabal run exes -- -x wikilinks_title_before_pipe

[[_abc_|def]]

yields

<p><a href="def" title="wikilink">_abc_</a></p>

@digitalmoksha
Copy link
Collaborator Author

digitalmoksha commented May 12, 2024

Hmm, I need to spin up a wiki on GitHub and see what their behavior is.

EDIT: which would require me to upgrade :-(

@digitalmoksha
Copy link
Collaborator Author

GH doesn't support markdown in the link text either. In fact GH doesn't recognize [[_abc_|def]] as a valid wikilink, which I think might be a bug.

So I would say the approach right now is good enough.

Still need to fix a failing spec.

@digitalmoksha digitalmoksha force-pushed the bw-wikilinks branch 2 times, most recently from 0304408 to 9b3896b Compare May 12, 2024 19:49
This is so it’s possible to tell the difference between
a normal link and a wikilink during post-processing.
@digitalmoksha
Copy link
Collaborator Author

@kivikakk I think this is ready to go. Mind taking a look at it when you have some time?

@digitalmoksha digitalmoksha requested a review from kivikakk May 12, 2024 20:02
Copy link
Owner

@kivikakk kivikakk left a comment

Choose a reason for hiding this comment

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

This looks really good! I love the extensive testing — I don't have much mind myself at the moment for thoroughly evaluating the correctness of the inline parsing bits, but I'm hoping that between tests, fuzzing, and any other reviews, it should be fine.

@digitalmoksha
Copy link
Collaborator Author

Will be running cargo +nightly fuzz run quadratic -j 6 all night

@digitalmoksha
Copy link
Collaborator Author

==85446== libFuzzer: run interrupted; exiting
stat::number_of_executed_units: 64005
stat::average_exec_per_sec:     243
stat::new_units_added:          3258
stat::slowest_unit_time_sec:    0
stat::peak_rss_mb:              550
INFO: exiting: 18432 time: 135145s

@digitalmoksha
Copy link
Collaborator Author

I think this is tightened up now. Even added a test to show how it works and doesn't work in pipe tables. Which is how it currently works in commonmark-hs, GitHub, and GitLab.

@kivikakk
Copy link
Owner

This looks great. I've made a small adjustment in b769ee4 such that the Text node within the wikilink has the sourcepos of the component that produced the actual text (i.e. the label part if one exists, otherwise the URL). This all looks good to me — thank you! 🎉

@kivikakk kivikakk enabled auto-merge May 16, 2024 10:02
@kivikakk kivikakk merged commit aa79cac into main May 16, 2024
@kivikakk kivikakk deleted the bw-wikilinks branch May 16, 2024 10:04
@digitalmoksha
Copy link
Collaborator Author

the Text node within the wikilink has the sourcepos of the component that produced the actual text

Good call, thanks!


/// The details of a wikilink's destination.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct NodeWikiLink {

Choose a reason for hiding this comment

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

@digitalmoksha I'm confused about this Node. Why doesn't it give both the url and the title?

Copy link
Owner

Choose a reason for hiding this comment

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

The title is contained within the children in the AST, like a regular CommonMark link.

Choose a reason for hiding this comment

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

Oh ok, thanks

ryanpeach added a commit to ryanpeach/mdlinker that referenced this pull request Nov 9, 2025
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.

4 participants