Raise different errors for empty label and empty hook

See latex3#1423.
muzimuzhi · Aug 9, 2024 · bd8616d · bd8616d
1 parent 413eb8d
commit bd8616d
Show file tree

Hide file tree

Showing 5 changed files with 231 additions and 63 deletions.
diff --git a/base/changes.txt b/base/changes.txt
@@ -6,6 +6,10 @@ to completeness or accuracy and it contains some references to files that are
 not part of the distribution.
 ================================================================================
 
+2024-08-09  Yukai Chou  <muzimuzhi@gmail.com>
+	* lthooks.dtx: (subsection{Parsing a label})
+	Raise different errors for empty label and empty hook (gh/1423)
+
 2024-07-27 Ulrike Fischer <Ulrike.Fischer@latex-project.org>
 	* lttagging.dtx: (subsection{Tagging support for table/tabular packages}): 
       Two sockets added to store and restore cell data when nesting tables to keep track

diff --git a/base/doc/ltnews40.tex b/base/doc/ltnews40.tex
@@ -234,10 +234,26 @@ \subsection{Avoiding keyval option clashes between classes and packages}
 \texttt{draft} option will be treated in the normal way by packages using
 keyvals, but they will ignore the \texttt{mode} option: it is effectively
 marked as \enquote{private} to the class.
+%
+\githubissue{1279}
 
 
+\subsection{Improved error raised by empty hook}
+
+When using the hook management, both hook and label names (if specified) 
+should be non-empty. Before empty hook and empty label both raised the
+same label-specific error.
+\begin{verbatim}
+! LaTeX hooks Error: Empty code label on line ....
+  Using 'top-level' instead.
+\end{verbatim}
+This has now been improved. Now empty hook raises
+\begin{verbatim}
+! LaTeX hooks Error: Empty hook on line ....
+\end{verbatim}
+%
+\githubissue{1423}
 
-\githubissue{1279}
 
 \section{Bug fixes}
 

diff --git a/base/lthooks.dtx b/base/lthooks.dtx
@@ -32,7 +32,7 @@
 %<*driver>
 % \fi
 \ProvidesFile{lthooks.dtx}
-             [2024/07/08 v1.1h LaTeX Kernel (hooks)]
+             [2024/08/09 v1.1i LaTeX Kernel (hooks)]
 % \iffalse
 %
 \documentclass{l3doc}
@@ -3298,21 +3298,26 @@
 %
 % \subsection{Parsing a label}
 %
-% \begin{macro}[EXP]{\@@_parse_label_default:n}
+% \begin{macro}[EXP]{\@@_parse_label_default:nN}
 %   This macro checks if a label was given (not \cs{c_novalue_tl}), and
 %   if so, tries to parse the label looking for a leading \verb|.| to
 %   replace by \cs{@@_currname_or_default:}.
+%   |#2| is a boolean representing if |#1| is a label name.
+% \changes{v1.1i}{2024/08/09}
+%         {Distinguish between empty label and empty hook (gh/1423)}
 %    \begin{macrocode}
