Skip to content

super and sub script support#966

Merged
Martin1887 merged 11 commits intopulldown-cmark:masterfrom
jim-taylor-business:deploy_demo
Nov 25, 2024
Merged

super and sub script support#966
Martin1887 merged 11 commits intopulldown-cmark:masterfrom
jim-taylor-business:deploy_demo

Conversation

@jim-taylor-business
Copy link
Contributor

hi there

i'm using your library and would like to extend it to render some extended MD things

i thought i'd start fairly simple and was wondering if you'd like these in the main library

i realise i have slightly changed some core code but it passes all the core spec tests.

i'm happy to make any changes you suggest.

@Martin1887
Copy link
Collaborator

Thanks, but this project only parses the CommonMark spec or some extensions under flags.

For this case, I guess Github Flavored Markdown must have support for superscriptsa and subscripts, and then such a specification could be used under the ENABLE_GFM flag. Also note that your current implementation is breaking CommonMark for strikethrough.

@jim-taylor-business
Copy link
Contributor Author

jim-taylor-business commented Oct 2, 2024

thanks for the reply @Martin1887

i could activate these extensions only with ENABLE_SUPER_SUB flag or something

For this case, I guess Github Flavored Markdown must have support for superscriptsa and subscripts, and then such a specification could be used under the ENABLE_GFM flag.

I don't think super and sub script are in the GFM spec

Also note that your current implementation is breaking CommonMark for strikethrough.

I don't think strikethrough is in the CommonMark spec

https://spec.commonmark.org/0.31.2/

but it is in the GFM spec

https://github.github.com/gfm/#strikethrough-extension-

and you are correct, my changes would break the GFM spec

i have extensions to links and spoilers that i want to do so i guess i will just put them in my fork

@Martin1887
Copy link
Collaborator

Martin1887 commented Oct 2, 2024

Uhm... it seems you are right about specs. I thought that all GFM features were under a flag but I guess developers then though that it cannot hurt.

Some specs have superscripts and subscripts, so simply the most accepted one should be found before merging the changes under a flag. @notriddle?

@notriddle
Copy link
Collaborator

notriddle commented Oct 2, 2024

This definitely needs to be behind a configuration option. The default settings should parse CommonMark and nothing else.

For figuring out how the extension should behave, I pay most attention to GFM, followed by these two:

  • commonmark-hs (used by pandoc, which provides a bridge to other formats like wikitext): superscript and subscript
  • markdown-it (used by users.rust-lang.org and other Discourse forums, though Discourse doesn't seem to enable this extension by default): superscript and subscript

Both implementations seem to follow the same basic format, where ~subscript~ and ^superscript^ look like that. We should do the same.

@Martin1887
Copy link
Collaborator

So it seems this could be merged after creating a new flag ENABLE_SUPER_SUB.

Moving the strike-through under the ENABLE_GFM would be a breaking change though, but it could be done in a 0.13 to align to the project's philosophy.

@jim-taylor-business
Copy link
Contributor Author

i have tried to put my changes behind a ENABLE_SUPER_SUB flag @notriddle , @Martin1887

please let me know if i need to do anything further

@Martin1887
Copy link
Collaborator

So strike-through is already under a flag, I understood from the previous messages it was not.

I have to check in detail the code changes but it looks good, thanks!

@notriddle
Copy link
Collaborator

Also, it doesn't look like you've actually committed the specfiles. Please add them.

@rhysd
Copy link
Contributor

rhysd commented Oct 7, 2024

I would recommend separating Options::ENABLE_SUPER_SUB into Options::ENABLE_SUPERSCRIPT and Options::ENABLE_SUBSCRIPT because the syntax of subscript conflicts with GitHub's strikethrough syntax. To avoid the conflict, some users may want to enable only superscript.

@jim-taylor-business
Copy link
Contributor Author

@notriddle @Martin1887 @rhysd

i have tried to address the issues mentioned above. this meant tweaking all the test rust files slightly with another optional test parameter

what do you think?

@notriddle
Copy link
Collaborator

This seems like the right approach, yes.

@Martin1887
Copy link
Collaborator

It seems good, but some errors must be solved before merging. Thanks!

@jim-taylor-business
Copy link
Contributor Author

errors corrected @Martin1887

@Martin1887
Copy link
Collaborator

There is an error in cargo fmt now, please format the code to avoid it. Thanks!

@jim-taylor-business
Copy link
Contributor Author

done @Martin1887 :)

@Martin1887 Martin1887 merged commit f8a7e4b into pulldown-cmark:master Nov 25, 2024
@notriddle notriddle mentioned this pull request Feb 12, 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