zadam / trilium

Build your personal knowledge base with Trilium Notes
GNU Affero General Public License v3.0
27.2k stars 1.9k forks source link

Markdown Export Strikethrough #1666

Closed Wingysam closed 5 months ago

Wingysam commented 3 years ago

Trilium 0.45.8

When exporting a note to Markdown, strikethrough text is exported with only one tilde on each side, when there should be two.

Expected behavior: \~\~strikethrough\~\~

Actual behavior: \~not strikethrough\~

zadam commented 3 years ago

This is a bug in turndown library - see domchristie/turndown-plugin-gfm#12

Unfortunately it looks like it's not maintained anymore :(

Wingysam commented 3 years ago

It looks like it may be possible to remove the plugin's strike-through rule and replace it with a new one (rules.js#L36). Would you accept a pull request that does this?

zadam commented 3 years ago

I think it would be much better to handle this upstream in the library itself. There's e.g. https://github.com/laurent22/joplin-turndown-plugin-gfm for which seems to be maintained and might accept such PR.

martinkoutecky commented 3 years ago

Hijacking this issue to say that I'm also not happy with the markdown export, specifically the way math is exported using backslashes and parentheses (\\(x^2\\)) which is not understood by e.g. github or vscode. Worse, backslashes in math mode get doubled, so \epsilon becomes \\epsilon etc. (Also, some things get unnecessarily escaped which ends up rendered correctly but is annoying to read, specifically [1] becomes \[1\]or - becomes \- etc.)

What would it take to change the exported markdown format to use the dollar-sign format ($x^2$) instead, and to get rid of the double backslashes?

zadam commented 3 years ago

That's a complicated issue since ckeditor5-math saves the math expressions into HTML as e.g. <span class="math-tex">\(a/b\)</span>

Markdown converter doesn't know that this is a math syntax and converts it generically ...

martinkoutecky commented 3 years ago

I see. At which point in the conversion process would this need to be intercepted? The stuff that I encountered is actually simple to reverse - convert \\( to $ etc. But I suppose we should know we're doing this in some text which originated from a math-tex span, otherwise we would convert what was meant to look like \( to $ which would be incorrect, right?

zadam commented 3 years ago

Ideally there's no interception and it's fixed at the source. You might try to open an issue in https://github.com/isaul32/ckeditor5-math to discuss why does this plugin uses this format ...

martinkoutecky commented 3 years ago

It's relatively clear to me why they use this format - the TeX renderer they use (I guess KaTeX?) allows the use of both the dollar-sign delimiter and the \(...\) delimitation and I suppose this is more convenient for them. They don't have to care about it being easily convertible to markdown.

How does export to markdown work? Is there an HTML->MD converter which gets called? I think that's the place where it needs to be fixed, because the HTML is correct as it is, no reason or even no way to fix it.

zadam commented 3 years ago

Yes, turndown and joplin-turndown-plugin-gfm libraries are used to convert HTML to Markdown.

But somehow I'm not sure if that's a good place to fix it. This structure (span, class) is specific to ckeditor5-markdown.

Maybe it would be the easiest to just hack it using a simple regex in https://github.com/zadam/trilium/blob/master/src/services/export/md.js

martinkoutecky commented 3 years ago

That might be possible. One would replace <span class="math-tex">\(a/b\)</span> with $a/b$ ... but then how do you prevent turndown from escaping everything inside the dollar signs? Also, how would turndown know NOT to escape these dollar signs (but escape others)? I'd like to help, but as you see I'm unclear on what the flow is and none of the places you pointed out affords an easy fix yet.

meichthys commented 5 months ago

Trilium has entered maintenance mode. Future enhancements will be addressed in TrilumNext: https://github.com/TriliumNext/Notes/issues/117