vincentb1 / fmtcount

Source for LaTeX fmtcount package
11 stars 5 forks source link

\ordinalstring (and variants) fail with German language #33

Closed FrankMittelbach closed 4 years ago

FrankMittelbach commented 6 years ago
\documentclass{report}

\setcounter{chapter}{3}

\usepackage[ngerman]{babel}
\usepackage[ngerman]{fmtcount}

\begin{document}
\ordinalstring{chapter} 
\Ordinalstring{chapter} 
\ORDINALstring{chapter} 
\end{document}

gives 3 times

! You can't use `the character 3' after \the.`
FrankMittelbach commented 6 years ago

without much checking I guess what is needed is

\renewcommand*{\ORDINALstring}[1]{%
  \expandafter\ORDINALstringnum\expandafter{%
    \the\value{#1}%
  }%
}
\renewcommand*{\ordinalstring}[1]{%
  \expandafter\ordinalstringnum\expandafter{%
    \the\value{#1}}%
}
\renewcommand*{\Ordinalstring}[1]{%
  \expandafter\Ordinalstringnum\expandafter{%
    \the\value{#1}}%
}
vincentb1 commented 6 years ago

Dear Frank and al,

I am over busy currently, I will take the issue after beginning of June.

Thank you for your patience.

  Vincent.

PS : To Nicola, if you wish to make the fix, you will need to log all the 3 or 4 latest changes, and checkout the latest commit just before the 1st arabic change, and then make a branch from there.

Arabic is for the time being under construction…

Le 23/05/2018 à 15:55, Frank Mittelbach a écrit :

without much checking I guess what is needed is

|\renewcommand{\ORDINALstring}[1]{% \expandafter\ORDINALstringnum\expandafter{% \the\value{#1}% }% } \renewcommand{\ordinalstring}[1]{% \expandafter\ordinalstringnum\expandafter{% \the\value{#1}}% } \renewcommand*{\Ordinalstring}[1]{% \expandafter\Ordinalstringnum\expandafter{% \the\value{#1}}% } |

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nlct/fmtcount/issues/33#issuecomment-391355962, or mute the thread https://github.com/notifications/unsubscribe-auth/AExSI2nmomaXHJ1c8ZDgMYC-vBCTas47ks5t1WpsgaJpZM4UKbvV.

nlct commented 6 years ago

I'm sorry I don't have time to make the fix at the moment. (I'm going away in a couple of days and need to get some other projects finished before I go.)

FrankMittelbach commented 6 years ago

no need for a rush at my end (it is just that right now it is definitely broken :-) and I would like to include the package in the next edition of TLC)

vincentb1 commented 6 years ago

Hello,

From the comments for changes the 3 \expandafter's are needed for compatibility with glossaries. To my recollection glossaries does some really dirty trick re-defining \the in a way that it assumes that if you do \expandafter\the… then if the re-defined \the takes the next token, that will suffice to take all of what the original \the expects.

Actually the \ORDINALstringnum and suchlikes in the end will call some \@ORDINAL@string which is \let-ed to the language dependent implementation. And the language dépendent implemenation is supposed to do some \edef or some counter assignment of the numeric argument. Let us assume that it is an \edef:

\edef\foo{\expandafter\expandafter\expandafter\the\value{somecounter}}

will work, because the 1st expansion will make the \foo expanded definition become \expandafter\the\csname c@somecounter\endcsname, and the 2nd expansion will make it become \the\c@somecounter, so \the\c@somecounter will finaly be \edef-ed to the roman representation of counter somecounter. The same reasoning holds for assigning the content to a counter.

vincentb1 commented 6 years ago

So what happens, is that for some reason the intended scheme does not work. Probably, in the context where you use it, \ordinalstring is preceded by some \expandafter. So may be a fix would be to do the following:

\makeatletter
\renewcommand*{\ordinalstring}[1]{%
  \ordinalstringnum{\@empty\expandafter\expandafter\expandafter
    \the\value{#1}}%
}
\makeatother

the trick is that the added \@empty will make the preceding \expandafter — which I suppose to be the root cause for the issue — without effect.

Of course, a better fix would be to make glossaries bullet-proof, and then simply drop all the \expandafter in the definition, as essentially, they are not needed by fmtcount itself — and in your case they cause some issue.

vincentb1 commented 6 years ago

Just as a side comment, I acknowledge that there is also an issue in fmtcount. It seems that the fc-german.def file does not fulfil the requirement that the numeric argument shall be \edef-ed. Well, it does not work when removing altogether the \expandafter and writting this:

\documentclass{report}

\setcounter{chapter}{3}

\usepackage[ngerman]{babel}
\usepackage[ngerman]{fmtcount}

\begin{document}

\renewcommand*{\ordinalstring}[1]{\ordinalstringnum{\the\value{#1}}}%
\renewcommand*{\Ordinalstring}[1]{\Ordinalstringnum{\the\value{#1}}}%
\renewcommand*{\ORDINALstring}[1]{\ORDINALstringnum{\the\value{#1}}}%

\ordinalstring{chapter} 
\Ordinalstring{chapter} 
\ORDINALstring{chapter} 
\end{document}

However, it works if instead of ngerman I have french, portugese or english. So I will have a look at the german numeric stuff and make a fix.

vincentb1 commented 6 years ago

The problem is actually that fc-german.def calls some ifthen boolean expresssion that directly uses the argument without prior \edef-ing. Following is a quick fix:

diff --git a/trunk/fc-german.def b/trunk/fc-german.def
index 4f5609c..2718c8a 100644
--- a/trunk/fc-german.def
+++ b/trunk/fc-german.def
@@ -564,9 +564,10 @@
      \fi
    \fi
 \fi
-\@strctr=#1\relax
+\@tmpstrctr=#1\relax   
+\@strctr=\@tmpstrctr
 \@FCmodulo{\@strctr}{100}%
-\ifthenelse{\@strctr=0 \and #1>0}{}{%
+\ifthenelse{\@strctr=0 \and \@tmpstrctr>0}{}{%
 \@@numberunderhundredthgerman{\@strctr}{#2}%
 }%
 }%

Maybe I will rewrite it w/o the dependence on ifthen. I also need to check why the problem was overlooked by the test bench, and possibly add new tests into it …

FrankMittelbach commented 6 years ago

Am 30.05.18 um 06:40 schrieb Vincent Belaïche:

To my recollection |glossaries| does some really dirty trick re-defining |\the| in a way that it assumes that if you do |\expandafter\the|… then if the re-defined |\the| takes the next token, that will suffice to take all of what the original |\the| expects.

that sounds rather strange and a redef of a low-level like \the is more or less bound to create issues in other packages.

however after loading glossaries \show\the shows that it is still the primitive so where and when is that change happening?

Actually the |\ORDINALstringnum| and suchlikes in the end will call some |\@ORDINAL@string| which is |\let|-ed to the language dependent implementation. And the language dépendent implemenation is supposed to do some |\edef| or some counter assignment of the numeric argument. Let us assume that it is an |\edef|:

|\edef\foo{\expandafter\expandafter\expandafter\the\value{somecounter}} |

will work, because the 1st expansion will make the |\foo| expanded definition become |\expandafter\the\csname c@somecounter\endcsname|, and the 2nd expansion will make it become |\the\c@somecounter|, so |\the\c@somecounter| will finaly be |\edef|-ed to the roman representation of counter |somecounter|. The same reasoning holds for assigning the content to a counter.

yes, that that is true but the same result is achieved with

\edef\foo{\the\value{somecounter}}

ie in both cases \foo will eventually hold the current value of the counter assuming \the is still the primitive \the and I repeat, if glossaries really would change that then it is not a package one can recommend to anybody.

vincentb1 commented 6 years ago

Actually, sorry for telling a unclear tale. glossaries does not change the definition of \the globally, but locally to a group, in order to use a different definition when producing an auxiliary file. The code of concern is the following one:

\newif\ifglswrallowprimitivemods
\glswrallowprimitivemodstrue
\newcommand*{\@@do@wrglossary}[1]{%
  \begingroup
    \let\orgthe\the
    \let\orgnumber\number
    \let\orgarabic\@arabic
    \let\orgromannumeral\romannumeral
    \let\orgalph\@alph
    \let\orgAlph\@Alph
    \let\orgRoman\@Roman
    \ifglswrallowprimitivemods
      \def\the##1{%
        \ifx##1\c@page \gls@numberpage\else\orgthe##1\fi}%
      \def\number##1{%
        \ifx##1\c@page \gls@numberpage\else\orgnumber##1\fi}%
    \fi
    \def\@arabic##1{%
      \ifx##1\c@page \gls@arabicpage\else\orgarabic##1\fi}%
    \def\romannumeral##1{%
      \ifx##1\c@page \gls@romanpage\else\orgromannumeral##1\fi}%
    \def\@Roman##1{%
      \ifx##1\c@page \gls@Romanpage\else\orgRoman##1\fi}%
    \def\@alph##1{%
      \ifx##1\c@page \gls@alphpage\else\orgalph##1\fi}%
    \def\@Alph##1{%
      \ifx##1\c@page \gls@Alphpage\else\orgAlph##1\fi}%
   \@wrglossarynumberhook
    \gls@disablepagerefexpansion
    \protected@xdef\@glslocref{\theglsentrycounter}%
  \endgroup
  \@gls@checkmkidxchars\@glslocref
  \expandafter\ifx\theHglsentrycounter\theglsentrycounter\relax
    \def\@glo@counterprefix{}%
  \else
    \protected@edef\@glsHlocref{\theHglsentrycounter}%
    \@gls@checkmkidxchars\@glsHlocref
    \edef\@do@gls@getcounterprefix{\noexpand\@gls@getcounterprefix
      {\@glslocref}{\@glsHlocref}%
    }%
    \@do@gls@getcounterprefix
  \fi
  \edef\@gls@label{\glsdetoklabel{#1}}%
  \@@do@@wrglossary
}
vincentb1 commented 6 years ago

The reason why glosaries does this, is that it needed to handle differently \the\c@page from any other \the… . Probably this code in glossaries can be improved, in order to be more robust to various counter formatting.

The idea is that if pages are formatted by \renewcommand\thepage{\ordinalstring{page}[m]}, then you would like to write to the auxiliary file something like \ordinalstringnum{\gls@numberpage}, and not \ordinalstringnum{} where Ω is the page number where the writing occurs, not the page number of the object referred to.

vincentb1 commented 6 years ago

For further reference, please have a look at issue #27. Nicola and I discussed at length this, and I did the treble \expandafter fix to fmtcount as some temporary trick : I had also proposed @nlct with some supposedly better code for glossaries, but she was somehow reluctant to change this part of the code, as it is not trivial, and any change may cause other troubles… However she 🐌 promised to have a look at my proposed code, when time allows 🙏 … 😃

nlct commented 6 years ago

Regarding glossaries. Consider the following:

\documentclass{article}
\usepackage[a4paper]{geometry}
\usepackage{lipsum}
\usepackage{fmtcount}
\usepackage[record]{glossaries-extra}
\GlsXtrLoadResources[src=example-glossaries-brief]
\renewcommand{\thepage}{\numberstring{page}}
\begin{document}
\lipsum[1-6]
\lipsum*[7] Reference \gls{lorem}.

\lipsum[8]
\printunsrtglossaries
\end{document}

This works fine with no hacks or messing with primitives. The indexing information is written to the .aux file using the usual \protected@write to ensure the page number is correct:

\glsxtr@record{lorem}{}{page}{glsnumberformat}{\numberstringnum {2}}

bib2gls (the indexing application in this case) has no problem with this as it has a simple TeX parser. Let's suppose I want to use xindy instead. If I use the package option esclocations=false, there are no hacks or messing with primitives. Here's the modified document:

\documentclass{article}
\usepackage[a4paper]{geometry}
\usepackage{lipsum}
\usepackage{fmtcount}
\usepackage[xindy,esclocations=false]{glossaries}
\makeglossaries
\loadglsentries{example-glossaries-brief}
\renewcommand{\thepage}{\numberstring{page}}
\begin{document}
\lipsum[1-6]
\lipsum*[7] Reference \gls{lorem}.

\lipsum[8]
\printglossaries
\end{document}

In this case, the indexing information is written to the .glo file as:

(indexentry :tkey (("lorem" "\\glossentry{lorem}") ) :locref "{}{\numberstringnum {2}}" :attr "pageglsnumberformat" )

Unfortunately within :locref backslash \ (and double-quote ") have a special meaning. In this case xindy doesn't see \numberstringnum as a (La)TeX control sequence but as \n followed by umberstringnum. The information must instead be written as:

(indexentry :tkey (("lorem" "\\glossentry{lorem}") ) :locref "{}{\\numberstringnum {2}}" :attr "pageglsnumberformat" )

Similarly, if the location happens to contain \" then it needs to be replaced with \\\".

The glossaries package has to both prevent the page number from being expanded too early (because of TeX's asynchronous output routine) but at the same time needs to fully expand \thepage and sanitize and escape xindy's special characters. So with esclocations=true, it temporarily redefines certain commands to replace things like \the\c@page, \@roman\c@page with special markers that don't expand and don't get sanitized and escaped, but when the write actually takes place they revert back to their normal behaviour and allow the page number to correctly expand.

I used \numberstring as an example in the glossaries user manual, and I raised the earlier issue when I noticed that the example stopped working because I wanted to find out why it broke. However, I've made a note in the glossaries user manual that the example doesn't work with the change, which may cause the page numbering to be one out.

I don't think that fmtcount needs to go out of its way to work in this context. It was an example to illustrate how to deal with custom page numbering systems. I doubt there are many real documents that use this type of page numbering, and, of those that do, I doubt there are many that need page locations indexed. If commands like \numberstring need to be changed to correct bugs, then they should be changed without reference to glossaries.

I have made some changes in v4.33 (which you can find in the documented code), but I'm not prepared to run the risk of breaking existing documents by introducing new hacks for the sake of rare documents formats.

vincentb1 commented 6 years ago

OK, then Following Nicola's statement:

I have made some changes in v4.33 (which you can find in the documented code), but I'm not prepared to run the risk of breaking existing documents by introducing new hacks for the sake of rare documents formats.

To my understanding the conclusion is that:

vincentb1 commented 6 years ago

Dear all, I have pushed to the repo a fix for this bug. Because the master branch is on the way of arabic langage development, I had to create a forked branch version_3.05+. Please feel free to get the code, install it and check whether you are happy with the fix.

# Define this envvar as the place where you want the package to be installed.
export TEXMF_INSTALL_DIR=/usr/share/texmf

temp_dir=$(mktemp /tmp/fmtcount-fix-bug33-XXXXXX)
rm "$temp_dir"
mkdir -p "$temp_dir"
cd "$temp_dir"
git clone -b version_3.05+ http://github.com/nlct/fmtcount.git 
cd fmtcount/dist
make install
# Cleanup
cd ../../..
rm -fr "./$(basename $temp_dir)"
vincentb1 commented 6 years ago

@FrankMittelbach BTW, do you know if there is some usual envvar to define where package updates are to be installed. Nicola and I use TEXMF_INSTALL_DIR, but may be there is a better choice.

vincentb1 commented 6 years ago

BTW, I noticed that Italian also suffers a similar issue. Probably a separate issue number would be better to create than follow on this one.

FrankMittelbach commented 6 years ago

Am 17.06.18 um 22:02 schrieb Vincent Belaïche:

BTW, I noticed that Italian also suffers a similar issue. Probably a separate issue number would be better to create than follow on this one.

well I would say the underlying issue is really the redef that happens in glossaries and is more than "fragile". So you are likely to encounter these and similar issues again if the root cause is not fixed/changed

FrankMittelbach commented 6 years ago

Am 17.06.18 um 22:00 schrieb Vincent Belaïche:

BTW, do you know if there is some usual envvar to define where package updates are to be installed. Nicola and I use |TEXMF_INSTALL_DIR|, but may be there is a better choice.

sorry, not really. for testing I would personally just use a local dir, for short/medium my local tree, bt then I'm not really very much into distrib setups. a quick check tells me that variable isn't even defined in my setup on the Mac

vincentb1 commented 6 years ago

Frank Mittelback wrote: well I would say the underlying issue is really the redef that happens in glossaries and is more than "fragile". So you are likely to encounter these and similar issues again if the root cause is not fixed/changed

Concerning the problem with Italian, I cannot say whether it was caused by my attempt to make fmtcount compatible with the unlikely event of somebody making some page number with fmtcount that would break some of the glossaries tricks, or whether the bug was already there.

Concerning your comment about glossaries, I must admit that I am not really aware of the details of the problem. If I am trying to recap Nicola's explanation, the root cause is that she wants to write to the auxiliary file some Xindy input instruction. So she has to do two things:

The problem is when doing T1 she does not want to expand the page number too early during the operation.

So the problem would be solved if there was some hook that would allow some execution to occur only immediately after page shipout and before any processing of the following page, would we register some macro in some hook to do some processing at the next page shipout, and then all the processing would occur at the right moment removing the needs for tricks to circumvent the asynchronous nature of writing to auxliary files, as this writing would be part of this processing.

vincentb1 commented 6 years ago

Just a very naive question @nlct: why expanding the page number too early makes it potentially wrong ? You mean that the text that is being typeset will be output in some page that is not the current one, but potentially the next one — or even the next-next one — so if you make an immediate write you are at a risk that you will write a page number N-1 to refer to some piece of text that is typeset on page N.

nlct commented 6 years ago

why expanding the page number too early makes it potentially wrong ?

Yes, that's due to TeX's asynchronous output routine. If it's expanded too early the page number will still refer to the previous page.

vincentb1 commented 6 years ago

So basically, the root cause for « tampering » with \the primitive is that the pdftex engine does not have any hook in order to do execution at page shipout. One can only use purely expandable macros , but not execute non expandable commands like \def or \advance… Maybe luatex has this, and the implementation could be made more robust when luatex engine is used…

FrankMittelbach commented 6 years ago

tested the fix and it looks fine with my use cases - thanks

FrankMittelbach commented 4 years ago

I guess I shouldn't have closed this. While the fix was done, it was never uploaded to CTAN so in the distributions the broken version is still current! Could you please upload 3.06 so that others can use the package?

It doesn't make much sense for me to give examples in TLC on this if it only works for me

nlct commented 4 years ago

@vincentb1 Are you able to send the latest stable version to CTAN?

vincentb1 commented 4 years ago

Yes I will do that. I think I did not do anything because there was still some confirmation pending about whether everybody is happy with the new macro names for HEXADecimal.

The last stable version is the HEAD of branch release_3.05+. Branch master has arabic language under work.

Version 3.05 on CTAN is 5aa96fdfd1e527d19d398d76a00eef441c20dd25 git sha1. Here is the history since then:

git checkout version_3.05+

git --no-pager log  -10 commit 9283a21bab77c9102543b13d6d0cd8261b793c21 Author: Vincent Belaïche vincentb1@users.sourceforge.net Date:   Wed Jun 27 22:40:31 2018 +0200

    mille → mil, somewhere.

    * trunk/fmtcount-manual.tex: Minor fix, mille → mil, in     documentation of French language.

commit 28b29dc79e70a46e59b0fad6bbde32a424eb3368 Author: Vincent Belaïche vincentb1@users.sourceforge.net Date:   Wed Jun 27 22:36:25 2018 +0200

    Rename Hexadecimal/Hexadecimalnum to HEXADecimal/HEXADecimalnum.

    * trunk/fmtcount-manual.tex: CheckSum.

    * trunk/fmtcount.sty: \Hexadecimal and \Hexadecimalnum renamed to     \HEXADecimal and \HEXADecimalnum. Old macro names are still     provided with a package warning indicating deprecation. Upper-case     and lower-case are code factorized.

    * trunk/test/sample-aaalph.tex: Rename Hexadecimal/Hexadecimalnum     to HEXADecimal/HEXADecimalnum wherever applicable.

    * trunk/test/reference/test-aaalph-l.html: Rename Hexadecimal to     HEXADecimal in caption.

commit 57b1049851b907cc895661b7f52ee9389b615562 Author: Vincent Belaïche vincentb1@users.sourceforge.net Date:   Tue Jun 26 08:14:15 2018 +0200

    HEXADECIMAL → HEXADecimal.

    Also added an explanatory section ``Macro naming conventions''.

commit b4a05b2cc0370ec94bd7854d3feed349c2b653fc Author: Vincent Belaïche vincentb1@users.sourceforge.net Date:   Sat Jun 23 20:39:42 2018 +0200

    Renaming Hexadecimal to HEXDECIMAL.

commit e261f4152b5a6f15974db50651c44077edc7ecbd Author: Vincent Belaïche vincentb1@users.sourceforge.net Date:   Sat Jun 23 19:49:39 2018 +0200

    Make the example using the section counter use the real command and current section number.

    Also fix the overfull hbox by inserting \discretionary{}{}{}     immediately after some of the \verb"\somecommand" examples.

commit 2be3b2267fecbf2e19f26a91c96176bd2f815bdf Author: Vincent Belaïche vincentb1@users.sourceforge.net Date:   Mon Jun 18 08:16:55 2018 +0200

    Bug #34

    See https://github.com/nlct/fmtcount/issues/34.

    I also replaced 3'' by\arabic{section}'' to be consistent with     the actual section number in which the example is given.

    I took the opportunity to thank also bug reporters.

commit 6293bff582b6c340cc25bf2c8ec475f2894e72b6 Author: Vincent Belaïche vincentb1@users.sourceforge.net Date:   Sun Jun 17 21:38:09 2018 +0200

    Fix bug #33.

    * trunk/fc-german.def (\@@ordinalstringgerman): Use \@orgargctr in     order to fix bug #33.

    * trunk/fmtcount-manual.tex: Update Checksum.

    * trunk/fmtcount.sty: Update version number.

commit fdaaf684b91d7bb2b18105761e3ac9165932f212 Author: Vincent Belaïche vincentb1@users.sourceforge.net Date:   Sun Jun 17 19:47:52 2018 +0200

    Add test to bug #33 with \ordinalstring broken for German language.

    See https://github.com/nlct/fmtcount/issues/33.

commit 07d77f6a79b6d3682fd89513cc4351c04d761eb6 Author: Vincent Belaïche vincentb1@users.sourceforge.net Date:   Sun Jun 17 19:40:16 2018 +0200

    Make call to htlatex in nonstopmode.

commit 5aa96fdfd1e527d19d398d76a00eef441c20dd25 Author: Vincent Belaïche vincent.b.1@hotmail.fr Date:   Tue Dec 26 22:09:41 2017 +0100

    Prepare 3.05 delivery to CTAN

    * dist/CHANGES: edit for delivery of 3.05 to CTAN.

    * trunk/fc-portuguese.def: docstrippable documentation improvement.

Le 24/01/2020 à 22:35, Nicola Talbot a écrit :

@vincentb1 https://github.com/vincentb1 Are you able to send the latest stable version to CTAN?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nlct/fmtcount/issues/33?email_source=notifications&email_token=ABGFEI7TCCZKB76IWDAXGL3Q7NNITA5CNFSM4FBJXPK2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJ4FGRA#issuecomment-578310980, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGFEI4DSRV3NDWXSQ3XRCTQ7NNITANCNFSM4FBJXPKQ.

vincentb1 commented 4 years ago

@nlct I prepared the following release note, are you happy with it:

Version 3.06:

* Fix French documentation « mille » → « mil » where applicable.

* Rename \Hexadecimal to \HEXADecimal, and document why. Same for
  \Hexadecimalnum renamed to \HEXADecimalnum. Keep support for old
  macro name with deprecation warning.

* Fix issue #34 (documentation concerning no more trailing space
  gobbling when the optional gender argument is omitted).

* Fix issue #33 (fix ordinalstringnum in German)
FrankMittelbach commented 4 years ago

I would certainly appreciate seeing those fixed.

vincentb1 commented 4 years ago

@FrankMittelbach As I was packaging the distribution, I ran the unitary test:

mkdir /tmp/fmt-test
cd /tmp/fmt-test
git clone https://github.com/nlct/fmtcount.git
cd fmtcount
git checkout version_3.05+
cd trunk/test
make clean all

and I get some warning/errors like that:

(test-crossref-uc+Hexadecimal.toc
! You can't use `\relax' after \the.
<recently read> \c@i 

