Revert "ammend 205257e" · reutenauer/polyglossia@83b1cbf

Udi-Fogiel · 2024-11-24T15:28:39Z

There is no starred variant of \selectlanguage anymore... Can you give a simple example of the problem?

jspitz · 2024-11-24T15:35:41Z

See Scott's mail to both of us.

Udi-Fogiel · 2024-11-24T18:31:33Z

See Scott's mail to both of us.

Oh, thanks, I've completely missed these messages.

Here is the source of the problem

\documentclass{article}
\usepackage{nameref}
\usepackage{color}
\usepackage{polyglossia}
\setmainlanguage{english}
\setotherlanguage{french}
\newfontfamily\englishfont{Latin Modern Roman}[Color=blue]
\newfontfamily\frenchfont{Latin Modern Roman}[Color=red]
\begin{document}

\begin{figure}
foo\caption{\label{foo}foo}
\end{figure}

\selectlanguage{french}
\nameref{foo}

\end{document}

If you will look in the aux file you will see the line

\newlabel{foo}{{1}{1}{\xpg@aux {\xpg@current@opts }{\languagename }foo}{figure.1}{}}

since \xpg@current@opts, \languagename are not expanded it caused an infinite loop.
In the latest commit I made sure \languagename would not be defined as it self, which
caused the infinite loop (which is basically returning to the situation before merging my branch),
but I'm not sure it is the correct solution. Should the reference in the example by typeset in French or English?

Currently it is in French

Udi-Fogiel · 2024-11-24T18:34:23Z

Note that it is the same in v2.3.

Udi-Fogiel · 2024-11-24T18:49:45Z

A possible patch

