Skip to content
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

forcing hyperref commands even if hyperref is not detected #585

Closed
u-fischer opened this issue May 16, 2017 · 27 comments
Closed

forcing hyperref commands even if hyperref is not detected #585

u-fischer opened this issue May 16, 2017 · 27 comments

Comments

@u-fischer
Copy link

I have a AtEndPreamble racing problem. biblatex tries to setup the support for hyperref in \AtEndPreamble. This means that the following example biblatex doesn't detect that hyperref has been loaded and so disables all hyperlinks for cite command. biblatex also undefines \blx@mkhyperref and so it is difficult to reenable the support.

Would it be possible to have an option "hyperref=manual" that works e.g. in \ExecuteBibliographyOptions or something similar so that one could use it after loading hyperref to enable the links in such situations?


\documentclass{article}

\usepackage[hyperref=true,style=authoryear]{biblatex}

\AtEndPreamble{\usepackage{hyperref}}
\addbibresource{biblatex-examples.bib}

\begin{document}
\cite{doody}

\printbibliography
\end{document}
@moewew
Copy link
Collaborator

moewew commented May 17, 2017

Mhhh, the problem is that the option is also executed in \AtEndPreamble. If hyperref is to be used, the code that is executed immediately assumes hyperref is loaded and uses its commands. If at that point hyperref has not been loaded we run into trouble. We would have to delay the execution of these hooks further (the problematic code is \appto\blx@mkhyperref...).

Would it work for you to go with

\documentclass{article}

\usepackage{etoolbox}
\AtEndPreamble{\usepackage{hyperref}}

\usepackage[style=authoryear]{biblatex}

\addbibresource{biblatex-examples.bib}

\begin{document}
\cite{doody}

\printbibliography
\end{document}

@u-fischer
Copy link
Author

I know that the problem is that biblatex handles the option in \AtEndPreamble. That is why I wrote that I "have a \AtEndPreamble racing problem". I also know that I can avoid it by rearranging the package loading. The problem is that loading hyperref in a class or a package is getting very tricky if too many packages try to automatically detect hyperref and its settings in \AtEndPreamble or \AtBeginDocument.

For a class writer it would be really helpful if there were some option to tell biblatex: "Trust me, I will load hyperref with the following settings. So don't check it but simply set up your code for this scenario."

I don't think that biblatex needs to patch command of hyperref (or commands patched by hyperref) but if this were the case, then a command that the class writer should call after loading hyperref would be fine.

(It would be also helpful if hyperref could be splitted so that its core could be loaded earlier, but this is not a biblatex problem ;-))

@moewew
Copy link
Collaborator

moewew commented May 17, 2017

I could think of a force option where you have to execute \blx@mkhyperref (that would then not be undefined) yourself. Would that be OK?

@u-fischer
Copy link
Author

Perhaps "delayed" or "manual" would be a better name. Then this

\usepackage[hyperref=manual]{biblatex}
\usepackage{hyperref} 

or this

\usepackage[hyperref=manual]{biblatex}

would work like hyperref=false (it would be okay, if this requires a manual call to \blx@mknohyperref after hyperref) and

\usepackage[hyperref=manual]{biblatex}
\usepackage{hyperref}
\makeatletter\blx@mkhyperref\makeatother

works like hyperref=true.

If more cleaning up or additional settings after hyperref are needed, this is fine too.
Only the solution should be "official" and documentated.

@moewew
Copy link
Collaborator

moewew commented May 17, 2017

Let me see whether that can be cleanly implemented.

Of course if we make this official, the internal macros \blx@mkhyperref and \blx@mknohyperref would also have to be documented and made official if we require them to be executed manually.

We should probably wait for @plk to have a say.

@plk
Copy link
Owner

plk commented Aug 12, 2017

I am fine with this being an option - it just means making those two macros user-facing and documented and adding one package option. Do you want to do this @moewew?

@moewew
Copy link
Collaborator

moewew commented Aug 13, 2017

Let me have a look. When I looked at this the last time, it looked a bit messy.

@moewew
Copy link
Collaborator

moewew commented Aug 13, 2017

Does anyone have a good idea for the new user-facing 'manual' macros? I assume we shouldn't use the internal \blx@ names? Where should that be documented in the manual?

moewew added a commit to moewew/biblatex that referenced this issue Aug 13, 2017
@moewew
Copy link
Collaborator

moewew commented Aug 13, 2017

Have a look at https://github.com/moewew/biblatex/tree/forcehyperref, in particular moewew@49ca052

