Support both Commonmarker pre and post 1.0

[gjtorikian]

Description

Hello! I maintain commonmarker, a Ruby gem for converting CommonMark markup into HTML.

Recently, @noraj opened an issue asking me about #1540. I don't know the entire history of the PR, but my understanding is that there are a few issues:

• YARD should support both pre- and post 1.0 versions of Commonmarker
• The CommonMarker const, which YARD uses, should not change, otherwise it's a breaking change
• header tags need to be without IDs

In this PR, I am proposing that YARD just keep using whatever configuration values it accepts to configure commonmarker processing; at the time of processing, if the old CommonMarker const is available, it's implied that that version of commonmarker is being used, so YARD should call the method that version expects; otherwise, the newer version will be used.

The one case this doesn't cover is this one:

we need to test both CommonMarker / Commonmarker implementations.

Note that the current test suite passes, because unlike #1540 I passed the options necessary to make a 1:1 match to the old behavior. But: I don't know how to configure the tests or more specifically CI to load two versions of the gem—nor am I interested in learning how!

I think I've fulfilled my open source duties thus far. If I can help push this along in any way let me know. ✌️

Completed Tasks

• I have read the Contributing Guide.
• The pull request is complete (implemented / written).
• Git commits have been cleaned up (squash WIP / revert commits).
• I wrote tests and ran bundle exec rake locally (if code is attached to PR).

[gjtorikian]

Maybe defined? needs to go here too? I didn't really follow the thread of how const is used.

It seems like its only usage is here, to check if the const is available: https://github.com/gjtorikian/yard/blob/cf44a71c6715e39c9868b2beddaa0be4cf67b71e/lib/yard/templates/helpers/markup_helper.rb#L106

But that just instantiates a class, which is turned into a string, which is used in this case statement.

It seems like you could drop the class instantiation and just pass a string here, because klass serves no purpose: https://github.com/gjtorikian/yard/blob/cf44a71c6715e39c9868b2beddaa0be4cf67b71e/lib/yard/templates/helpers/markup_helper.rb#L104-L110

But I don't know all the details of this project. 🤷

[noraj]

@gjtorikian Shouldn't Commonmarker be added to spec/templates/helpers/html_helper_spec.rb as well?

@lsegal what is blocking here?

[lsegal]

This actually looks good but adding a test would be helpful.

[gjtorikian]

What sort of test would you like to see? I can’t install both versions of the gem in CI, can I?

Note also that the old test suite with the previous gem version still passes.

[lsegal]

@gjtorikian Actually, the tests are being skipped because the suite isn't configured to work with 1.0+.

https://github.com/lsegal/yard/actions/runs/14940811833/job/41977521502?pr=1601

This would need to be corrected. Ideally we're running both versions but I think this is a ruby limitation we would be unlikely to get past so it's not a hard requirement.

[lsegal]

On second thought it could be possible to activate one version of the library from a gem and the other from a GitHub repo. Since the namespaces don't conflict this could be an approach to test both versions and should probably be the preferred approach.

[gjtorikian]

Ok. I’m on vacation currently and can look again when I’m back in June.

[hakanensari]

Not sure if it’s overkill, but you can consider using the Appraisal gem for this.

[gjtorikian]

It turns out I don't really have time during the summer to work on adding tests for this—sorry, life stuff.

It appears to me that tests haven't ever been running for Commonmark (recent test build showing Missing markup renderer markdown) and I don't know why that is.