Add GFM alerts extension by ia3andy · Pull Request #420 · commonmark/commonmark-java

ia3andy · 2026-03-03T16:05:42Z

Note

I have used Claude to code most of it, I am currently reviewing the generated code which seems to be overall valid. I've requested to follow the pattern of existing extensions (and compare to other implementations of this feature).

Summary

Adds new extension module for GitHub Flavored Markdown alerts
Implements alert detection as a PostProcessor (transforms BlockQuote nodes into Alert nodes after parsing)
Includes HTML and Markdown renderers
Supports all five standard GFM alert types: NOTE, TIP, IMPORTANT, WARNING, CAUTION
Case-insensitive type matching ([!note], [!Note] work like [!NOTE])
Supports custom alert types and title overrides via builder API (e.g., for localization)
Empty/whitespace-only alerts render as normal blockquotes (matching GitHub behavior)
Alerts only at document level (not inside list items or nested blockquotes, matching GitHub)

Test approach

AlertsSpecTest — parameterized spec-based tests from alerts-spec.txt, expectations verified against GitHub Markdown API (gh api markdown -f mode=gfm)
AlertsTest — custom types, builder validation, AST assertions
AlertsMarkdownRendererTest — roundtrip rendering tests

Changes from review

Rewrote from custom BlockParser to PostProcessor approach
Added case-insensitive type matching
Allow overriding standard type titles
Removed unnecessary TextContentRenderer (core handles it)
Constructor injection for Alert (immutable type, no public setter)
Moved STANDARD_TYPES from Alert node to AlertsExtension
Removed getCustomTypes() config leak from public API
Fixed visitor traversal (early return after successful conversion)
Removed dead code (capitalize fallback)

Closes #327

ia3andy · 2026-03-03T16:38:11Z

I've been through the code which seems good to me so I can say I am taking responsibilities for it like my own. Still, another set of eyes would be good :)

ia3andy · 2026-03-21T07:49:43Z

@robinst any thought ?

robinst

Some first comments, mostly around parsing and edge cases.

I was curious how GitHub implements this in cmark-gfm, and the answer seems to be that they don't:

github/cmark-gfm#350 (comment)

Alerts are not implemented in this project. They don't relate to their parser. They don't relate to syntax. So they're not here.

They are implemented as a post processing step on a sort of "DOM" version of the resulting document.

Can you explore what the implementation would look like as a PostProcessor instead? That has the advantage of not having to copy all the parsing logic from block quotes. (From the edge cases with nesting I commented about, it seems like they only look at top-level nodes.)

...rk-ext-gfm-alerts/src/main/java/org/commonmark/ext/gfm/alerts/internal/AlertBlockParser.java

commonmark-ext-gfm-alerts/src/main/java/org/commonmark/ext/gfm/alerts/AlertsExtension.java

commonmark-ext-gfm-alerts/src/test/java/org/commonmark/ext/gfm/alerts/AlertsEdgeCasesTest.java

...k-ext-gfm-alerts/src/test/java/org/commonmark/ext/gfm/alerts/AlertsMarkdownRendererTest.java

ia3andy · 2026-03-25T10:11:12Z

Thanks for the review! That all makes sense, will have a look soonish :)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

ia3andy · 2026-03-27T16:15:23Z

I made all the changes, I am currently generating a small jbang script to generate the spec from gh api result, not needed but cool :)

Add a jbang script (generate-alerts-spec.java) that generates alerts-spec.txt from a template (alerts-spec-template.md) by rendering each example through the GitHub Markdown API and normalizing the HTML. Configure AlertsSpecTest with softbreak("<br>") to match GitHub's rendering, reducing the normalizations needed in the generator. Co-Authored-By: Claude Opus 4.6 <[email protected]>

ia3andy · 2026-03-27T16:47:38Z

@robinst it is ready for another review thanks!

robinst

More feedback. Getting closer, but I think we can still simplify.

Neat idea to generate the test data by using the gh CLI.

robinst · 2026-03-28T00:30:29Z

...t-gfm-alerts/src/main/java/org/commonmark/ext/gfm/alerts/internal/AlertHtmlNodeRenderer.java

