Re: PDF outline anomaly

[G. Branden Robinson]

Hi Peter,

The news gets worse.

At 2024-04-29T21:41:06-0400, Peter Schaffter wrote:
> Branden --
> 
> On Mon, Apr 29, 2024, G. Branden Robinson wrote:
> > I have bad news.  You might not be on the true bleeding edge of Git.
> 
> It would appear so.  I haven't done a fresh build/install for a
> while.
> 
> > Not only can I reproduce this, the problem is _worse_ now.
> 
> Oh, dear.
> 
> > I'll do another bisect to see where the 'u]' started creeping in,
> > but it looks to me like there's more than one problem to solve.
> 
> I rather suspected it might be the tip of an iceberg.

I was prepared to fall on my sword over this--and there still may be an
opportunity for that--but this is the commit that introduced the
undesired 'u]' text into the PDF outline.

2387949ffc11b753dd91480dcb85be6c22819790 is the first bad commit
commit 2387949ffc11b753dd91480dcb85be6c22819790
Author: Deri James <deri@chuzzlewit.myzen.co.uk>
Date:   Fri Apr 12 17:10:49 2024 +0100

    Correct linear search introduced in commit cd9fde325f

    * tmac/pdf.tmac: Various problems have been rectified, speed
    improvements to the linear search, Keith protection against
    illegal characters being used in string identifiers.

    * contrib/mom/om.tmac: Switch to using Branden's linear search
    so that we don't have to maintain two different methods.

    Fixes <https://savannah.gnu.org/bugs/?65585>

 ChangeLog           | 31 ++++++++++++++++++++++
 contrib/mom/om.tmac |  5 ++--
 tmac/pdf.tmac       | 76 +++++++++++++++++++++++------------------------------
 3 files changed, 67 insertions(+), 45 deletions(-)

This commit could probably use another set of eyeballs.

Here are the changes to om.tmac (simple, straightforward IMO) and
pdf.tmac--more complex.

