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

Patching footnotes fails with polyglossia and Hebrew. #412

Closed
dcpurton opened this issue Apr 21, 2016 · 13 comments
Closed

Patching footnotes fails with polyglossia and Hebrew. #412

dcpurton opened this issue Apr 21, 2016 · 13 comments

Comments

@dcpurton
Copy link
Contributor

The following MWE gives a warning about patching footnotes.

\documentclass{article}
\usepackage{fontspec}
\usepackage{polyglossia}
\usepackage{csquotes}
\usepackage[backend=biber]{biblatex}
\setdefaultlanguage{english}
\setotherlanguage{hebrew}
\setmainfont{Linux Libertine O}
\begin{document}
Paragraph Hebrew:

\begin{hebrew}
  בְּרֵאשִׁית, בָּרָא אֱלֹהִים, אֵת הַשָּׁמַיִם, וְאֵת הָאָרֶץ.
\end{hebrew}

BIDI Hebrew: \texthebrew{בראשית}.
\end{document}
@plk
Copy link
Owner

plk commented Apr 21, 2016

Hmm, @josephwright - any idea? Looks like hebrew isn't that well supported in polyglossia anyway but that's a confusing message nevertheless.

@dcpurton
Copy link
Contributor Author

It looks like it affects all RTL languages (arabic, farsi, urdu, etc.)

You can also trigger the warning by loading the bidi package without polyglossia.

@moewew
Copy link
Collaborator

moewew commented Apr 22, 2016

Maybe this is also at least tangentially related to #157.

On TeX.SX there is Biblatex authoryear-icomp doesn't work as expected with polyglossia hebrew support on.

@plk plk closed this as completed in 65ab8fa Apr 25, 2017
plk added a commit that referenced this issue Apr 25, 2017
Patch footnotes when bidi is loaded (fixes #412)
@dcpurton
Copy link
Contributor Author

@plk, I might have been a bit hasty. This certainly fixes some problems (like dcpurton/biblatex-sbl#49), but the ibidtracker still doesn't seem to behave correctly. I'll keep looking into it.

@dcpurton
Copy link
Contributor Author

Got it, I think! \toggletrue{blx@footnote} needs to be inside a group, otherwise it remains true after the footnote. Sorry! I'll create another pull request.

@plk
Copy link
Owner

plk commented Apr 25, 2017

Ok, please base it on the last request as it's already merged ...

@dcpurton
Copy link
Contributor Author

Will do. But I'm having trouble with the macro patch :(.

The bidi macro looks like this:

\long\def\@footnotetext#1{%
  \footdir@write
  \footdir@fntext{\bidi@footdir@footnote}%
  \stepcounter{footdir@label}%
  \footdir@fntext{\bidi@footdir@footnote}%
  \footdir@toks{#1}%
  \footdir@toks\expandafter
    {\the\expandafter\footdir@toks
     \expandafter \zref@labelbyprops
     \expandafter{\thefootdir@label}{abspage}}%
     \expandafter\footdir@ORG@bidi@footnotetext
     \expandafter{\the\footdir@toks}}

The \toggletrue{blx@footnote} needs to be inserted before the #1. Except using #1 as a hook in patchcmd fails (even if I don't use docsvlist).

Any ideas?

@plk
Copy link
Owner

plk commented Apr 25, 2017

@moewew ?

@dcpurton
Copy link
Contributor Author

@dcpurton
Copy link
Contributor Author

dcpurton commented Apr 26, 2017

I've come up with what I think is a suitable fix. I'll do a little more testing this time... Sorry!

This is the current proposed patch. Feel free to comment.

diff --git a/tex/latex/biblatex/biblatex.sty b/tex/latex/biblatex/biblatex.sty
index 398136c..33f0027 100644
--- a/tex/latex/biblatex/biblatex.sty
+++ b/tex/latex/biblatex/biblatex.sty
@@ -75,6 +75,25 @@
 
 %% Compatibility
 
+\catcode`\#=12
+\def\blx@patchbidifootnotes{%
+  \patchcmd\@footnotetext
+    {#1}
+    {\toggletrue{blx@footnote}#1}
+    {\togglefalse{blx@tempa}}
+    {}
+  \patchcmd\@LTRfootnotetext
+    {#1}
+    {\toggletrue{blx@footnote}#1}
+    {\togglefalse{blx@tempa}}
+    {}
+  \patchcmd\@RTLfootnotetext
+    {#1}
+    {\toggletrue{blx@footnote}#1}
+    {\togglefalse{blx@tempa}}
+    {}}
+\catcode`\#=6
+
 \AtEndPreamble{%
   \def\do#1{%
     \@ifpackageloaded{#1}
@@ -195,16 +214,8 @@
        {\togglefalse{blx@tempa}}
        {}}
     {}%
-  \@ifpackageloaded{bidi}{%   bidi
-    \def\do#1{
-      \pretocmd#1
-        {\toggletrue{blx@footnote}}
-        {\togglefalse{blx@tempa}}
-        {}}%
-    \docsvlist{%
-      \@footnotetext,
-      \@LTRfootnotetext,
-      \@RTLfootnotetext}}%
+  \@ifpackageloaded{bidi}%    bidi
+    {\blx@patchbidifootnotes}
     {}%
   \@ifclassloaded{memoir}
     {\def\do#1{%

@moewew
Copy link
Collaborator

moewew commented Apr 26, 2017

Thank you for the patch.

Maybe it is safer to wrap the \catcode`\#=12 into \begingroup...\endgroup that saves us the reset (\catcode`\#=6). Precedence for that is in biblatex.sty already.

@dcpurton
Copy link
Contributor Author

I thought of that. But then you need to use \global\togglefalse{blx@tempa} and I wasn't certain if that would have undesired side effects. It's probably safe though. I'll check.

@dcpurton
Copy link
Contributor Author

dcpurton commented Apr 26, 2017

Actually... Of course it won't... Brain fail. I'll use the \begingroup

dcpurton added a commit to dcpurton/biblatex that referenced this issue Apr 29, 2017
    - Commit 65ab8fa did not properly
      deal with this
    - fixes plk#157 and plk#412
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