Use `pdf-engine` variable instead of extensions...

...to determine what to do about `.pdfhref` macros in `ms`
output.

When no PDF engine is specified, we don't use the `.pdfhref`
macros at all. This gives better results for links in formats
other than PDF, since the link text would simply disappear if
it exists only in a `.pdfhref` macro.

When a PDF engine is specified,  escape the argument of
`.pdfhref O` in a way that is appropriate.

Remove `groff` extension.

Text.Pandoc.Extensions: remove `Ext_groff` constructor.

See #10738.  This revises the earlier commit
3adcb4bd8089cdb8408da5f17780cd49513b7cec.

5 files changed, 51 insertions, 31 deletions

diff --git a/MANUAL.txt b/MANUAL.txt
index fce4a4a0e..8d4e556e0 100644
--- a/MANUAL.txt
+++ b/MANUAL.txt
@@ -1475,6 +1475,14 @@ header when requesting a document from a URL:
     - `-t ms`:  `pdfroff`
     - `-t typst`: `typst`
 
+    This option is normally intended to be used when a PDF
+    file is specified as `-o/--output`.  However, it may still
+    have an effect when other output formats are requested.
+    For example, `ms` output will include `.pdfhref` macros
+    only if a `--pdf-engine` is selected, and the macros will
+    be differently encoded depending on whether `groff` or
+    `pdfroff` is specified.
+
 `--pdf-engine-opt=`*STRING*
 
 :   Use the given string as a command-line argument to the `pdf-engine`.
@@ -3446,6 +3454,10 @@ on the output format, and include the following:
 
     If you need absolute paths, use e.g. `$curdir$/$sourcefile$`.
 
+`pdf-engine`
+:   name of PDF engine if provided using `--pdf-engine`, or the
+    default engine for the format if PDF output is requested.
+
 `curdir`
 :   working directory from which pandoc is run.
 
@@ -3857,13 +3869,6 @@ additional markers for paragraphs and alternative markup for
 emphasized text. The `emphasis-command` template variable is set
 if the extension is enabled.
 
-### Extension: `groff` ###
-
-Enabling this extension with `ms` output will cause the `.pdfhref O`
-macros to be formatted in the way `groff -Tpdf` likes, instead of
-the way `pdfroff` likes. This extension is enabled automatically
-when `groff` is used as a `--pdf-engine`.
-
 # Pandoc's Markdown
 
 Pandoc understands an extended and slightly revised version of
diff --git a/src/Text/Pandoc/Extensions.hs b/src/Text/Pandoc/Extensions.hs
index cc1afd26c..7f1b35c17 100644
--- a/src/Text/Pandoc/Extensions.hs
+++ b/src/Text/Pandoc/Extensions.hs
@@ -144,7 +144,6 @@ data Extension =
     | Ext_xrefs_name          -- ^ Use xrefs with names
     | Ext_xrefs_number        -- ^ Use xrefs with numbers
     | Ext_yaml_metadata_block -- ^ YAML metadata block
-    | Ext_groff               -- ^ Use groff form for .pdfhref macro in ms
     | CustomExtension T.Text  -- ^ Custom extension
     deriving (Show, Read, Eq, Ord, Data, Typeable, Generic)
 
@@ -660,5 +659,4 @@ getAllExtensions f = universalExtensions <> getAll f
     [ Ext_smart ]
   getAll "typst"           = extensionsFromList [Ext_citations, Ext_smart]
   getAll "djot"            = extensionsFromList [Ext_sourcepos]
-  getAll "ms"              = extensionsFromList [Ext_groff]
   getAll _                 = mempty
diff --git a/src/Text/Pandoc/PDF.hs b/src/Text/Pandoc/PDF.hs
index ae6de706b..e3157ce96 100644
--- a/src/Text/Pandoc/PDF.hs
+++ b/src/Text/Pandoc/PDF.hs
@@ -45,8 +45,7 @@ import Text.Pandoc.Definition
 import Text.Pandoc.Error (PandocError (PandocPDFProgramNotFoundError))
 import Text.Pandoc.MIME (getMimeType)
 import Text.Pandoc.Options (HTMLMathMethod (..), WriterOptions (..))