diff --git a/contrib/mom/om.tmac b/contrib/mom/om.tmac
index 299cbc949..c7d20272e 100644
--- a/contrib/mom/om.tmac
+++ b/contrib/mom/om.tmac
@@ -23631,8 +23631,9 @@ .             defcolor pdf:href.colour rgb \\$1
 .    if '\\*[PDF_AST]'*' \{\
 .        chop PDF_TXT
 .        ie '\\*[.T]'pdf' \{\
-.           ie d pdf:look(\\*[PDF_NM]) \
-.               as PDF_TXT 
\&\\*[PDF_AST_Q]\\*[pdf:look(\\*[PDF_NM])]\\*[PDF_AST_Q]
+.           pdf:lookup \\*[PDF_NM]
+.           ie !'\\*[pdf:lookup-result]'' \
+.               as PDF_TXT \&\\*[PDF_AST_Q]\\*[pdf:lookup-value]\\*[PDF_AST_Q]
 .           el \{\
 .               as PDF_TXT Unknown
 .               if !rPDF_UNKNOWN .tm \
diff --git a/tmac/pdf.tmac b/tmac/pdf.tmac
index 1b2415270..745d4ee37 100644
--- a/tmac/pdf.tmac
+++ b/tmac/pdf.tmac
@@ -68,6 +68,7 @@ .de pdf:SS
 .pdf:SS \[*z]
 .char \[lh] \X'pdf: xrev'\[rh]\X'pdf: xrev'
 .nr pdf:bm.nl 1
+.nr pdf:bm.nk 0
 .de pdfmark
 . nop \!x X ps:exec [\\$* pdfmark
 ..
@@ -156,7 +157,8 @@ .de pdfnote
 .\"
 .\" Parse any specified (recognisable) PDFNOTE options
 .\"
-.   while dpdf:note\\$1 \{\
+.   while \A'\\$1' \{\
+.      if !dpdf:note\\$1 .break
 .      pdf:note\\$1 \\$@
 .      shift \\n[pdf:note.argc]
 .      \}
@@ -201,9 +203,11 @@ .de pdf:note-T
 .de pdf:lookup
 .nr pdf:index 0 1
 .ds pdf:lookup-result \" empty
+.ds pdf:lookup-value  \" empty
 .while d pdf:bm\\n+[pdf:index].tag \{\
 .   if '\\$1'\\*[pdf:bm\\n[pdf:index].tag]' \{\
 .      ds pdf:lookup-result \\*[pdf:bm\\n[pdf:index].tag]\"
+.      ds pdf:lookup-value \\*[pdf:bm\\n[pdf:index].val]\"
 .      break
 .      \}
 .   \}
@@ -230,8 +234,10 @@ .de pdfbookmark
 .   \" Make the bookmark name "untagged" by default,
 .   \" then parse any specified options, to set a "tag", if required
 .   \"
+.      nr pdf:bm.nr +1
 .      ds pdf:href-T
-.      while dpdf:href.opt\\$1 \{\
+.      while \A'\\$1' \{\
+.         if !dpdf:href.opt\\$1 .break
 .         pdf:href.opt\\$1 \\$@
 .         shift \\n[pdf:href.argc]
 .         \}
@@ -267,38 +273,26 @@ .de pdfbookmark
 .   \" in order to generate a uniquely serialised bookmark name,
 .   \" ( which we return in the string "PDFBOOKMARK.NAME" ),
 .   \"
-.      nr pdf:bm.nr +1
 .      ie '\\*[pdf:href-T]'' .ds PDFBOOKMARK.NAME pdf:bm\\n[pdf:bm.nr]
 .      el .ds PDFBOOKMARK.NAME \\*[pdf:href-T]
 .      pdf:href.sety
 .      ds pdf:cleaned \\$*
-.      ev pdfcln
-.      tr \[em]-
-.      nf
-.      box pdf:clean
-.      nop \\$*
-.      fl
-.      box
-.      chop pdf:clean
-.      asciify pdf:clean
-.      ds pdf:cleaned \\*[pdf:clean]
-.      rm pdf:clean
-.      ev
-.      tr \[em]\[em]
-.      ie d PRINTSTYLE \{\
-.         ds pdf:look(\\*[PDFBOOKMARK.NAME]) \\*[pdf:cleaned]
-.         if dPDF.EXPORT .tm .ds pdf:look(\\*[PDFBOOKMARK.NAME]) 
\\*[pdf:cleaned]
+.      if !'\\*[pdf:href-T]'' \{\
+.         nr pdf:bm.nk +1
+.         if !d pdf:bm\\n[pdf:bm.nk].tag \{\
+.            ds pdf:bm\\n[pdf:bm.nk].tag \\*[PDFBOOKMARK.NAME]
+.            ds pdf:bm\\n[pdf:bm.nk].val \\*[pdf:cleaned]
 .         \}
-.      el \{\
-.         ds pdf:bm\\n[pdf:bm.nr].tag \\*[PDFBOOKMARK.NAME]
-.         if d PDF.EXPORT .tm .ds pdf:bm\\n[pdf:bm.nr].tag 
\\*[PDFBOOKMARK.NAME]
+.         if d PDF.EXPORT \{\
+.            tm .ds pdf:bm\\n[pdf:bm.nk].tag \\*[PDFBOOKMARK.NAME]
+.            tm .ds pdf:bm\\n[pdf:bm.nk].val \\*[pdf:cleaned]
 .         \}
+.      \}
 .      pdfmark /Dest /\\*[PDFBOOKMARK.NAME] /View [\\*[PDFBOOKMARK.VIEW]] /DEST
 .      nop \!x X ps:exec [/Dest /\\*[PDFBOOKMARK.NAME] /Title 
(\\*[pdf:cleaned]) /Level \\n[pdf:bm.lev] /OUT pdfmark
 .      pdf:href.options.clear
 .      rr PDFPAGE.Y
 .      rm pdf:cleaned
-.      rm pdf:clean
 .      \}
 .   \}
 .el \{\
@@ -597,7 +591,8 @@ .de pdf*href-M
 .\" options -- anything which is not recognised is assumed to start
 .\" the "descriptive text" component of the argument list).
 .\"
-.while dpdf:href.opt\\$1 \{\
+.while \A'\\$1' \{\
+.   if !dpdf:href.opt\\$1 .break
 .   pdf:href.opt\\$1 \\$@
 .   shift \\n[pdf:href.argc]
 .   \}
@@ -613,16 +608,17 @@ .de pdf*href-M
 .\" if any, and set the marker -- if we still can't identify the name
 .\" for the destination, then this marker will not be created.
 .\"
+.nr pdf:bm.nk +1
 .ds PDFBOOKMARK.NAME "\\*[pdf:href-N]\\*[pdf:href-D]
 .pdf*href.set \\*[PDFBOOKMARK.NAME] \\$1
-.ie d PRINTSTYLE \{\
-.   ds pdf:look(\\*[PDFBOOKMARK.NAME]) \\$*
-.   if dPDF.EXPORT .tm .ds pdf:look(\\*[PDFBOOKMARK.NAME]) \\$*
-.   \}
-.el \{\
-.   ds pdf:bm\\n[pdf:bm.nr].tag \\*[PDFBOOKMARK.NAME]
-.   if d PDF.EXPORT .tm .ds pdf:bm\\n[pdf:bm.nr].tag \\*[PDFBOOKMARK.NAME]
-.   \}
+.if !d pdf:bm\\n[pdf:bm.nk].tag \{\
+.   ds pdf:bm\\n[pdf:bm.nk].tag \\*[PDFBOOKMARK.NAME]
+.   ds pdf:bm\\n[pdf:bm.nk].val \\$*
+.\}
+.if d PDF.EXPORT \{\
+.   tm .ds pdf:bm\\n[pdf:bm.nk].tag \\*[PDFBOOKMARK.NAME]
+.   tm .ds pdf:bm\\n[pdf:bm.nk].val \\$*
+.\}
 .\"
 .\"
 .\" Irrespective of whether this marker is created, or not,
@@ -686,7 +682,8 @@ .de pdf*href
 .\" options -- anything which is not recognised is assumed to start
 .\" the "link text" component of the argument list).
 .\"
-.while dpdf:href.opt\\$1 \{\
+.while \A'\\$1' \{\
+.   if !dpdf:href.opt\\$1 .break
 .   pdf:href.opt\\$1 \\$@
 .   shift \\n[pdf:href.argc]
 .   \}
@@ -746,13 +743,8 @@ .de pdf*href
 .      \}
 .   el \{\
 .      ds PDFHREF.DESC Unknown
-.      ie d PRINTSTYLE \{\
-.         if dpdf:look(\\*[pdf:href-D]) .ds PDFHREF.DESC 
\\*[pdf:look(\\*[pdf:href-D])]
-.         \}
-.      el \{\
-.         pdf:lookup \\*[pdf:href-D]
-.         if !'\\*[pdf:lookup-result]'' .ds PDFHREF.DESC \\*[pdf:lookup-result]
-.         \}
+.      pdf:lookup \\*[pdf:href-D]
+.      if !'\\*[pdf:lookup-result]'' .ds PDFHREF.DESC \\*[pdf:lookup-value]
 .      \}
 .   \" Apply border and colour specifications to the PDFMARK string
 .   \" definition, as required.
@@ -862,11 +854,9 @@ .de pdf*href-I
 .
 .de pdfmarksuspend
 .nop \!x X pdf: marksuspend
-.nr pdf*is-mark-suspended 1
 ..
 .de pdfmarkrestart
-.if \\n[pdf*is-mark-suspended] .nop \!x X pdf: markrestart
-.nr pdf*is-mark-suspended 0
+.nop \!x X pdf: markrestart
 ..
 .de pdfpagename
 .nop \!x X pdf: pagename \\$1

I find myself wishing that the "various problems" had reproducers to
accompany them that I could turn into automated tests.

Regards,
Branden

signature.asc
Description: PGP signature