New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hyperref redefinition of `\contentsline` adds one level of braces if `linktoc=page` #65

Open
jfbu opened this Issue Jul 4, 2018 · 8 comments

Comments

Projects
None yet
3 participants
@jfbu
Contributor

jfbu commented Jul 4, 2018

In the definition of \contentsline, there is the following

8636 \def\Hy@temp{#3}%
8637 \ifx\Hy@temp\ltx@empty
8638 \csname l@#1\endcsname{#2}{#3}%
8639 \else
8640 \csname l@#1\endcsname{{#2}}{%
8641 \hyper@linkstart{link}{\Hy@tocdestname}{#3}\hyper@linkend
8642 }%
8643 \fi

This is triggered by linktoc=page option.

The issue (if there is one) is the {{#2}} at code line 8640. This is the only location in \contentsline definition where #2 ends up wrapped this way.

This makes it harder to see if #2 starts with \numberline or variants (depending on document class). In fact this causes a bug in etoc (a bug which was there since initial release of etoc), which does not detect the \numberline and leaves it in \etocname, thus it delayed expansion (which depends on then current setting of \@tempdima) causes varying effect. This surfaced in an issue in yathesis.

Is there any reason for the added braces?

@jfbu

This comment has been minimized.

Contributor

jfbu commented Jul 4, 2018

Sorry for my initial posting with empty description, I hit the return key accidentally on keyboard.

@jfbu

This comment has been minimized.

Contributor

jfbu commented Jul 4, 2018

Pasting here the code lines from repo (my line numbers in description were pasted from hyperref.pdf)

hyperref/hyperref.dtx

Lines 12557 to 12564 in de4100b

\def\Hy@temp{#3}%
\ifx\Hy@temp\ltx@empty
\csname l@#1\endcsname{#2}{#3}%
\else
\csname l@#1\endcsname{{#2}}{%
\hyper@linkstart{link}{\Hy@tocdestname}{#3}\hyper@linkend
}%
\fi

@davidcarlisle

This comment has been minimized.

Contributor

davidcarlisle commented Jul 4, 2018

I can't think of a reason (nor is one documented) although test coverage is rather sketchy...

@touhami

This comment has been minimized.

touhami commented Jul 5, 2018

I guess, because in the two other cases (section, all) #2 is inside a group by \hyper@linkstart...\hyper@linkend.

@jfbu

This comment has been minimized.

Contributor

jfbu commented Jul 17, 2018

@touhami actually \l@section in article class adds it own group scope (cf my comment to the tex.sx question linked-to next), which allows some abuse like \section{A \color{red} B} although this causes problems with hyperref (see this tex.sx question). Checking now:

  • article class: \begingroup/\endgroup for \l@section, {...} for \@dottedtocline.

  • book class: \begingroup/\endgroup for \l@chapter, {...} for \@dottedtocline,

but the page number will be contaminated by the color for article/section and book/chapter but not in other cases due to a \normalcolor. It could be that hyperref's intent could have been to correct this, but the example of \color is dubious due to the aspects recalled in the tex.sx question linked to above, and \textcolor should be used, for which no extra group is needed.

@jfbu

This comment has been minimized.

Contributor

jfbu commented Jul 17, 2018

@touhami besides, at

hyperref/hyperref.dtx

Lines 12549 to 12552 in de4100b

\else
\ifcase\Hy@linktoc % none
\csname l@#1\endcsname{#2}{#3}%
\or % section

there is no hyperlink added, and despite this the #2 is not enhanced with braces.

@jfbu

This comment has been minimized.

Contributor

jfbu commented Jul 17, 2018

BTW, regardnig etoc I added a workaround at release v1.08p [2018/07/04]

@touhami

This comment has been minimized.

touhami commented Jul 18, 2018

@jfbu as i said this is just a guess (sorry). In fact, what i mean is the two other cases (section and all) that's mean L12552->12556 for section and L12565->12571 for all. In these two cases #2 is inside a group by \hyper@linkstart...\hyper@linkend, so may be to keep things in the same way the explicits braces were added in the case of page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment