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

Also record labels as metadata for longtables, when requested in lxRDFa #862

Merged
merged 1 commit into from Aug 24, 2017

Conversation

Projects
None yet
2 participants
@dginev
Collaborator

dginev commented Aug 4, 2017

This PR is a follow-up to #596 and I would appreciate a quick review & merge, if possible.

It turns out that my generic recording of \label as metadata did not extend to packages redefining it, such as longtable.

The problem there is that longtable redefines (via Let) the \label macro on each environment start, which makes it impossible to maintain the RDFa hook I introduced globally in the preamble. However, I can hook into the lower level \@longtable@label macro, and it all works great again!

Example test being:

\documentclass{article}
\usepackage{longtable}
\usepackage[labels]{lxRDFa}
\begin{document}

\begin{longtable}[]{@{}lccc@{}}
1 & 2 & 3 & 4 \\

\caption{\label{table:long} caption}
\end{longtable}

I should be able to refer to Table \ref{table:long}.

\end{document}

with the expected snippet of metadata produced:

<figcaption class="ltx_caption" property="dcterms:alternative" content="table:long">

Tested locally and all works well.

@brucemiller

This comment has been minimized.

Owner

brucemiller commented Aug 8, 2017

Hmm... I guess the original option "labels" was a patch from you, and I accepted it? But...

It kinda seems the wrong way to go; since we'd conceivably need to mung every package that plays around with \label. Shouldn't it be more correct to use Tag (or something similar) to check for any labels attributes, and add RDF when appropriate?

@dginev

This comment has been minimized.

Collaborator

dginev commented Aug 8, 2017

But then I would need to add that rule to all heading-like tags? Or attach it to * which would hurt performance badly.

I double-checked the bindings, i think this is the only case one needs to make a special exception. All other table packages, such as tabularx or tabulary keep using the standard label.

Happy to backtrack when it becomes unmanageable (which I hope will never happen)?

@brucemiller

This comment has been minimized.

Owner

brucemiller commented Aug 23, 2017

Have you actually tried whether it really hurts performance? I'd bet it's not as bas as you think (but might be).

@dginev

This comment has been minimized.

Collaborator

dginev commented Aug 23, 2017

I mean I have a perfectly safe 3-line PR that does everything one needs with no issue...

@dginev

This comment has been minimized.

Collaborator

dginev commented Aug 24, 2017

Maybe a compromise - if we merge here as-is, I promise to rework this using Tag the next time I find a missing label macro that isn't supported.

@brucemiller

This comment has been minimized.

Owner

brucemiller commented Aug 24, 2017

@dginev

This comment has been minimized.

Collaborator

dginev commented Aug 24, 2017

XS? I love XS! I'm sure it will work out in the end :) Thanks for merging 👍

@brucemiller

This comment has been minimized.

Owner

brucemiller commented Aug 24, 2017

OK, I'm convinced!

@brucemiller brucemiller reopened this Aug 24, 2017

@brucemiller brucemiller merged commit 93421a3 into brucemiller:master Aug 24, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment