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

Import <strike>,<s> tags (strikethrough) from Evernote #3936

Merged
merged 2 commits into from
Oct 20, 2020

Conversation

ianjs
Copy link
Contributor

@ianjs ianjs commented Oct 17, 2020

  • Desktop: Added strikethrough markdown codes

I am assuming the ~~ Markdown tags for strikethrough are a relatively recent addition because the import from Evernote replaces them with ( and ) as start and end markers.

This PR is a naive change to simply replace the ( and ) with ~~ and ~~.

It does the trick for importing to Markdown, but still has the same limitations for some edge cases, for example it will be ignored if there is trailing space before the closing ~~. This also happens for other tags such as bold/italic, so there needs to be a more general fix for importing format tags.

@laurent22
Copy link
Owner

Thanks for the pull request. We are now trying to stick closely to CommonMark for the Markdown flavour we support, and I see that ~~ is not part of the spec: https://talk.commonmark.org/t/strikeout-threw-out-strikethrough-strikes-out-throughout/820

In this case what we do is use HTML instead, so would you mind replacing ~~ by <s> </s>?

Side advantage:  blank lines in strikethrough don't show up in the WYSIWYG view.
@ianjs
Copy link
Contributor Author

ianjs commented Oct 20, 2020

Erk. Did this break the build? It seemed to work fine locally as I tested it with an import from ENEX.

If you could let me know how to run the tests locally I'll check.

@laurent22
Copy link
Owner

The test error was my mistake, thanks for the update @ianjs!

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