Commonmarker 1.0 support by ParadoxV5 · Pull Request #1540 · lsegal/yard

ParadoxV5 · 2024-03-23T02:01:29Z

Description

Add Commonmarker (all lower-case ms) provider for commonmarker ~> 1.0.0
- Fixes Commonmarker 1.0 support #1528
Remove tests for Markdown header IDs or the lack of, considering they may improve with newer provider versions and that YARD shouldn’t constraint their growth
Also change “Missing markup renderer” from skip to raise to catch this sort of issues in the future

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).

Fixes lsegal#1528 Co-Authored-By: Andrew Haines <andrew@haines.org.nz>

Preference for the provider’s highlighting is outside of this PR’s scope.

ParadoxV5 · 2024-03-23T02:02:58Z

                             :no_intraemphasis).to_html
+        when 'Commonmarker'
+          # GFM configs are on by default; use YARD for syntax highlighting
+          Commonmarker.to_html(text, plugins: {syntax_highlighter: nil})


Suggested change

Commonmarker.to_html(text, plugins: {syntax_highlighter: nil})

Commonmarker.to_html(text)

However, preference for the provider’s highlighting is outside of this PR’s scope.

Testing providers’ implementation is outside of the scope of YARD. I let the backslash line break difference live since that’s probably provider-specific.

lsegal

There's an issue in supporting both CommonMarker and Commonmarker in this PR. At first glance it seems like YARD is no longer able to support the pre 1.0 commonmarker lib due to a change in the markup_helper lookup table. If we can address that, I think this mostly looks good.

lsegal · 2024-08-26T07:07:46Z

  before(:each) do
    if html_renderer.markup_class(markup).nil?
-      skip "Missing markup renderer #{markup}"
+      raise "Missing markup renderer #{markup}"


This should stay as skip for systems that do not have the dependencies installed (testing outside of bundle exec).

I am unable to run rake/rspec locally without a prior bundle install.

Could not find redcarpet-3.6.0 in locally installed gems Run `bundle install` to install missing gems. While loading spec_helper an `exit` / `raise SystemExit` occurred, RSpec will now quit. Failure/Error: require 'bundler/setup' SystemExit: exit # ./spec/spec_helper.rb:10:in `<top (required)>' # ------------------ # --- Caused by: --- # Bundler::GemNotFound: # Could not find redcarpet-3.6.0 in locally installed gems # ./spec/spec_helper.rb:10:in `<top (required)>'

yard/spec/spec_helper.rb

Lines 9 to 13 in ad2c1c4

begin

require 'bundler/setup'

rescue LoadError

nil # noop

end

lsegal · 2024-08-26T07:13:34Z

          {:lib => :maruku, :const => 'Maruku'},
          {:lib => :'rpeg-markdown', :const => 'PEGMarkdown'},
          {:lib => :rdoc, :const => 'YARD::Templates::Helpers::Markup::RDocMarkdown'},
+          {:lib => :commonmarker, :const => 'Commonmarker'},


This is problematic, as the lib value is a unique key passed via -m. This will effectively mean that the CommonMarker (pre 1.0?) class could never be activated. We should definitely maintain support for the old format, which would require this commonmarker key be changed-- it's not idela, but maybe :commonmarker1?

CommonMarker seems still accessible.

yard/lib/yard/templates/helpers/markup_helper.rb

Lines 91 to 110 in ad2c1c4

providers = MARKUP_PROVIDERS[type.to_sym]

return true if providers && providers.empty?

if providers && options.markup_provider

providers = providers.select {|p| p[:lib] == options.markup_provider }

end

if providers.nil? || providers.empty?

log.error "Invalid markup type '#{type}' or markup provider " \

"(#{options.markup_provider}) is not registered."

return false

end

# Search for provider, return the library class name as const if found

providers.each do |provider|

begin require provider[:lib].to_s; rescue LoadError; next end if provider[:lib]

begin klass = eval("::" + provider[:const]); rescue NameError; next end # rubocop:disable Lint/Eval

MarkupHelper.markup_cache[type][:provider] = provider[:lib] # Cache the provider

MarkupHelper.markup_cache[type][:class] = klass

return true

end

Even if it’s not, v1.0 CommonMaker should have priority over pre-1.0 Commonmarker.
Separating a commonmarker1 key keeps commonmarker out of date.
YARD does not have a system that checks provider versions and has long¹ exposed users to Commonmarker 1.0’s backward incompatibility. We should improve this compatibility rather than make users work an extra step.

