8354320: Changes to jpackage.md cause pandoc warning

[alexeysemenyukoracle]

Escape the dollar sign.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8354320: Changes to jpackage.md cause pandoc warning (Bug - P3)

Reviewers

• Alan Bateman (@AlanBateman - Reviewer) Review applies to 6709a723
• Alexander Matveev (@sashamatveev - Reviewer)
• Magnus Ihse Bursie (@magicus - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/24585/head:pull/24585
$ git checkout pull/24585

Update a local copy of the PR:
$ git checkout pull/24585
$ git pull https://git.openjdk.org/jdk.git pull/24585/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 24585

View PR using the GUI difftool:
$ git pr show -t 24585

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/24585.diff

Using Webrev

Link to Webrev Comment

[alexeysemenyukoracle]

@sashamatveev PTAL

[bridgekeeper]

👋 Welcome back asemenyuk! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@alexeysemenyukoracle This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8354320: Changes to jpackage.md cause pandoc warning

Reviewed-by: almatvee, ihse, alanb

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 27 new commits pushed to the master branch:

• e8bcedb: 8354452: Shenandoah: Enforce range checks on parameters controlling heuristic sleep times
• d14e84c: 8354446: [BACKOUT] Remove friends for ObjectMonitor
• 1d7138f: 8353614: JFR: jfr print --exact
• ... and 24 more: https://git.openjdk.org/jdk/compare/60f3d607412dfe289f33dd922dfc1c9ff766810f...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@alexeysemenyukoracle The following label will be automatically applied to this pull request:

• core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[alexeysemenyukoracle]

@magicus PTAL

[mlbridge]

Webrevs

• 02: Full - Incremental (c9d11ffa)
• 01: Full - Incremental (33364c72)
• 00: Full (6709a723)

[AlanBateman]

Thanks for jumping on this, just to fix the warning until it is looked at more closely.

[magicus]

I'm not entirely sure this is a good solution. Yes, it makes pandoc happy and removes the warning, but this is using a pandoc extension and not standard markdown. A standard markdown renderer will show this as the literal \$, and not just a $ which is intended.

I will repeat my suggestion to put the special characters inside backticks. Not only will it prevent pandoc from trying to do any expansion, it will also follow a general practice of formatting code where literals the user should type is in monotype. So I think it would kill two birds with one stone.

[alexeysemenyukoracle]

"`${`"

emits a slightly different warning:

[WARNING] Could not convert TeX math ) and the first following non-alphanumeric character. Alternatively, it can be enclosed between "`, rendering as TeX:
  e enclosed between "`
                     ^
  unexpected '"' 

Similar with

`${`

the warning is:

[WARNING] Could not convert TeX math ) and the first following non-alphanumeric character. Alternatively, it can be enclosed between "`, rendering as TeX:
  e enclosed between "`
                     ^
  unexpected '"' 

It seems like it can't handle the dollar character unless it is escaped with the backslash.

Even though raw jpackage.md renders the backslash, man and HTML outputs look correct.

I'm not an expert in pandoc or markdown. If you have any other ideas about how to fix it, I'll be happy to try them out.

[magicus]

Oh, that's too bad. :( Weird that pandoc tries to modify stuff inside ``. I think the better solution then is to pass -tex_math_dollars to pandoc to stop it from trying to parse the dollar sign. We should not be using the TeX math extension anyway. I'll whip up a build patch, one sec...

[alexeysemenyukoracle]

Meanwhile, I fixed a case of missing escape backslash in the following paragraph. I verified that it is rendered properly in raw markdown and in pandoc output.

[magicus]

Try this patch:

diff --git a/make/autoconf/basic_tools.m4 b/make/autoconf/basic_tools.m4
index eac14207b1d..516529116c3 100644
--- a/make/autoconf/basic_tools.m4
+++ b/make/autoconf/basic_tools.m4
@@ -468,7 +468,14 @@ AC_DEFUN_ONCE([BASIC_SETUP_PANDOC],
     AC_MSG_CHECKING([if the pandoc smart extension needs to be disabled for markdown])
     if $PANDOC --list-extensions | $GREP -q '+smart'; then
       AC_MSG_RESULT([yes])
-      PANDOC_MARKDOWN_FLAG="markdown-smart"
+      PANDOC_MARKDOWN_FLAG="$PANDOC_MARKDOWN_FLAG-smart"
+    else
+      AC_MSG_RESULT([no])
+    fi
+    AC_MSG_CHECKING([if the pandoc tex_math_dollars extension needs to be disabled for markdown])
+    if $PANDOC --list-extensions | $GREP -q '+tex_math_dollars'; then
+      AC_MSG_RESULT([yes])
+      PANDOC_MARKDOWN_FLAG="$PANDOC_MARKDOWN_FLAG-tex_math_dollars"
     else
       AC_MSG_RESULT([no])
     fi

(and remove the \ in front of $).

[alexeysemenyukoracle]

Ironically, I came up with the same patch a few minutes ago. Even the change to PANDOC_MARKDOWN_FLAG="$PANDOC_MARKDOWN_FLAG-smart" is the same.

[alexeysemenyukoracle]

With the latest fix, the warning is gone, the raw markdown looks as expected when viewed online, and the man output looks correct.

[sashamatveev]

Looks good.

[magicus]

Maybe wait for someone else to review the build change too...

[alexeysemenyukoracle]

@erikj79 PTAL

[AlanBateman]

Thanks for getting this to the right place.