-import Text.Pandoc.Extensions (enableExtension, disableExtension,
-                               Extension(Ext_smart, Ext_groff))
+import Text.Pandoc.Extensions (disableExtension, Extension(Ext_smart))
 import Text.Pandoc.Process (pipeProcess)
 import System.Process (readProcessWithExitCode)
 import Text.Pandoc.Shared (inDirectory, stringify, tshow)
@@ -113,9 +112,7 @@ makePDF program pdfargs writer opts doc =
                     paperargs ++ pdfargs
       generic2pdf program args source
     "groff" -> do
-      source <- writer opts{ writerExtensions =
-                              enableExtension Ext_groff
-                               (writerExtensions opts) } doc
+      source <- writer opts doc
       let paperargs =
             case lookupContext "papersize" (writerVariables opts) of
               Just s
diff --git a/src/Text/Pandoc/Writers/Ms.hs b/src/Text/Pandoc/Writers/Ms.hs
index 33dc658c5..6fca72dde 100644
--- a/src/Text/Pandoc/Writers/Ms.hs
+++ b/src/Text/Pandoc/Writers/Ms.hs
@@ -28,7 +28,7 @@ import Data.Char (isAscii, isLower, isUpper, ord)
 import Data.List (intercalate, intersperse)
 import Data.List.NonEmpty (nonEmpty)
 import qualified Data.Map as Map
-import Data.Maybe (catMaybes)
+import Data.Maybe (catMaybes, isNothing)
 import Data.Text (Text)
 import qualified Data.Text as T
 import Network.URI (escapeURIString, isAllowedInURI)
@@ -42,6 +42,7 @@ import Text.Pandoc.ImageSize
 import Text.Pandoc.Logging
 import Text.Pandoc.Options
 import Text.DocLayout hiding (Color)
+import Text.DocTemplates (lookupContext)
 import Text.Pandoc.Shared
 import Text.Pandoc.Templates (renderTemplate)
 import Text.Pandoc.Writers.Math
@@ -127,12 +128,15 @@ toSmallCaps opts s = case T.uncons s of
 -- line.  roff treats the line-ending period differently.
 -- See http://code.google.com/p/pandoc/issues/detail?id=148.
 
+getPdfEngine :: WriterOptions -> Maybe Text
+getPdfEngine opts = lookupContext "pdf-engine" (writerVariables opts)
+
 blockToMs :: PandocMonad m
           => WriterOptions -- ^ Options
           -> Block         -- ^ Block element
           -> MS m (Doc Text)
 blockToMs opts (Div (ident,cls,kvs) bs) = do
-  let anchor = if T.null ident
+  let anchor = if isNothing (getPdfEngine opts) || T.null ident
                   then empty
                   else nowrap $
                          literal ".pdfhref M "
@@ -189,6 +193,7 @@ blockToMs opts (Header level (ident,classes,_) inlines) = do
                               "unnumbered" `notElem` classes
                              then (".NH", "\\*[SN]")
                              else (".SH", "")
+  let mbPdfEngine = getPdfEngine opts
   let anchor = if T.null ident
                   then empty
                   else nowrap $
@@ -199,9 +204,9 @@ blockToMs opts (Header level (ident,classes,_) inlines) = do
                                       (if T.null secnum
                                           then ""
                                           else "  ") <>
-                                      (if isEnabled Ext_groff opts
-                                          then id
-                                          else escapePDFString)
+                                      (case mbPdfEngine of
+                                         Just "groff" -> id
+                                         _ -> escapePDFString)
                                       plainContents)
   let backlink = nowrap (literal ".pdfhref L -D " <>
        doubleQuotes (literal (toAscii ident)) <> space <> literal "\\") <> cr <>