Footnotes

Commonmarker 1.0 released last Christmas Eve! ↩

Even if it’s not, v1.0 CommonMaker should have priority over pre-1.0 Commonmarker.

Unfortunately this would be a breaking change and would need a major version bump-- we're very unlikely to major version bump YARD just for a single (optional) markup library.

YARD is currently still in 0.x versions, so I’m not sure what you meant by ‘major’.

That said, together with #1540 (comment), I see your concern.
YARD currently has designs tailored for CommonMarker v0.x but it’s not documented anywhere.
We can only treat Commonmarker v1.x as a separate provider.

YARD is currently still in 0.x versions, so I’m not sure what you meant by ‘major’.

YARD's 0.x releases are a major version. Believe it or not, there was a time before "SemVer", and YARD predates the existence of the now-popular semantic versioning specification, along with its arbitrary distinction that only 1.0+ are considered API stable. YARD's 0.x releases are considered a stable API. "0" is the current major version for YARD, we simply use 0-based indexing.

YARD does not have a system that checks provider versions

We might wanna add one. But for now, …

YARD currently has designs tailored for CommonMarker v0.x but it’s not documented anywhere.

… I’ll add a comment in the code since it’s now linked in the README.

Prerequisite: documentation about markdown providers #1544

At first glance it seems like YARD is no longer able to support the pre 1.0 commonmarker lib due to a change in the markup_helper lookup table.

Actually, we might not be able to support both Commonmarker and CommomMarker at all.
Commonmarker 1.x shadows CommonMarker 0.x. We’d have to write individual Bundler.requires in our specs.

lsegal · 2024-08-26T07:14:56Z


    include_examples 'shared examples for markdown processors'

-    it 'generates level 2 header without id' do


This should remain but should be scoped to CommonMarker

Why? Does it have an impact?

Testing providers’ implementation is outside of the scope of YARD.

Remove tests for Markdown header IDs or the lack of, considering they may improve with newer provider versions and that YARD shouldn’t constraint their growth

Testing providers’ implementation is outside of the scope of YARD.

Not when we're explicitly testing system integration. The fact that this behavior changes is exactly why we have tests. An id-less h2 heading actually has implications for downstream YARD code, since we have special code paths for heading id attributes.

Actually, expanding this comment outward, it seems like this change removes all integration tests for CommonMarker, which means we're now only testing Commonmarker 1.0. This is also insufficient. We should keep the old tests for CommonMarker and add the new ones for this 1.0 version, not replace.

tl;dr we need to test both CommonMarker / Commonmarker implementations. Not just one. You can remove the h2 test for Commonmarker 1.0 if you want, but both libraries should be integration tested.

Remove tests for Markdown header IDs or the lack of, considering they may improve with newer provider versions and that YARD shouldn’t constraint their growth

YARD relies on this IDs though, that's the issue, and that's why we test.

noraj · 2025-01-25T22:56:23Z

I'm not sure if I understood all that was said here about the technical incompatibilities causes.

Why support several versions of commonmarker, why isn't yard supporting only the last version for each markup? Yes that's a potential breaking (of an optional feature, not of yard core), introducing new major version, and so what? People that want to keep older versions of the markup can still use older version of yard. Else, yard will still always be locked using outdated versions of markups. That would also be less cumbersome and maintenance work than having a system for version requirements and maintaining several plugins for each major version of each markup. If only one version of commonmarker should be supported, it should be the latest.

lsegal · 2026-04-11T20:05:56Z

Closing as stale. If we want to improve support for CommonMarker it has to be done in a backward compatible way. An easy way to do this would be to introduce a back compat layer that can handle either version. Unfortunately yard can't break behavior with old gem versions.

noraj · 2026-04-12T14:31:25Z

#1663 (comment)

Feedback was already provided on #1540 that was never addressed. That PR should probably be closed as stale. It would be easy to open a PR that addresses the feedback by creating a proper compatibility layer and does not break backward compatibility. You're free to open such a PR.

I'd be glad to open such a PR. But I fear what seems easy for you, is not for me. I'm not sure I understood why this PR can't be merged.

In this comment https://github.com/lsegal/yard/pull/1540/changes#r1730792625, you said the new code is problematic because :lib should be a unique value.

-{:lib => :commonmarker, :const => 'Commonmarker'},
+{:lib => :commonmarker1, :const => 'Commonmarker'},
{:lib => :commonmarker, :const => 'CommonMarker'}

