\begin{lang}{<lang>} broken

[thomwiggers]

Using Polyglossia 2.1 in TexLive 2024 (as distributed on MacOS by Homebrew as 20240312 and as distributed on Arch Linux as 2024.0-3), the following snippet of code no longer works:

\documentclass{scrartcl}

\usepackage{polyglossia}

\setmainlanguage{english}
\setotherlanguage{dutch}

\begin{document}

\begin{lang}{dutch}
  Hallo allemaal!
\end{lang}

This is typeset in English.

\end{document}

The following does work as expected:

\documentclass{scrartcl}

\usepackage{polyglossia}

\setmainlanguage{english}
\setotherlanguage{dutch}

\begin{document}

\begin{dutch}
  Hallo allemaal!
\end{dutch}

This is typeset in English.

\end{document}

Naturally, this used to work in TexLive 2023 (Polyglossia 1.60)

[jspitz]

Yes, indeed. Fixed for the next version.

[Udi-Fogiel]

@jspitz I don't think this is the correct fix. The lines

% Alias to {<lang>}, but more suitable
% for specific (esp. tag-based) aliases
% where {<alias>} would cause clashes
% (e.g., \fi)
\DeclareEnvironmentCopy { lang } { otherlanguage }

should simply be moved after otherlanguae is declared, i.e.

diff --git a/tex/polyglossia.sty b/tex/polyglossia.sty
index 4e6fa20..204c7b1 100644
--- a/tex/polyglossia.sty
+++ b/tex/polyglossia.sty
@@ -1729,12 +1729,6 @@
 % inside case changing commands (e.g. \MakeUppercase)
 \tl_put_right:Nn \l_text_case_exclude_arg_tl { \textlang }

-% Alias to {<lang>}, but more suitable
-% for specific (esp. tag-based) aliases
-% where {<alias>} would cause clashes
-% (e.g., \fi)
-\DeclareEnvironmentCopy { lang } { otherlanguage }
-
 % wrapper for foreignlanguage and otherlanguage*
 \newcommand*\polyglossia@setforeignlanguage[2][]{
   \select@@language[#1]{#2}
@@ -2364,6 +2358,12 @@
     \__xpg_unstack_language:n{xpg@set@language@only@aux}
   }

+% Alias to {<lang>}, but more suitable
+% for specific (esp. tag-based) aliases
+% where {<alias>} would cause clashes
+% (e.g., \fi)
+\DeclareEnvironmentCopy { lang } { otherlanguage }
+
 \newcommand{\setlocalhyphenmins}[3]{
    \xpg@ifdefined{#1}{
       \expandafter\ifx\csname l@#1\endcsname\l@nohyphenation

[jspitz]

OK, fine. The fix was not "incorrect" though. Maybe slightly less elegant. The more important question is how this could have slipped in without being noticed in the first place.

[Udi-Fogiel]

Does the manual have any instances of the lang environment? The best thing to do to avoid regressions like this one (which admittedly was probably my fault) is to create a github action to compile some test .tex files after every commit to master.

[jspitz]

I think in the given case it would have sufficed to simply quickly test the environment after its definition has been changed. I generally test all things I touch.

[Udi-Fogiel]

I think in the given case it would have sufficed to simply quickly test the environment after its definition has been changed.

The change of the lang environment was part of code modernization commit (5b56b61) and I thought testing the manual was good enough.

I generally test all things I touch.

It is a bit cumbersome to do that, what should be considered a good test? what about internals that do not have a forward way to be tested?

For example, consider the following code

\documentclass{article}
\usepackage{polyglossia}
\begin{document}
\expandafter\ShowCommand\csname rmfamily \endcsname
\stop

before 0b9d4ae the log file contains the lines

> \rmfamily =macro:
->\xpg@defaultfont@rm \def \familytype {rm}.
<argument> \rmfamily  

l.4 ...dafter\ShowCommand\csname rmfamily \endcsname

but after 0b9d4ae the log shows

> \rmfamily =\long macro:
->\xpg@defaultfont@rm .
<argument> \rmfamily  

l.4 ...dafter\ShowCommand\csname rmfamily \endcsname

so clearly the patching of \rmfamily (note the space) is faulty.

The answer to what is easy to test, how it should be tested and when, is not trivial in my opinion. I think the best thing to do is to add regression tests each time there is one.

[jspitz]

Of course a test suite would be helpful. I'm looking into the patching issue you reported.

[Udi-Fogiel]

I'm looking into the patching issue you reported.

Just pushed a fix

[jspitz]

Thank you. You just beat me to it (with the same code).

[thomwiggers]

Do I correctly infer from the above discussion that \begin{otherlanguage}{dutch} would be more preferred syntax? (I didn't gather as much from skimming the manual)

[thomwiggers]

On the subject of a test suite, there does appear to exist a tests folder; it looks as if the MWE from my initial post could simply be added to that folder---running through that folder in CI also seems like a reasonable proposal.

[jspitz]

Do I correctly infer from the above discussion that \begin{otherlanguage}{dutch} would be more preferred syntax? (I didn't gather as much from skimming the manual)

No, that's the babel legacy command. But it is synonymous.

[thomwiggers]

ack; I'll switch to the other command in my document as it's easier than incorporating a patch and more future-proof.

[Udi-Fogiel]

ack; I'll switch to the other command in my document as it's easier than incorporating a patch and more future-proof.

I'm not Infront of a computer right now, but I believe you can simply add the line \DeclareEnvironmentCopy{lang}{otherlanguage} after loading polyglossia as a workaround until the next release

[thomwiggers]

Eh, making the other change was easy and more predictable to whatever person I next give the source code to my PhD thesis to use as a template :)

[jspitz]

I would personally go for \begin{dutch}, but it is up to you. The {lang}{<lang>} environment is only for cases which do not work in the other case. These are some BCP shorthands such as it and fi which are internally defined in TeX (\it, \fi).

[thomwiggers]

Right, I'm not sure if \begin{<lang>} is recognized by spell checkers, in my case LTeX.

[jspitz]

Right, I'm not sure if \begin{<lang>} is recognized by spell checkers, in my case LTeX.

Good point, but I suggest you check and report to them if it doesn't.

[thomwiggers]

Agreed that it's not your problem; unfortunately, in this particular case it also seems like the LTeX project is somewhat dead 😢

[jspitz]

They state to support babel commands (not polyglossia), so using babel legacy commands is probably the way to go for now.

[thomwiggers]

Ah, that's fair too. Oh well.

[Udi-Fogiel]

Of course a test suite would be helpful. I'm looking into the patching issue you reported.

@jspitz I've added an automatic test suit in 1a26afe. It is based on a github action that will trigger after each push or a PR to the main branch. It tests regressions using l3build. I currently added only one test file, it is in /testfiles (I will add more during the weekend). Note that currently .map files are not installed using l3build install, so don't add test files that check anything related to that.

l3build can also help in the process of updating version tags in files and ctan uploads. Do you think it will be better to transition to l3build from the current setup? I've never used the current scripts, so I can't really judge here, but from an objective point of view it will need less dependencies, less scripts and it should be OS independent (although I'm encountering few problems with l3build on my windows machine).

[Udi-Fogiel]

Note that currently .map files are not installed using l3build install, so don't add test files that check anything related to that.

As of f1dfd1c, .tec files are generated automatically, and both .map and .tec files are installed using l3build install