TEIC / Stylesheets

TEI XSL Stylesheets
231 stars 124 forks source link

Allow the `<ref>` elements to get processed: #542

Closed sydb closed 2 years ago

sydb commented 2 years ago

Stylesheets group, with big thanks to @hcayless, realized a) these things (contents of <hi> in intermediate Lite stage that contain the <ref>s that were not working) were being processed with <value-of>, not <apply-templates>; and b) furthermore, the templates for <ref> would have been processed if these templates for <hi> did not exist, and since all they were doing in 2 of 3 cases was adding boldface we did not like (and failing to process the <ref>s), we just nuked 2 of them and fixed the 3rd (which also adds the pointy brackets for element names).

We tested this once, in a Docker container, and it seemed to work. If one of either @peterstadler, @martindholmes, or @npcole can reproduce that this works (by generating PDF and looking at the list of macro names in section 1.4.1 “Standard Content Models” on page 16 of the PDF, and ensuring they are working links, and also looking around elsewhere in the PDF Guidelines and making sure we didn’t break anything else), then I think one of the other two can just merge it.

martindholmes commented 2 years ago

@sydb I just checked out the branch and ran a build (make clean pdf), and I do indeed see links in the 1.4.1 section where they did not exist before. I can't find any other changes or regressions. I think this is working as intended. I think if either @peterstadler or @npcole also see the same result, they should merge the PR.

peterstadler commented 2 years ago

I fear the change is too simple. Compare the current dev output current dev output and the output of this branch branch sydb_issue_537

There's at least two issues:

  1. The square brackets are missing to signal to LaTeX the custom item label (in a LaTeX list)
  2. the angle brackets are not part of the hyperlink (which they usually are)
peterstadler commented 2 years ago

A third issue is the removal of the bold face. Was that done on purpose?

I just reminded myself to read the original message and found the answer there :)

sydb commented 2 years ago

I think @peterstadler raises absolutely valid concerns that should be addressed. That said, I think the output from this branch is still better than that from the current released branch. So I would be inclined to merge this and generate a new ticket to fix the LaTeX square bracket and pointy-brackets-in-link issues. (And perhaps have a larger discussion as to whether the boldface should stay or go.) But if others disagree, we can happily just not merge this puppy now and fix those issues here.

peterstadler commented 2 years ago

I'm already having a look and hope to provide the required fixes asap

peterstadler commented 2 years ago

Sorry, I might only get at it this weekend. My idea is to keep the old switch cases, remove the \textbf since you don't like it, and call makeInternalLink (instead of simply value-of). I think there's not much markup that would be missed (by not calling apply-templates) but the links could be added (including the angle brackets for element names). If someone wants to try this, please feel free!

peterstadler commented 2 years ago

Here's the updated output: Bildschirmfoto 2022-04-16 um 09 15 53 Bildschirmfoto 2022-04-16 um 09 16 37

Links are there, indent level is right. A quick diff of the tex files revealed only intended changes, e.g. changing from

\item [\textbf{<s>}] (s-unit) contains a sentence-like division of a text.

to

\item [{\hyperref[TEI.s]{<s>}}] (s-unit) contains a sentence-like division of a text.

Yet, I'm still not sure whether I like the un-bolding of those labels.

peterstadler commented 2 years ago

May I ask @martindholmes and @npcole to review. I think this is an improvement and could go into the release

martindholmes commented 2 years ago

@peterstadler Definitely an improvement, and should go into this release. I say yes to merge.

sydb commented 2 years ago

Will test & merge as soon as set with release-blocker …