diff --git a/tex/polyglossia.sty b/tex/polyglossia.sty
index 8f9013eb..c8b376b9 100644
--- a/tex/polyglossia.sty
+++ b/tex/polyglossia.sty
@@ -1474,13 +1474,10 @@
    % Since captions might float to other language regions,
    % we need to specify the language here (#542)
    \cs_set_eq:cc { __xpg_save_caption:n } { @caption }
-   \cs_new:Npn \xpg@current@opts {}
 
    \cs_set:Npn \@caption #1 [#2] #3
      {
-           % we might be outside of l3 catcode regime
-           \tl_set_eq:NN \xpg@current@opts \xpg__current_options_tl
-           \__xpg_save_caption:n { #1 } [ { \xpg@aux{\xpg@current@opts}{\languagename}#2 } ] { #3 }
+          \exp_args:NNNe \__xpg_save_caption:n { #1 } [ { \xpg@aux{\xpg__current_options_tl}{\languagename}\exp_not:n{#2} } ] { #3 }
      }
 }

jspitz · 2024-11-25T08:22:57Z

The reference should be in French since it is in a French context (the figure caption in the LOF should be in English). So this looks correct to me.

Udi-Fogiel · 2024-11-25T08:38:13Z

The reference should be in French since it is in a French context (the figure caption in the LOF should be in English). So this looks correct to me.

So we have a redundant language switch...

And what about the following?

\documentclass{article}
\usepackage{nameref}
\usepackage{color}
\usepackage{polyglossia}
\setmainlanguage{hebrew}
\setotherlanguage{french}
\newfontfamily\hebrewfont{DavidCLM}[Color=blue]
\newfontfamily\frenchfont{Latin Modern Roman}[Color=red]
\begin{document}

\begin{figure}
foo\caption{\label{foo}שלום}
\end{figure}

\selectlanguage{french}
\nameref{foo}

\end{document}

Its seems a bit counter productive to me.

jspitz · 2024-11-25T09:40:14Z

Maybe, but that's how nameref works. This is not related to floating objects. See:

% !TeX TS-program = xelatex
\documentclass{article}
\usepackage{nameref}
\usepackage{color}
\usepackage{polyglossia}
\setmainlanguage{hebrew}
\setotherlanguage{french}
\newfontfamily\hebrewfont{David CLM}[Color=blue]
\newfontfamily\frenchfont{Garamond}[Color=red]
\begin{document}
	
\section{\label{bar}שלום}
	\begin{figure}
		foo\caption{\label{foo}שלום}
	\end{figure}
	
	\selectlanguage{french}
	\nameref{foo}
	
	\nameref{bar}
	
\end{document}

I don't think polyglossia should tinker with that.

Udi-Fogiel · 2024-11-25T10:34:52Z

Just note that we did change the code in the aux file because of our patch to \caption. Compare the lines in the aux

\newlabel{foo}{{1}{1}{\xpg@aux {\xpg@current@opts }{\languagename }שלום}{figure.1}{}} % from the caption
\newlabel{bar}{{1}{1}{שלום}{section.1}{}} % from the section

It works, but maybe removing the undesired \xpg@aux {\xpg@current@opts }{\languagename } would be better.

jspitz · 2024-11-25T10:39:16Z

We added it since captions might float to regions with different language, and then the language of the caption entry can be wrong (in LOF). We have a ticket for this, query for it.
If you find a solution that does not break the example in the ticket, I am all for removing redundant code.

Udi-Fogiel · 2024-11-25T11:16:29Z

We added it since captions might float to regions with different language, and then the language of the caption entry can be wrong (in LOF). We have a ticket for this, query for it.

Yes, I'm aware of that.

If you find a solution that does not break the example in the ticket, I am all for removing redundant code.

How about the following patch?

diff --git a/tex/polyglossia.sty b/tex/polyglossia.sty
index 03f3939a..d5b05835 100644
--- a/tex/polyglossia.sty
+++ b/tex/polyglossia.sty
@@ -1473,14 +1473,20 @@
    % for the lot/lof.
    % Since captions might float to other language regions,
    % we need to specify the language here (#542)
-   \cs_set_eq:cc { __xpg_save_caption:n } { @caption }
-   \cs_new:Npn \xpg@current@opts {}
-
-   \cs_set:Npn \@caption #1 [#2] #3
+   \cs_if_exist:cTF { NR@@caption }
      {
-           % we might be outside of l3 catcode regime
-           \tl_set_eq:NN \xpg@current@opts \xpg__current_options_tl
-           \__xpg_save_caption:n { #1 } [ { \xpg@aux{\xpg@current@opts}{\languagename}#2 } ] { #3 }
+       \cs_set_eq:Nc \__xpg_save_caption:n { NR@caption }
+       \cs_set:Npn \NR@caption #1 [#2] #3
+         {
+           \__xpg_save_caption:n { #1 } [ { \xpg@aux{\xpg__current_options_tl}{\languagename}#2 } ] { #3 }
+         }
+     }
+     {
+       \cs_set_eq:Nc \__xpg_save_caption:n { @caption }
+       \cs_set:Npn \@caption #1 [#2] #3
+         {
+           \__xpg_save_caption:n { #1 } [ { \xpg@aux{\xpg__current_options_tl}{\languagename}#2 } ] { #3 }
+         }
      }
 }

jspitz · 2024-11-25T12:42:12Z

What are NR@caption and NR@@caption?

Udi-Fogiel · 2024-11-25T12:57:11Z

\NR@@caption is where nameref save the original \@caption (same as \__xpg_save_caption:n), \NR@caption is a typo

jspitz · 2024-11-25T13:00:45Z

So if \NR@@caption exists we do not need to patch \@caption for the LOF to be correct? Why is that?

Udi-Fogiel · 2024-11-25T13:04:55Z

Correct patch

diff --git a/tex/polyglossia.sty b/tex/polyglossia.sty
index b7c628b1..c9e244a1 100644
--- a/tex/polyglossia.sty
+++ b/tex/polyglossia.sty
@@ -1473,14 +1473,24 @@
    % for the lot/lof.
    % Since captions might float to other language regions,
    % we need to specify the language here (#542)
-   \cs_set_eq:cc { __xpg_save_caption:n } { @caption }
    \cs_new:Npn \xpg@current@opts {}
-
-   \cs_set:Npn \@caption #1 [#2] #3
+   \cs_if_exist:cTF { NR@@caption }
      {
+       \cs_set_eq:cc { __xpg_save_caption:n } { NR@@caption }
+       \cs_set:Npn \NR@@caption #1 [#2] #3
+         {
+           % we might be outside of l3 catcode regime
+           \tl_set_eq:NN \xpg@current@opts \xpg__current_options_tl
+           \__xpg_save_caption:n { #1 } [ { \xpg@aux{\xpg@current@opts}{\languagename}#2 } ] { #3 }
+         }
+     }{
+       \cs_set_eq:cc { __xpg_save_caption:n } { @caption }
+       \cs_set:Npn \@caption #1 [#2] #3
+         {
            % we might be outside of l3 catcode regime
            \tl_set_eq:NN \xpg@current@opts \xpg__current_options_tl
            \__xpg_save_caption:n { #1 } [ { \xpg@aux{\xpg@current@opts}{\languagename}#2 } ] { #3 }
+         }
      }
 }

Udi-Fogiel · 2024-11-25T13:12:02Z

So if \NR@@caption exists we do not need to patch \@caption for the LOF to be correct? Why is that?

nameref redefines \@caption as

\@caption=\long macro:
#1[#2]->\NR@gettitle {#2}\NR@@caption {#1}[{#2}]

where \NR@@caption is the original \@caption, and \NR@title is a macro that saves the label title.
We want the language information only in the LOF slot, which the second argument of \NR@@caption
is responsible for, while not add the language information to the label title.

Since \NR@@caption is in fact the original \@caption this is what we want to patch.

jspitz · 2024-11-25T13:13:48Z

I see. OK, then (haven't tested, though)

-Original file line number
+Diff line change
@@ Expand Up / @@ -1480,7 +1480,7 @@ @@
          {
                % we might be outside of l3 catcode regime
                \tl_set_eq:NN \xpg@current@opts \xpg__current_options_tl
-               \__xpg_save_caption:n { #1 } [ { \xpg@aux{\xpg@current@opts}{\languagename}#2 } ] { #3 }
+               \__xpg_save_caption:n { #1 } [ { \selectlanguage*[\xpg@current@opts]{\languagename}#2 } ] { #3 }
          }
     }
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Commit

There are no files selected for viewing

17 comments on commit `83b1cbf`

Udi-Fogiel commented on `83b1cbf` Nov 24, 2024 •

edited

Loading

jspitz commented on `83b1cbf` Nov 24, 2024 •

edited

Loading

Udi-Fogiel commented on `83b1cbf` Nov 24, 2024 •

edited

Loading

Udi-Fogiel commented on `83b1cbf` Nov 24, 2024

Udi-Fogiel commented on `83b1cbf` Nov 24, 2024

jspitz commented on `83b1cbf` Nov 25, 2024

Udi-Fogiel commented on `83b1cbf` Nov 25, 2024

jspitz commented on `83b1cbf` Nov 25, 2024

Udi-Fogiel commented on `83b1cbf` Nov 25, 2024

jspitz commented on `83b1cbf` Nov 25, 2024 •

edited by Udi-Fogiel

Loading

Udi-Fogiel commented on `83b1cbf` Nov 25, 2024 •

edited

Loading

jspitz commented on `83b1cbf` Nov 25, 2024

Udi-Fogiel commented on `83b1cbf` Nov 25, 2024

jspitz commented on `83b1cbf` Nov 25, 2024

Udi-Fogiel commented on `83b1cbf` Nov 25, 2024

Udi-Fogiel commented on `83b1cbf` Nov 25, 2024

jspitz commented on `83b1cbf` Nov 25, 2024

Commit

There are no files selected for viewing

17 comments on commit 83b1cbf

Udi-Fogiel commented on 83b1cbf Nov 24, 2024 • edited Loading

Choose a reason for hiding this comment

jspitz commented on 83b1cbf Nov 24, 2024 • edited Loading

Choose a reason for hiding this comment

Udi-Fogiel commented on 83b1cbf Nov 24, 2024 • edited Loading

Choose a reason for hiding this comment

Udi-Fogiel commented on 83b1cbf Nov 24, 2024

Choose a reason for hiding this comment

Udi-Fogiel commented on 83b1cbf Nov 24, 2024

Choose a reason for hiding this comment

jspitz commented on 83b1cbf Nov 25, 2024

Choose a reason for hiding this comment

Udi-Fogiel commented on 83b1cbf Nov 25, 2024

Choose a reason for hiding this comment

jspitz commented on 83b1cbf Nov 25, 2024

Choose a reason for hiding this comment

Udi-Fogiel commented on 83b1cbf Nov 25, 2024

Choose a reason for hiding this comment

jspitz commented on 83b1cbf Nov 25, 2024 • edited by Udi-Fogiel Loading

Choose a reason for hiding this comment

Udi-Fogiel commented on 83b1cbf Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

jspitz commented on 83b1cbf Nov 25, 2024

Choose a reason for hiding this comment

Udi-Fogiel commented on 83b1cbf Nov 25, 2024

Choose a reason for hiding this comment

jspitz commented on 83b1cbf Nov 25, 2024

Choose a reason for hiding this comment

Udi-Fogiel commented on 83b1cbf Nov 25, 2024

Choose a reason for hiding this comment

Udi-Fogiel commented on 83b1cbf Nov 25, 2024

Choose a reason for hiding this comment

jspitz commented on 83b1cbf Nov 25, 2024

Choose a reason for hiding this comment

17 comments on commit `83b1cbf`

Udi-Fogiel commented on `83b1cbf` Nov 24, 2024 •

edited

Loading

jspitz commented on `83b1cbf` Nov 24, 2024 •

edited

Loading

Udi-Fogiel commented on `83b1cbf` Nov 24, 2024 •

edited

Loading

Udi-Fogiel commented on `83b1cbf` Nov 24, 2024

Udi-Fogiel commented on `83b1cbf` Nov 24, 2024

jspitz commented on `83b1cbf` Nov 25, 2024

Udi-Fogiel commented on `83b1cbf` Nov 25, 2024

jspitz commented on `83b1cbf` Nov 25, 2024

Udi-Fogiel commented on `83b1cbf` Nov 25, 2024

jspitz commented on `83b1cbf` Nov 25, 2024 •

edited by Udi-Fogiel

Loading

Udi-Fogiel commented on `83b1cbf` Nov 25, 2024 •

edited

Loading

jspitz commented on `83b1cbf` Nov 25, 2024

Udi-Fogiel commented on `83b1cbf` Nov 25, 2024

jspitz commented on `83b1cbf` Nov 25, 2024

Udi-Fogiel commented on `83b1cbf` Nov 25, 2024

Udi-Fogiel commented on `83b1cbf` Nov 25, 2024

jspitz commented on `83b1cbf` Nov 25, 2024