The name of the user-facing macro will probably have to change (I'm grateful for any suggestions), we could also consider defining it in the flow of the .sty and not just \AtEndPreamble (maybe in \AtEndOfPackage).
Documentation is still missing. But testing and comments would be appreciated.

@plk
Copy link
Owner

plk commented Aug 16, 2017

This looks ok but perhaps we can call the user-facing macros just \HyperrefOn/\HyperrefOff?

@moewew
Copy link
Collaborator

moewew commented Aug 16, 2017

Yes, maybe. But I feared that due to the generic name we might run into conflicts with other packages. \HyperrefOn sounds as though many packages could use this command.

@plk
Copy link
Owner

plk commented Sep 5, 2017

Fair enough - @u-fischer - can you comment so we can close this?

@plk
Copy link
Owner

plk commented Oct 28, 2017

@u-fischer - can you check this? We are aiming for a release shortly.

@u-fischer
Copy link
Author

I will check tomorrow. (Sorry about the delay, I saw the message on the 5. september but got ill that day and forgot).

@u-fischer
Copy link
Author

I think there is no real test version yet, so I only looked at the code. If I got it right, then I can do \usepackage[hyperref=manual]{biblatex} and later I must use one of \BiblatexManualHyperrefOn or \BiblatexManualHyperrefOff where "later" means currently in \AtEndPreamble as the test for the option is there.

Imho it will work as wanted in my case but that one should test / check the following questions:

  1. As \blx@mkhyperref is undef after use, this is perhaps also necessary for the user commands (what happens if \BiblatexManualHyperrefOn is used twice? Or is both commands are used?)

  2. The \ifnum\blx@hyperref=\thr@@ \let\BiblatexManualHyperrefOn\blx@mkhyperref \let\BiblatexManualHyperrefOff\blx@mknohyperref branch could perhaps be moved outside the \AtEndPreamble. There is no real need to delay it so long. And perhaps some use pops up where it is needed to activate the hyperref support earlier.

@moewew
Copy link
Collaborator

moewew commented Oct 29, 2017

Thank you very much for your feedback.

Indeed the changes have not been merged yet so can't really be tested. For easier testing I created a gist with a drop-in replacement for biblatex.sty https://gist.github.com/moewew/86d34f18e0a8342a236b15bd9545c407/. But you can also have a look at https://github.com/moewew/biblatex/tree/forcehyperref

I have taken up your suggestion 1 there already.

2 also sounds good, but I can't think of a proper place to put it. It would have to go directly into the \DeclareBibliographyOption[boolean]{hyperref}[true] I think. So that would require restructuring of that option.

Any comment and code review is appreciated.

@u-fischer
Copy link
Author

I'm not sure that 2 is so complicated. Can't one do

   \def\BiblatexManualHyperrefOn{\blx@mkhyperref  ....undef  code for \blx@mkhyperref etc..}

at an arbitrary place in biblatex and in the \AtEndPreamble do nothing if \blx@hyperref=\thr@@?

@moewew
Copy link
Collaborator

moewew commented Oct 30, 2017

But that would make \BiblatexManualHyperrefOn potentially available for all settings of the hyperref option. I would like to only make it active for hyperref=manual.

@u-fischer
Copy link
Author

I don't really think that there is much point to hide the command. After all \blx@mkhyperref is defined, and calling it in the wrong place would do the same harm that a wrongly called \BiblatexManualHyperrefOn.

But you could add the test \ifnum\blx@hyperref=\thr@@ inside \BiblatexManualHyperrefOn. You could also as some protection give the command a bit more internal name, e.g.\biblatex@manualhyperrefon (but it should have a prefix that distinguish it from real internal commands).

@moewew
Copy link
Collaborator

moewew commented Oct 31, 2017

You are right. I didn't want to define \BiblatexManualHyperrefOn in all cases, because I feared people would start to randomly use it hoping to turn on hyperref interaction with it (just like many people use hyperref=true which adds little value over the default hyperref=auto).

We could make \BiblatexManualHyperrefOn internal, but I don't want to do that. It has to be an officially documented command and I'd rather not use an @ then (I don't think any documented macro contains an @ in biblatex, I'd rather not break with that tradition).

I have pushed moewew@b0df2fd and moewew@37cd212 to https://github.com/moewew/biblatex/tree/forcehyperref The drop-in replacement for 3.7's biblatex.sty on gist is also updated https://gist.github.com/moewew/86d34f18e0a8342a236b15bd9545c407

With that commit \BiblatexManualHyperrefOn and \BiblatexManualHyperrefOff are defined at all times, but they check if hyperref=manual is turned on internally and only act then. If the commands are used incorrectly, a warning is emitted. There is naturally no warning if none of the commands is used, but that will produce errors later on.

@moewew
Copy link
Collaborator

moewew commented Oct 31, 2017

Blast, I forgot: @plk At the moment \BiblatexManualHyperrefOn and \BiblatexManualHyperrefOff are only mentioned in the manual in the section about hyperref. There was no sensible place to put an explanation for the two commands. Is what we have now enough, or do I need to explain \BiblatexManualHyperrefOn and \BiblatexManualHyperrefOff in more detail in another section, if so, where would be a good place?

@plk
Copy link
Owner

plk commented Oct 31, 2017

I don't see the need to explain more - it's a very specific command.

@moewew
Copy link
Collaborator

moewew commented Oct 31, 2017

Very good.

I'll wait for @u-fischer to respond and will merge if there are no further changes needed.

@moewew
Copy link
Collaborator

moewew commented Nov 1, 2017

@u-fischer The current status can be found in dev...moewew:forcehyperref, https://gist.github.com/moewew/86d34f18e0a8342a236b15bd9545c407. Comments and testing would be appreciated. I hope I could take up most of your suggestions.

@u-fischer
Copy link
Author

Imho it looks good. You could add in the docu e.g. at the end something like "This option should be only needed by package writers to solve special problems with the loading order of packages."

moewew added a commit to moewew/biblatex that referenced this issue Nov 1, 2017
Add hyperref=manual option, see plk#585.
@moewew
Copy link
Collaborator

moewew commented Nov 1, 2017

Thank you for your comment and suggestion. I added a short line in the spirit of your suggestion to the docs.

The changes are merged now in e423c75

@moewew
Copy link
Collaborator

moewew commented Nov 2, 2017

Closing this for now. If anything relevant comes up again, please comment here and we can reopen this.

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

No branches or pull requests

3 participants