l.1 ...upper-cased.}\HEXADecimalnum {1}}Sample}{1}

I'm forgetting what you said and using zero instead.

! You can't use `\relax' after \the.
<recently read> \c@i 

l.1 ...upper-cased.}\HEXADecimalnum {1}}Sample}{1}

I'm forgetting what you said and using zero instead.

! You can't use `\relax' after \the.
<recently read> \c@c 

l.1 ...upper-cased.}\HEXADecimalnum {1}}Sample}{1}

I'm forgetting what you said and using zero instead.

OK, this would certainly disapear if I replace \Hexadecimal by \HEXADecimal in the test, but it shows that adding the warning message breaks the backward compatibility:

\def\Hexadecimalnum{%
  \PackageWarning{fmtcount}{\string\Hexadecimalnum\space is deprecated, use \string\HEXADecimalnum\space
    instead. The \string\Hexadecimalnum\space control sequence name is confusing as it can mislead in thinking
    that only the 1st letter is upper-cased.}%
  \HEXADecimalnum}

Maybe just replacing \def\Hexadecimalnum by \protected\def\Hexadecimalnum could help… I will try that.

vincentb1 commented 4 years ago

Changing \def to \newrobustcmd* seems to help, however the test is still failed maybe because the warning prevents latexmk to make all the necessary runs to get the final output before comparing it to the reference output.

