bug#71436: Fwd: preview in LuaTeX with RTL as main language does not work

[Udicoudco]

On Wed, Jun 12, 2024 at 2:25 PM Arash Esbati <arash@gnu.org> wrote:
> Thanks for the patch.  I have some minor comments (and I'm not good at
> writing TeX-code):
>
> > +\let\pr@set@pagerightoffset\@empty
>
> Why not defining the macro properly instead of \let-ing?
>
> \newcommand*\pr@set@pagerightoffset{}

no real reason, I've tried to write the code
in the same style of the package. I was reading
the lines

\let\pr@ship@start\@empty
\let\pr@ship@end\@empty

just before writing this one. If you prefer to use
\newcommand, that's fine (althogh personally
I don't really like to use \newcommand for internal
commands).

> > +\ifx\pagerightoffset\@undefined\else
> > +  \def\pr@set@pagerightoffset{%
> > +    \ifnum\pagedirection=1
>
> I don't think this relevant here, but
>
> \ifnum\pagedirection=1 %
>
> is the safe version?

Define "safe". The version in my patch
can theoretically break if \endlinechar is -1
at the time of the definition of  \def\pr@set@pagerightoffset
for example, as the end of line after \ifnum\pagedirection=1
will be removed and no space will be created after TeX tokenize
the macro definition, so if there are more digits (of something that
expands to digits) after  =1 we can be in trouble, but in this case
what appears after it is \pagerightoffset which is not expandable,
so it is safe.

For example with

\documentclass{article}
\begin{document}
\newcount\foo\foo=20
\endlinechar=-1
\def\bar{%
    \ifnum 1 = 1
    \the\foo Yes\else No\fi
  }
\show\bar\bar
\endlinechar=13
\end{document}

You will get no, as the definition of \ab does
not  have a space between 1 and \the (which
would be there if \endlinecahe wasn't -1), and
\the\foo expands to 20 and so the test actually
check \ifnum 1=120 (after \the\foo \ifnum encounter the 'Y'
which is not a digit so it stops).

With the following (which is similar to the code in the patch)

\documentclass{article}
\begin{document}
\newcount\foo\foo=20
\endlinechar=-1
\def\bar{%
    \ifnum 1 = 1
    \foo=1 Yes\else No\fi
  }
\show\bar\bar
\endlinechar=13
\end{document}

you get 'Yes' because \foo is unexpandable
and is not a digit nor is equivalent in some sense to a digit,
so \ifnum does not include him in the comparison.

Using \ifnum\pagedirection=1 % assume that '%'
has category code 14 (which is usually the case).

In any case, probably the safest way to go
is to use \ifnum\pagedirection=\@ne as it does not
rely on what appears next (\@ne is an unexpandable
token, to \ifnum stop looking after him, and it is equivalent to 1).

I don't really mind doing these changes,
it is really your call. Should I prepare
a new patch using \newcommand and
\ifnum\pagedirection=\@ne?

Regards,
Udi