@@ -220,9 +225,9 @@ blockToMs opts (Header level (ident,classes,_) inlines) = do
   modify $ \st -> st{ stFirstPara = True }
   return $ (literal heading <> space <> literal (tshow level)) $$
            contents $$
-           bookmark $$
-           anchor $$
-           tocEntry
+           case mbPdfEngine of
+             Nothing -> mempty
+             Just _ -> bookmark $$ anchor $$ tocEntry
 blockToMs opts (CodeBlock attr str) = do
   hlCode <- highlightCode opts attr str
   setFirstPara
@@ -484,17 +489,25 @@ inlineToMs opts Space = handleNotes opts space
 inlineToMs opts (Link _ txt (T.uncons -> Just ('#',ident), _)) = do
   -- internal link
   contents <- inlineListToMs' opts $ map breakToSpace txt
-  return $ literal "\\c" <> cr <> nowrap (literal ".pdfhref L -D " <>
-       doubleQuotes (literal (toAscii ident)) <> literal " -A " <>
-       doubleQuotes (literal "\\c") <> space <> literal "\\") <> cr <>
-       literal " -- " <> doubleQuotes (nowrap contents) <> cr <> literal "\\&"
+  return $
+    case getPdfEngine opts of
+      Just _ -> literal "\\c" <> cr <> nowrap (literal ".pdfhref L -D " <>
+            doubleQuotes (literal (toAscii ident)) <> literal " -A " <>
+            doubleQuotes (literal "\\c") <> space <> literal "\\") <> cr <>
+            literal " -- " <> doubleQuotes (nowrap contents) <> cr <>
+            literal "\\&"
+      Nothing -> contents
 inlineToMs opts (Link _ txt (src, _)) = do
   -- external link
   contents <- inlineListToMs' opts $ map breakToSpace txt
-  return $ literal "\\c" <> cr <> nowrap (literal ".pdfhref W -D " <>
-       doubleQuotes (literal (escapeUri src)) <> literal " -A " <>
-       doubleQuotes (literal "\\c") <> space <> literal "\\") <> cr <>
-       literal " -- " <> doubleQuotes (nowrap contents) <> cr <> literal "\\&"
+  return $
+    case getPdfEngine opts of
+      Just _ -> literal "\\c" <> cr <> nowrap (literal ".pdfhref W -D " <>
+            doubleQuotes (literal (escapeUri src)) <> literal " -A " <>
+            doubleQuotes (literal "\\c") <> space <> literal "\\") <> cr <>
+            literal " -- " <> doubleQuotes (nowrap contents) <> cr <>
+            literal "\\&"
+      Nothing -> contents
 inlineToMs opts (Image attr alternate (src, _)) = do
   let desc = literal "[IMAGE: " <>
              literal (escapeStr opts (stringify alternate)) <> char ']'
diff --git a/test/Tests/Old.hs b/test/Tests/Old.hs
index f8cc80625..ad642b1a5 100644
--- a/test/Tests/Old.hs
+++ b/test/Tests/Old.hs
@@ -196,7 +196,14 @@ tests pandocPath =
     [ testGroup "writer" $ writerTests' "muse"
     ]
   , testGroup "ms"
-    [ testGroup "writer" $ writerTests' "ms"
+    [ test' "basic"  ["-f", "native", "-t", "ms", "--columns=80",
+                      "--variable", "pandoc-version=",
+                      "--pdf-engine", "pdfroff", "-s"]
+      "testsuite.native" "writer.ms"
+    , test' "tables" ["-f", "native", "-t", "ms", "--columns=80",
+                      "--variable", "pandoc-version=",
+                      "--pdf-engine", "pdfroff"]
+      "tables.native"  "tables.ms"
     ]
   , testGroup "typst"
     [ testGroup "writer" $ writerTests' "typst" ++ extWriterTests' "typst"