+        if (customTypeTitles.containsKey(type)) {
+            return customTypeTitles.get(type);


Suggested change

if (customTypeTitles.containsKey(type)) {

return customTypeTitles.get(type);

var customTypeTitle = customTypeTitles.get(type);

if (customTypeTitle != null) {

return customTypeTitle;

}

robinst · 2026-03-28T00:31:42Z

...k-ext-gfm-alerts/src/main/java/org/commonmark/ext/gfm/alerts/internal/AlertNodeRenderer.java

+public abstract class AlertNodeRenderer implements NodeRenderer {
+
+    @Override
+    public Set<Class<? extends org.commonmark.node.Node>> getNodeTypes() {


Add an import please.

robinst · 2026-03-28T00:33:23Z

...-ext-gfm-alerts/src/main/java/org/commonmark/ext/gfm/alerts/internal/AlertPostProcessor.java

+        public void visit(BlockQuote blockQuote) {
+            // Only convert top-level block quotes (direct children of Document).
+            // This matches GitHub's behavior where alerts are only detected at the document level.
+            if (blockQuote.getParent() instanceof Document) {


If you do it this way, you're still visiting the full document tree, which is unnecessary.

Just manually iterate over the direct children of Document instead of using an AbstractVisitor please.

robinst · 2026-03-28T00:33:58Z

...-ext-gfm-alerts/src/main/java/org/commonmark/ext/gfm/alerts/internal/AlertPostProcessor.java

+        }
+
+        private boolean tryConvertToAlert(BlockQuote blockQuote) {
+            Node firstChild = blockQuote.getFirstChild();


nit: Use var here and in other places.

Suggested change

Node firstChild = blockQuote.getFirstChild();

var firstChild = blockQuote.getFirstChild();

robinst · 2026-03-28T00:37:40Z

...-ext-gfm-alerts/src/main/java/org/commonmark/ext/gfm/alerts/internal/AlertPostProcessor.java

+            String markerText;
+            Node afterMarker = firstInline.getNext();
+            if (afterMarker instanceof SoftLineBreak || afterMarker instanceof HardLineBreak || afterMarker == null) {
+                markerText = literal;


Why is this assignment in here? And having literal as a variable seems unnecessary?

robinst · 2026-03-28T00:50:16Z

...-ext-gfm-alerts/src/main/java/org/commonmark/ext/gfm/alerts/internal/AlertPostProcessor.java

+                    if (!((Text) next).getLiteral().trim().isEmpty()) {
+                        return false;
+                    }
+                } else if (!(next instanceof SoftLineBreak) && !(next instanceof HardLineBreak)) {


Apart from after the marker, I don't think we can have a soft/hard line break inside a block quote if it's completely empty.

In fact, I don't think we need the logic below either. Or can you show me a test case that breaks if we just check if there's another node present after the marker/line break?

robinst · 2026-03-28T00:51:33Z

commonmark-ext-gfm-alerts/src/main/java/org/commonmark/ext/gfm/alerts/AlertsExtension.java

+    private final Map<String, String> customTypes;
+
+    private AlertsExtension(Builder builder) {
+        this.customTypes = new LinkedHashMap<>(builder.customTypes);


AFAICT, no need for a LinkedHashMap here, just a normal HashMap is fine.

robinst · 2026-03-28T00:54:29Z

commonmark-ext-gfm-alerts/src/main/java/org/commonmark/ext/gfm/alerts/AlertsExtension.java

+            if (title == null || title.isEmpty()) {
+                throw new IllegalArgumentException("Title must not be null or empty");
+            }
+            if (!type.equals(type.toUpperCase())) {


Suggested change

if (!type.equals(type.toUpperCase())) {

if (!type.equals(type.toUpperCase(Locale.ROOT))) {

(Otherwise, e.g. in a Turkish locale i will uppercase to İ, which is probably unexpected.)

robinst · 2026-03-28T00:57:30Z

commonmark-ext-gfm-alerts/README.md

+        .build();
+```
+
+Custom types must be UPPERCASE and cannot override standard types.


Outdated now.

robinst · 2026-03-28T00:58:45Z

commonmark-ext-gfm-alerts/src/main/java/org/commonmark/ext/gfm/alerts/AlertsExtension.java

+     * Builder for configuring the alerts extension.
+     */
+    public static class Builder {
+        private final Map<String, String> customTypes = new LinkedHashMap<>();


Same here, no need for a LinkedHashMap

ia3andy marked this pull request as draft March 3, 2026 16:05

ia3andy force-pushed the gfm-alerts branch from 1b8aef6 to 48bb560 Compare March 3, 2026 16:22

ia3andy marked this pull request as ready for review March 3, 2026 16:37

ia3andy mentioned this pull request Mar 3, 2026

Feature request: Admonition blocks quarkiverse/quarkus-roq#406

Open

robinst requested changes Mar 25, 2026

View reviewed changes

Add GFM alerts extension

057f13d

Co-Authored-By: Claude Opus 4.6 <[email protected]>

ia3andy force-pushed the gfm-alerts branch from 48bb560 to 057f13d Compare March 27, 2026 16:01

robinst reviewed Mar 28, 2026

View reviewed changes

		if (customTypeTitles.containsKey(type)) {
		return customTypeTitles.get(type);

-        if (customTypeTitles.containsKey(type)) {
-            return customTypeTitles.get(type);
+        var customTypeTitle = customTypeTitles.get(type);
+        if (customTypeTitle != null) {
+          return customTypeTitle;
+        }

	Node firstChild = blockQuote.getFirstChild();
	var firstChild = blockQuote.getFirstChild();

	if (!type.equals(type.toUpperCase())) {
	if (!type.equals(type.toUpperCase(Locale.ROOT))) {

Conversation

ia3andy commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Note

Summary

Test approach

Changes from review

Uh oh!

ia3andy commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ia3andy commented Mar 21, 2026

Uh oh!

robinst left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ia3andy commented Mar 25, 2026

Uh oh!

ia3andy commented Mar 27, 2026

Uh oh!

ia3andy commented Mar 27, 2026

Uh oh!

robinst left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ia3andy commented Mar 3, 2026 •

edited

Loading

ia3andy commented Mar 3, 2026 •

edited

Loading