nlct commented 4 years ago

The counter commands need to expand when they are written to the .aux file, so I wouldn't put the warning in \Hexadecimal. It should be okay inside \Hexadecimalnum (provided it's robust) and if \Hexadecimal uses \Hexadecimalnum the warning will still be present.

Perhaps the warning could be configured so that it can be made into an informational message instead if it's necessary to use the old name for backward-compatibility.

vincentb1 commented 4 years ago

@nlct Definitely you are right, I had completely missed the point, I inerpreted the incorrect numbers in the output as if latexmk hadn't done all the compilation passes, but the root cause is that by protecting \Hexadecimal I got \Hexadecimal {section} everywhere in the .aux instead of the correct numbers.

vincentb1 commented 4 years ago

@nlct OK, I think I have found a solution, I just made the warning a robust command for it not to break anything, so the new code is as follows:

\newrobustcmd*\FC@Hexadecimal@warning{%
  \PackageWarning{fmtcount}{\string\Hexadecimal\space is deprecated, use \string\HEXADecimal\space
    instead. The \string\Hexadecimal\space control sequence name is confusing as it can mislead in thinking
    that only the 1st letter is upper-cased.}%
}
\def\Hexadecimal{%
  \FC@Hexadecimal@warning
  \HEXADecimal}
vincentb1 commented 4 years ago

Now, it seems that there are other regressions in the tests since the latest delivery to CTAN. I am looking at that, some of them are just not related to fmtcount so I need to confirm this, and I need to document it:

vincentb1 commented 4 years ago

@FrankMittelbach @nlct FYI, I have just uploaded v3.06.

FrankMittelbach commented 4 years ago

thanks. I checked at my end and the issues seem to be gone.