vincentb1 / fmtcount

Source for LaTeX fmtcount package
11 stars 5 forks source link

File fmtcount.sty needs to detect luatex #28

Closed ghost closed 6 years ago

ghost commented 7 years ago

Currently, fmtcount.sty detects xetex. But it does not detect luatex. If the compiler is luatex (using polyglossia), the logic is incorrectly forked to a non-xetex branch that assumes pdftex.

Solution:

\usepackage{ifluatex}
% Then, in several places:
\ifxetex
  % code for xetex branch
\else
   \ifluatex
      % code for luatex branch, probably same as for xetex
  \else
     % code for pdftex branch
  \fi
\fi
vincentb1 commented 6 years ago

Hello Robert (@RobtAll), I have pushed to the repo a fix with a different approach. I do not try to check for which compiler is used — well I try to minimize the use of this information. Instead I check for the multilingual packages like babel, polyglossia and so on. This was already done to some extent, but now

  1. I check more of such packages (e.g. mlp is also checked now).
  2. The checking code is factorized in a loop I have also added to the test bench a test with lualatex. I hope that this fixes the point. I have not yet delivered it to CTAN: would you like to beta-test it from the repo?
ghost commented 6 years ago

Will grab from repo and beta test.

vincentb1 commented 6 years ago

Great! And thanks for being so much more reactive than I have been.

ghost commented 6 years ago

Compiling with LuaLaTeX, very basic article document class, without explicitly loaded any packages other than fmtcount: Even though babel was not loaded (verified), there is no warning from fmtcount. But it is your intent to throw an error or warning in this case, yes?

Does not matter whether or not I loaded fontspec (which I should), or not.

vincentb1 commented 6 years ago

Hello, This is the normal behaviour. fmtcount has two modes:

Multilingual mode is triggered when some package such as babel, mlp, polyglossia or ngerman is loaded. Multilingual mode detects the language based on which package definition file was loaded. For instance \usepackage[french]{babel} will detect French. When no package such as babel and suchlikes is detected, then fmtcount falls back to default mode, in which \languagename is not used, and the language is English.

Probably this deserves better documenting. FYI, probably mlp does not work properly, I have just added triggering of multilingual mode, but language detection is not implemented, and I have not developped any tests.

ghost commented 6 years ago

OK, will continue beta testing in English and French. When I wrote my prior comment, I noticed that there was still an \ifxetex test in the *.sty file.

Be sure that the final documentation insists that main language be set prior to loading fmtcount. A French user of my own package first brought the problem to my attention. He loaded polyglossia in advance, presumable with Enlgish default, so no problem detected by fmtcount. But then he changed the main language after loading fmtcount. Not clear why that should be a problem in the code, but then it's fixable at the user end.

Also: `make dist' throws an error, so I'm working directly from the *.sty file.

vincentb1 commented 6 years ago

Yes, I know about the left-over \ifxetex. I acknowledge that this is a little bit inconsistent with my initial intent to test only for packages, and no longer for compilers. I left it because polyglossia needs xetex, doesn't it ? Maybe polyglossia works fines too with luatex too, so I should define it otherwise to account for lualatex case. Probably it would be better to just do \@ifpackageloaded{polyglossia}{...}{...} instead of \ifxetex, as anyway polyglossia language definition files can be loaded only if polyglossia was loaded in the first place. I also noticed that the code could be optimized: \@fc@loadifbabelorpolyglossialdf is iterated on all languages, so it would be less processing to test xetex/luatex outside the loop to control the definition of \@fc@loadifbabelorpolyglossialdf, rather than making the test at each iteration.

ghost commented 6 years ago

Package polyglossia works great with LuaLaTeX, and is the preferred method. It also loads babel, I believe. But a lot of advice on the Internet is old, and refers back to a time when polyglossia was more experimental. So, those old sites often suggest using babel alone.

Assume that anyone using either XeTeX or LuaLaTeX has loaded the right language package.

\@ifpackageloaded{polyglossia}
   {do something with polyglossia } % has polyglossia, probably also has babel
  {\@ifpackageloaded{babel} % no polyglossia, but does have babel
    {do something with babel}
    {error message, tell user to get polyglossia or babel}
} % end

I think it is safe to assume that the user knows whether polyglossia or babel is preferred. If a pdflatex user attempts to load polyglossia, then there should be an error message from that package. If a LuaLaTeX user loads babel without polyglossia, then the document can be compiled as long as there is no polyglossia-dependent code. I do not know about XeTeX, since I don't use it.

vincentb1 commented 6 years ago

I have done the change, and pushed it to repo. I also improved the documentation concerning multilingual mode.

Actually the lualatex test case was not working properly before this using of \@ifpackageloaded instead of \ifxetex. As a matter of fact the language definition was loaded in the document body, rather than in the preamble, because fmtcount was successful in detected multilingual mode, but failing in detecting the needed language — namely French for the concerned sole test made for lualatex. This loading in the document body does not work so well — it inserts spurious spaces, I have created issue #30 about this.

With this change where \@ifpackageloaded is used, the needed dialect is detected right in the preamble, and there aren't any spurious spaces occuring in the document body on the first fmcount language dependant macro call. I must admit that I had overlooked this issue when creating the lualatex test.

vincentb1 commented 6 years ago

BTW: happy Christmas! 🎄

ghost commented 6 years ago

Pulled new code, will continue testing, and will also look at the new spacing issue. And a Happy Christmas to you and all! Being retired, every day is a holiday for me. But today, somebody else does the cooking. :)

vincentb1 commented 6 years ago

This issue #30 caused extra spacing with the old code, as the lualatex test case was for French, but it would have caused a compilation error with Italian (for which I have not created any lualatex test). So this issue #30 cannot be fixed with Italian: I mean that one still can minimize the risk that \FCloadlang is called in the document body — which what the new code does — but you cannot prevent it for all cases if the use makes silly code. For instance this code will work but will have spurious spacing before word zehn:

\documentclass{minimal}
\usepackage[french]{fmtcount}
\begin{document}
\def\languagename{german}
$>$\numberstringnum{10}$<$
\end{document}

but this code will fail:

\documentclass{minimal}
\usepackage[french]{fmtcount}
\begin{document}
\def\languagename{italian}
$>$\numberstringnum{10}$<$
\end{document}

In the two examples above french option passed to fmtcount triggers the multilingual mode. Then the \def\languagename{} triggers some \FCloadlang by \numberstringnum.

The reason is that with French the definition files do not contain any \RequirePackage so they can be input in the document body, while the Italian is just a wrapper around some other package, so it cannot be input in the doc body. The extra spaces are just spaces in the definition files that are not typeset when the input occurs in the preamble, but which go to output if it occurs in the document body, which was the case with the old code. The fix of issue #30 is to remove all useless spaces in definition files which are not wrappers — ie all definition files except Italian.

ghost commented 6 years ago

Appears OK in LuaLaTeX, whether using babel or polyglossia, English or French.

vincentb1 commented 6 years ago

Great, I will close the issue and ship it to CTAN.