reutenauer / polyglossia

An alternative to Babel for XeLaTeX and LuaLaTeX
http://www.ctan.org/pkg/polyglossia
MIT License
189 stars 51 forks source link

Entries in List of tables do not respect language settings like babelshorthands #657

Open LSinev opened 1 month ago

LSinev commented 1 month ago

This was working someday at 2021, but now it fails. MWE

% !TeX TS-program = xelatex
\documentclass{article}
\usepackage[babelshorthands=true]{polyglossia}
\setmainfont{LiberationSerif}[Ligatures=TeX]
\newfontfamily\cyrillicfont{LiberationSerif}[Ligatures=TeX,Script=Cyrillic]
\setdefaultlanguage[babelshorthands=true]{russian}
\listfiles
% tested with: polyglossia 2023/12/11 v1.66
% tested with: polyglossia.sty    2024/07/15 v2.2 
\begin{document}

................  кириллица

-- is "en-dash"

--- is "em-dash"

"--- is "cyrdash"

\cyrdash is "cyrdash"

\begin{table}[ht]%
    \caption{Любя, съешь щипцы, "--- вздохнёт мэр, {\cyrdash} кайф --- жгуч}%
    \label{tab:test}
    Imagine some tabular here
\end{table}

\listoftables

\end{document}

tested with versions 1.66 and 2.2.

It looks like this Снимок экрана от 2024-08-04 16-26-01 but I want entry in LoT to respect babelshorthands and other language settings.

TeXlive 2021 (Overleaf, polyglossia.sty 2021/04/12 v1.53) result Снимок экрана от 2024-08-04 16-37-24

TeXLive 2022 (Overleaf, polyglossia.sty 2022/04/20 v1.56) result Снимок экрана от 2024-08-04 16-39-59

I think, storing of language settings at least from language setup is somehow missed or overriden after this change: https://github.com/reutenauer/polyglossia/commit/d4ae868f4ad154499b48f4ab69e317424e210628

cc @jspitz

jspitz commented 1 month ago

Should be fixed.

jspitz commented 1 month ago

Note that the glitch mentioned at https://github.com/reutenauer/polyglossia/issues/542#issuecomment-1190287161 is back, but that's due to a different change.

I am not very happy with the current solution anyway, maybe @Udi-Fogiel has a better idea.

Udi-Fogiel commented 1 month ago

@jspitz Is there a reason why we do not use \selectlanguage* like we do in toc?

jspitz commented 1 month ago

Cannot remember the details now, but I think the output was not correct in all cases.

jspitz commented 1 month ago

Ideally, we would intrude the language switch at the beginning of the second argument of \contentsline. This would avoid that counters and page numbers use a different font if defined so for the contextual language.

Maybe we should request a respective hook.

jspitz commented 1 month ago

Ideally, we would intrude the language switch at the beginning of the second argument of \contentsline.

\numberline, for that matter

LSinev commented 1 month ago

not sure about it, but may be useful to document also the way for users to add such functionality in their custom \listof* (list of program/algorithm listings, list of theorems, list of footnotes and so on)

LSinev commented 1 month ago

By the way, how do you make the tds package of the polyglossia dev version to test it locally or at Overleaf (as simply getting the polyglossia.sty produces errors)?

Udi-Fogiel commented 1 month ago

Ideally, we would intrude the language switch at the beginning of the second argument of \contentsline. This would avoid that counters and page numbers use a different font if defined so for the contextual language.

Maybe we should request a respective hook.

Is it true for toc as well?

Udi-Fogiel commented 1 month ago

By the way, how do you make the tds package of the polyglossia dev version to test it locally or at Overleaf (as simply getting the polyglossia.sty produces errors)?

You can run l3build install locally on your machine to install the development version. On over leaf i don't know if you can do anything, sometimes the development version requires an up to date format.

jspitz commented 1 month ago

This patch works for all cases AFAICS (also with the caption package). A hook would be better, though, then we would not need to redefine commands which is always risky. 657.diff.txt

jspitz commented 1 month ago

Is it true for toc as well?

Theoretically yes. But I have no case for this.

jspitz commented 1 month ago

Is it true for toc as well?

Theoretically yes. But I have no case for this.

Sorry, no, it isn't. It is needed for lof and lot as floats might end up in other places where we have different context language.

jspitz commented 1 month ago

BTW @LSinev there shouldn't be a linebreak after \cyrdash as in your MWE, right? So I suppose we need to change:

diff --git a/tex/gloss-russian.ldf b/tex/gloss-russian.ldf
index 34b2abd2..6423b646 100644
--- a/tex/gloss-russian.ldf
+++ b/tex/gloss-russian.ldf
@@ -148,7 +148,7 @@
   \def\@Ccdash{\leavevmode
    \nobreak\cyrdash\nobreak\hskip.35em\ignorespaces}%
   \ifx\cyrdash\undefined
-    \def\cyrdash{\hbox to.8em{\textendash\hss\textendash}}%
+    \def\cyrdash{\leavevmode\hbox to.8em{\textemdash\hss\textendash}}%
   \fi
   \declare@shorthand{russian}{",}{\nobreak\hskip.2em\ignorespaces}%
 }
LSinev commented 1 month ago

there shouldn't be a linebreak

Yeah, this is good to be fixed.

But, after your change there will be change in length, I suppose, because was combination of 2 en-dashes, and after change it is em-dash + en-dash. I think this is typo.

jspitz commented 1 month ago

Yes, this was a typo. I've fixed it in all glosses that use it at f501eabe5ab.

jspitz commented 1 month ago

I accidentally also committed the patch from https://github.com/reutenauer/polyglossia/issues/657#issuecomment-2271148266.

I will revert it if we agree on that, but maybe we should just proceed with that approach.

Udi-Fogiel commented 1 month ago

This patch works for all cases AFAICS (also with the caption package). A hook would be better, though, then we would not need to redefine commands which is always risky. 657.diff.txt

If this is a direction we are willing to go for, I would not declare the second argument of \@caption as optional, as it is not one in the original definition, it is just delimited by square brackets (also the internal structure of commands created by xparse is weird, and should probably be avoided here). The \protect does nothing as \selectlanguage is protected in the e-TeX sense. Lastly, probably using \text<lang> (or an internal variant) would be better here, for bidi stuff in XeTeX.

Is it true for toc as well?

Theoretically yes. But I have no case for this.

Sorry, no, it isn't. It is needed for lof and lot as floats might end up in other places where we have different context language.

I don't understand this comment. The output is not related to the location, in toc I get one thing if the direction of the entry is different from the direction of the table itself while in lof and lot I get another. So which one is correct?

I accidentally also committed the patch from #657 (comment).

I will revert it if we agree on that, but maybe we should just proceed with that approach.

If I go back to 3fae6228 and remove the caption patch, I don't get the problem reported in #542. So maybe it is a non-existing problem? maybe the problem was elsewhere?

Udi-Fogiel commented 1 month ago

If I go back to 3fae622 and remove the caption patch, I don't get the problem reported in #542. So maybe it is a non-existing problem? maybe the problem was elsewhere?

never mind, I was wrong here...

jspitz commented 1 month ago

If this is a direction we are willing to go for, I would not declare the second argument of \@caption as optional, as it is not one in the original definition, it is just delimited by square brackets (also the internal structure of commands created by xparse is weird, and should probably be avoided here). The \protect does nothing as \selectlanguage is protected in the e-TeX sense. Lastly, probably using \text<lang> (or an internal variant) would be better here, for bidi stuff in XeTeX.

Feel free to improve on the change as you see fit. I thin that's easier than telling me how to do it.

jspitz commented 3 weeks ago

I have adapted the code at b2589c780c5ad

I'd like to do a release soon. After that, you can start implementing the changes from your branch. OK?

Udi-Fogiel commented 1 week ago

Feel free to improve on the change as you see fit. I thin that's easier than telling me how to do it.

Sorry, I wasn't near a suitable computer to push the changes at the time.

I have adapted the code at b2589c7

I'd like to do a release soon. After that, you can start implementing the changes from your branch. OK?

Thanks! I've simplified things a bit. Lets wait with the release until we'll figure the latest regression? (the tests are failing now...)