gusbrs / zref-clever

Clever LaTeX cross-references based on zref
LaTeX Project Public License v1.3c
11 stars 4 forks source link

Load hyperref before any `\newtcolorbox` #14

Closed muzimuzhi closed 1 year ago

muzimuzhi commented 1 year ago

From the testfile one can see the previous hyperref target names like tcb@cnt@pabox.1 are not unique throughout the document, for example the first pabox in section 1 and section 2 will use the same target name.

Loading hyperref before any \newtcolorbox and friends fixes this problem. Now the hyperref target are named like tcb@cnt@pabox.1.1 (<box name>.<sec num>.<box num>), which reflect the setting number within=section.

See a related issue in https://github.com/T-F-S/tcolorbox/issues/135.

PS: Since \tcbuselibrary may load packages as well, just in case, loading of hyperref is moved after \tcbuselibrary.

gusbrs commented 1 year ago

Hi @muzimuzhi , thank you. And you're right of course. I also have the habit of keeping setup for a package close to the package call, but that sometimes bites us. :-)

I'll merge, of course, and I'll take a look at the failing test, which considering your care in the PR is probably from the xetex engine. Btw, do you mind if, in the process, I drop the "section 2" in the test file? I usually keep the how-tos regression tests in sync with the actual How-to in the manual. I do get you were trying to make your point, and you did, but the effect of the change is already clear in the first section's anchors. WDYT?

muzimuzhi commented 1 year ago

Btw, do you mind if, in the process, I drop the "section 2" in the test file?

I'll delete it, update the testfiles and push a new commit.

BTW, a "section 2" test allows me to check the output by clicking links in the output PDF. 😄

muzimuzhi commented 1 year ago

Commits and changes are messed. Will tidy them.

muzimuzhi commented 1 year ago

Commits and changes are messed. Will tidy them.

done. Sorry it results in a force push.

gusbrs commented 1 year ago

Merged. And I'll cut a release because, though this is a small change, I have nothing lined up at the moment, and I also see no reason to hold this for an indefinite amount of time (and keep spreading a bad practice in the meantime).

Thank you very much @muzimuzhi !

BTW, a "section 2" test allows me to check the output by clicking links in the output PDF. :smile:

Well, yes, the proof of the pudding is in the eating. :smile: With time, I'm getting a little better at reading these logs, but whenever I create a new test file what I check first and foremost is the PDF. And I suspect I'll keep doing this for a long time. Sometimes, I'll create a dummy extra end page so that there's enough room to scroll, and I can "be sure the anchor is right". :wink:

muzimuzhi commented 1 year ago

Sometimes, I'll create a dummy extra end page so that there's enough room to scroll, and I can "be sure the anchor is right". 😉

+1 🎉