Reversed logic in handling "printSOfinal", in def of \pdfmarkupcomment

Issue #23 closed
Yukai Chou created an issue

In the doc of option "printSofinal" (Sec. 1.2.37), it is said

true | false, default is true

With the option printSOfinal you can automatically delete PDF StrikeOut markup annotations including the text while using one of the options final or disable.

In my opinion, the logic would be

IF (global.final || global.disable) THEN
  IF (local.markup == StrikeOut && local.printSOfinal) THEN
    omit both annotation and text
  ELSE
    omit annotation only
  FI
ELSE
  show both annotation and text
FI

In the definition of \pdfmarkupcomment, there are two copies of almost-the-same implementations of the above logic. One is for math mode, the other one is for text mode. The related codes in text mode are (lines from 2213 to 2237, see [1])

    \ifpc@gopt@final%
      \ifthenelse{\equal{\pc@lopt@markup}{StrikeOut}}%
      {%
        \ifthenelse{\equal{\pc@lopt@printSOfinal}{true}}%
        {\SOUL@{#2}\global\pc@ignorespacesfalse}%        <<< HERE, extra \SOUL@
        {\global\pc@ignorespacestrue}%
      }%
      {%
        #2\global\pc@ignorespacesfalse%
      }%
    \else%
      \ifpc@lopt@disable%
        \ifthenelse{\equal{\pc@lopt@markup}{StrikeOut}}%
        {%
          \ifthenelse{\equal{\pc@lopt@printSOfinal}{true}}%
          {#2\global\pc@ignorespacesfalse}%
          {\global\pc@ignorespacestrue}%
        }%
        {%
          #2\global\pc@ignorespacesfalse%
        }%
      \else%
        \SOUL@{#2}\global\pc@ignorespacesfalse%
      \fi%
    \fi%

From my point of view, there are two bugs in the above codes

  1. Only when "printSOfinal == false", text is omitted. This shows a reversed logic with what is said in the document. In the document, and in my personal understanding, text would be omitted only when "printSOfinal == true".

The codes for math mode also have this problem.

  1. In line 2217 (marked by "<<< HERE" in the above code snip, see [2]), I think there is an extra \SOUL@.

Ref to pdfcomment.sty

[1] https://bitbucket.org/kleberj/pdfcomment/src/9834b5052089bc458b74a67355a6e2e34d3e7d19/dev/tex/latex/pdfcomment/pdfcomment.sty#lines-2213:2237

[2] https://bitbucket.org/kleberj/pdfcomment/src/9834b5052089bc458b74a67355a6e2e34d3e7d19/dev/tex/latex/pdfcomment/pdfcomment.sty#lines-2217

Comments (9)

  1. Josef Kleber repo owner

    This option was introduced due to a user request, who wanted striked out text to be auto deleted. This was a no go, as it would have broken default behaviour. I added the option printSOfinal with default value true in order to not break default behaviour. So read this option as 'print Strike Out markup text in case of final (or disable)' or do not print in case of false

    \documentclass[11pt]{article}
    \usepackage{xcolor}
    \usepackage[color=red,final]{pdfcomment}
    \begin{document}
    surrounding text \pdfmarkupcomment[markup=StrikeOut,printSOfinal=true]{text to be deleted}{delete text} surrounding text 
    
    surrounding text \pdfmarkupcomment[markup=StrikeOut,printSOfinal=false]{text to be deleted}{delete text} surrounding text 
    \end{document}
    

    minimal.jpg

    I agree the description could be more clear!

  2. Josef Kleber repo owner

    Sorry, I was too fast! Your report about line 2217 is of course right. That's a bug. I will fix this with the next update. In the meantime i saw your patch for the other issue. Thanks! ;-) I will check as soon as possible, but I'm a bit tight on time at the moment. Might take a while.

  3. Yukai Chou reporter

    read this option as ... or do not print in case of false

    no bug but inprecise documentation

    I agree.


    Thanks for your quick response and informative explanations.

  4. Log in to comment