So using :commonmarker1 for CM 1.x / 2.x would be enough?

For the spec, you said it should stay as skip but paradox say it still requires the gem? You have not yet answered to that https://github.com/lsegal/yard/pull/1540/changes#r1730786139.

https://github.com/lsegal/yard/pull/1540/changes#r1730794283 For the tests, what should we do? Let CommonMarker tests as it and duplicate them for Commonmarker?

Is there anything else needed? You were talking about a compatibility layer? Do you meant that instead of having :commonmarker and :commonmarker1 we should only have :commonmarker and have :const => commonmarkerDectection where commonmarkerDectection would be a method returning Commonmarker or CommonMarker based on the installed version installed?

noraj · 2026-04-12T14:33:10Z

Please tell me what to do before I spend time working on a PR.

ParadoxV5 · 2026-04-12T19:47:20Z

Hi @noraj, there’s another PR from the maintainer of Commonmarker themself, #1601.

ParadoxV5 and others added 3 commits March 22, 2024 17:22

Surface missing/incompatible markups rather than skip

2c0bac2

Support Commonmarker (lowercase m) from v1.x

3472342

Fixes lsegal#1528 Co-Authored-By: Andrew Haines <andrew@haines.org.nz>

Disable Commonmarker syntax highlighting for compatibility

0e274a0

Preference for the provider’s highlighting is outside of this PR’s scope.

ParadoxV5 commented Mar 23, 2024

View reviewed changes

Remove tests for Markdown header IDs or the lack of

9e869c9

Testing providers’ implementation is outside of the scope of YARD. I let the backslash line break difference live since that’s probably provider-specific.

ParadoxV5 marked this pull request as ready for review March 23, 2024 18:34

ParadoxV5 added a commit to ParadoxV5/template-ruby-gem that referenced this pull request Mar 23, 2024

Use my tag for lsegal/yard#1540

a89e3eb

lsegal requested changes Aug 26, 2024

View reviewed changes

ParadoxV5 marked this pull request as draft August 26, 2024 22:53

noraj mentioned this pull request Jan 26, 2025

Compatibility with yard broken since commonmarker 1.x gjtorikian/commonmarker#348

Closed

gjtorikian mentioned this pull request Jan 27, 2025

Support both Commonmarker pre and post 1.0 #1601

Closed

4 tasks

KitaitiMakoto mentioned this pull request Apr 10, 2026

Fix documentation generation task ruby-gnome/ruby-gnome#1706

Merged

noraj mentioned this pull request Apr 11, 2026

Gracefully handle missing irb/notifier #1663

Merged

lsegal closed this Apr 11, 2026

github-project-automation Bot moved this from 🔜 To Do to 💯 Done in General Projects Apr 11, 2026

	Commonmarker.to_html(text, plugins: {syntax_highlighter: nil})
	Commonmarker.to_html(text)

	providers = MARKUP_PROVIDERS[type.to_sym]
	return true if providers && providers.empty?
	if providers && options.markup_provider
	providers = providers.select {\|p\| p[:lib] == options.markup_provider }
	end

	if providers.nil? \|\| providers.empty?
	log.error "Invalid markup type '#{type}' or markup provider " \
	"(#{options.markup_provider}) is not registered."
	return false
	end

	# Search for provider, return the library class name as const if found
	providers.each do \|provider\|
	begin require provider[:lib].to_s; rescue LoadError; next end if provider[:lib]
	begin klass = eval("::" + provider[:const]); rescue NameError; next end # rubocop:disable Lint/Eval
	MarkupHelper.markup_cache[type][:provider] = provider[:lib] # Cache the provider
	MarkupHelper.markup_cache[type][:class] = klass
	return true
	end


		include_examples 'shared examples for markdown processors'

		it 'generates level 2 header without id' do

Uh oh!

Conversation

ParadoxV5 commented Mar 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Completed Tasks

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lsegal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ParadoxV5 Aug 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

noraj commented Jan 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lsegal commented Apr 11, 2026

Uh oh!

noraj commented Apr 12, 2026

Uh oh!

noraj commented Apr 12, 2026

Uh oh!

ParadoxV5 commented Apr 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ParadoxV5 commented Mar 23, 2024 •

edited

Loading

ParadoxV5 Aug 26, 2024 •

edited

Loading

noraj commented Jan 25, 2025 •

edited

Loading