-\cs_new:Npn \@@_parse_label_default:n #1
+\cs_new:Npn \@@_parse_label_default:nN #1#2
   {
     \tl_if_novalue:nTF {#1}
       { \@@_currname_or_default: }
-      { \tl_trim_spaces_apply:nN {#1} \@@_parse_dot_label:n }
+      { \tl_trim_spaces_apply:nN {#1} \@@_parse_dot_label:nN #2 }
   }
 %    \end{macrocode}
 % \end{macro}
 %
-% \begin{macro}[EXP]{\@@_parse_dot_label:n}
+% \begin{macro}[EXP]{\@@_parse_dot_label:nN}
+% \changes{v1.1i}{2024/08/09}
+%         {Distinguish between empty label and empty hook (gh/1423)}
 % \begin{macro}[EXP]{
 %     \@@_parse_dot_label:w,
 %     \@@_parse_dot_label_cleanup:w,
@@ -3325,12 +3330,15 @@
 %   If these requirements are fulfilled, the leading
 %   \verb|.| is replaced with \cs{@@_currname_or_default:}.  Otherwise
 %   the label is returned unchanged.
+%   |#2| is a boolean representing if |#1| is a label name.
 %    \begin{macrocode}
-\cs_new:Npn \@@_parse_dot_label:n #1
+\cs_new:Npn \@@_parse_dot_label:nN #1#2
   {
     \tl_if_empty:nTF {#1}
       {
-        \msg_expandable_error:nn { hooks } { empty-label }
+        \bool_if:NTF #2
+          { \msg_expandable_error:nn { hooks } { empty-label } }
+          { \msg_expandable_error:nn { hooks } { empty-hook } }
         \@@_currname_or_default:
       }
       {
@@ -3407,6 +3415,8 @@
 %   macro does the entire operation within a group so that csnames made
 %   by \cs{@@_make_name:n} are wiped off before continuing.  This means
 %   that this function cannot be used for \cs{hook_use:n}!
+% \changes{v1.1i}{2024/08/09}
+%         {Distinguish between empty label and empty hook (gh/1423)}
 %    \begin{macrocode}
 \cs_new_protected:Npn \@@_normalize_hook_args_aux:Nn #1 #2
   {
@@ -3420,24 +3430,24 @@
 \cs_new_protected:Npn \@@_normalize_hook_args:Nn #1 #2
   {
     \@@_normalize_hook_args_aux:Nn #1
-      { { \@@_parse_label_default:n {#2} } }
+      { { \@@_parse_label_default:nN {#2} \c_false_bool } }
   }
 \cs_new_protected:Npn \@@_normalize_hook_args:Nnn #1 #2 #3
   {
     \@@_normalize_hook_args_aux:Nn #1
       {
-        { \@@_parse_label_default:n {#2} }
-        { \@@_parse_label_default:n {#3} }
+        { \@@_parse_label_default:nN {#2} \c_false_bool }
+        { \@@_parse_label_default:nN {#3} \c_true_bool }
       }
   }
 \cs_new_protected:Npn \@@_normalize_hook_rule_args:Nnnnn #1 #2 #3 #4 #5
   {
     \@@_normalize_hook_args_aux:Nn #1
       {
-        { \@@_parse_label_default:n {#2} }
-        { \@@_parse_label_default:n {#3} }
+        { \@@_parse_label_default:nN {#2} \c_false_bool }
+        { \@@_parse_label_default:nN {#3} \c_true_bool }
         { \tl_trim_spaces:n {#4} }
-        { \@@_parse_label_default:n {#5} }
+        { \@@_parse_label_default:nN {#5} \c_true_bool }
       }
   }
 %    \end{macrocode}
@@ -7374,6 +7384,14 @@
   }
 %    \end{macrocode}
 %
+% \changes{v1.1i}{2024/08/09}{New message ``empty-hook'' (gh/1423).}
+%    \begin{macrocode}
+\msg_new:nnn { hooks } { empty-hook }
+  {
+    Empty~hook~name~\msg_line_context:.
+  }
+%    \end{macrocode}
+%
 %    \begin{macrocode}
 \msg_new:nnn { hooks } { no-default-label }
   {

diff --git a/base/testfiles-lthooks/lthooks-errors.lvt b/base/testfiles-lthooks/lthooks-errors.lvt
@@ -10,62 +10,84 @@
 \START
 
 
+\NewHook{xxx}
 \NewReversedHook{yyy}
+\NewHookWithArguments{zzz}{1}
 
-% activate-error
-\DisableGenericHook{yyy}
+% no message!
+\TEST{activate-error}{
+  \DisableGenericHook{yyy}
+}
 
-% activate-disabled
-\ActivateGenericHook{yyy}
+\TEST{Message "activate-disabled"}{
+  \ActivateGenericHook{yyy}
+}
 
-% hook-disabled
-\AddToHook{yyy}{error}
+\TEST{Message "hook-disabled"}{
+  \AddToHook{yyy}{error}
+}
 
-% empty-label
-\AddToHook{yyy}[]{error}
+\TEST{Message "empty-hook"}{
+  \AddToHook{}{error}
+  \AddToHookWithArguments{}{error '#1'}
+}
 
-% no-default-label
-\PushDefaultHookLabel{}
+\TEST{Message "empty-label"}{
+  \AddToHook{yyy}[]{error}
+  \AddToHookWithArguments{zzz}[]{error '#1'}
+}
+
+\TEST{Message "no-default-label"}{
+  \PushDefaultHookLabel{}
+}
 
 \AddToHook{xxx}[label1]{ foo1}
 \AddToHook{xxx}[label2]{ foo2}
 \AddToHook{xxx}[label3]{ foo2}
 \AddToHook{xxx}[label4]{ foo3}
 \AddToHook{xxx}[label5]{ foo5}
 
-% unknown-rule
-\DeclareHookRule {xxx} {label1} {unknown} {label2}
+\TEST{Message "empty-hook"}{
+  \DeclareHookRule{}{label1}{unrelated}{label2}
+}
 
-% labels-incompatible warning
-\DeclareHookRule {xxx} {label3} {incompatible-warning} {label4}
+\TEST{Message "empty-label"}{
+  \DeclareHookRule{xxx}{label1}{before}{}
+  \DeclareHookRule{xxx}{}      {after} {label2}
+}
 
-% labels-incompatible error
-\DeclareHookRule {xxx} {label1} {incompatible-error} {label5}
+\TEST{Message "unknown-rule"}{
+  \DeclareHookRule {xxx} {label1} {unknown} {label2}
+}
 
-\LogHook{xxx}
+\TEST{"labels-incompatible" warning and error}{
+  \DeclareHookRule {xxx} {label3} {incompatible-warning} {label4}
+  \DeclareHookRule {xxx} {label1} {incompatible-error} {label5}
+  \LogHook{xxx}
+}
 
 \makeatletter
 \@expl@@@filehook@file@pop@@
 
-% set-top-level
-\SetDefaultHookLabel{my-document}
-
-\AddToHook{begindocument}[top-level]{\TIMO\typeout{allowed}\OMIT}
-\PushDefaultHookLabel{my-document}
-  % top-level not allowed with other labels
-  % misused-top-level
-  \AddToHook{begindocument}[top-level]{not allowed}
-  % set-top-level
-  \SetDefaultHookLabel{top-level}
-\PopDefaultHookLabel
-
-% extra-pop-label
-\PopDefaultHookLabel
-
-% missing-pop-label
-\PushDefaultHookLabel{error}
-\PushDefaultHookLabel{should}
-\PushDefaultHookLabel{this}
+\TEST{Message "set-top-level" and "misused-top-level"}{
+  \SetDefaultHookLabel{my-document}
+  \AddToHook{begindocument}[top-level]{\TIMO\typeout{allowed}\OMIT}
+  \PushDefaultHookLabel{my-document}
+    % top-level not allowed with other labels
+    \AddToHook{begindocument}[top-level]{not allowed}
+    \SetDefaultHookLabel{top-level}
+  \PopDefaultHookLabel
+}
+
+\TEST{Message "extra-pop-label"}{
+  \PopDefaultHookLabel
+}
+
+\TEST{Message "missing-pop-label"}{
+  \PushDefaultHookLabel{error}
+  \PushDefaultHookLabel{should}
+  \PushDefaultHookLabel{this}
+}
 
 \OMIT
 \begin{document}