Skip to content

fix: preserve preformatted help descriptions - #1214

Open
efegokdemir wants to merge 3 commits into
apache:masterfrom
efegokdemir:codex/issue-845-preformatted-help
Open

efegokdemir wants to merge 3 commits into
apache:masterfrom
efegokdemir:codex/issue-845-preformatted-help

Conversation

@efegokdemir

Copy link
Copy Markdown

Summary

Fixes #845 (MPLUGIN-535) by preserving whitespace in JavaDoc <pre> blocks when generating plain-text plugin help output. Previously, code examples were flattened and their indentation was lost.

Codex assisted with repository analysis, implementation, and test preparation. The change has been reviewed locally, and all changed lines and validation results are disclosed here for human maintainer review.

Changes

  • Preserve raw text and line structure while converting <pre> content to plain text.
  • Keep the existing whitespace normalization for ordinary HTML descriptions.
  • Preserve leading spaces in generated help lines instead of treating them as word separators.
  • Add converter and help-basic-jdk11 regression coverage for a multi-line XML example.

Testing

  • mvn -pl maven-plugin-tools-generators -am -Dspotless.check.skip=true -Dspotless.apply.skip=true test -q ✅
  • mvn -pl maven-plugin-plugin -am -DskipTests -DskipITs -Dspotless.check.skip=true -Dspotless.apply.skip=true install -q ✅
  • mvn -pl maven-plugin-plugin -Prun-its -Dinvoker.test=help-basic-jdk11 -Dspotless.check.skip=true -Dspotless.apply.skip=true integration-test -q ⚠️ the fixture build succeeds, but the post-build Groovy hook cannot run with the local JDK 27 because the bundled Groovy runtime does not support class-file major version 71
  • Equivalent help-basic-jdk11 output comparison from the generated help.log against expected-help.txt ✅
  • git diff --check ✅
  • Spotless check ⚠️ local environment incompatibility: palantir-java-format fails with NoSuchFieldError against the installed JDK 27 compiler API

Notes

The production fix is shared by both legacy and Maven 4 help templates. The generated fixture output was verified independently because the repository's Groovy invoker hook is not compatible with the installed JDK 27. The commit is cryptographically signed and includes the required Signed-off-by trailer.

@gnodet-bot gnodet-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct fix for MPLUGIN-535 — the sentinel-char approach, the preformatted counter, and the whitespace-preservation logic all look sound. Two items worth cleaning up before merge.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Comment on lines +95 to +103
if (name.equals("pre")) {
preformatted--;
preformattedEndsWithNewline = accum.length() > 0 && accum.charAt(accum.length() - 1) == '\n';
accum.append(PRE_END);
}
if (name.equals("pre")) {
if (!preformattedEndsWithNewline) {
accum.append('\n');
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Redundant second if (name.equals("pre")) block — the two consecutive if blocks can be merged. After the first block appends PRE_END, name is still "pre", so the second check is always true when the first was true. Merge them into a single block:

Suggested change
if (name.equals("pre")) {
preformatted--;
preformattedEndsWithNewline = accum.length() > 0 && accum.charAt(accum.length() - 1) == '\n';
accum.append(PRE_END);
}
if (name.equals("pre")) {
if (!preformattedEndsWithNewline) {
accum.append('\n');
}
if (name.equals("pre")) {
preformatted--;
preformattedEndsWithNewline = accum.length() > 0 && accum.charAt(accum.length() - 1) == '\n';
accum.append(PRE_END);
if (!preformattedEndsWithNewline) {
accum.append('\n');
}
```

Comment on lines 86 to +93
}

@Test
void preformattedTextPreservesWhitespace() {
String test =
"For example:<pre>\n &lt;binary&gt;\n &lt;url&gt;https://example.test/plugin.exe&lt;/url&gt;\n &lt;/binary&gt;\n</pre>";
assertEquals(
"For example:\n <binary>\n <url>https://example.test/plugin.exe</url>\n </binary>\n",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Consider adding edge-case tests for robustness:

  1. <pre> content that does not end with a newline (tests the preformattedEndsWithNewline branch)
  2. Multiple <pre> blocks in the same input
  3. Nested <pre> handling (if expected to be unsupported, a comment saying so would help)

The current test only covers the trailing-newline path.

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.

[MPLUGIN-535] maven-plugin-plugin clobbers code block formatting

2 participants