slatex / LaTeXML-Plugin-sTeX

A LaTeXML Plugin for Semantic LaTeX (sTeX)
LaTeX Project Public License v1.3c
2 stars 3 forks source link

Fix: avoid casting Locator's to strings, use ->toAttribute instead. #121

Closed dominique-unruh closed 5 years ago

dominique-unruh commented 5 years ago

(Directly casting to string gives garbled attributes like "LaTeXML::Common::Locator=HASH(...)")

dginev commented 5 years ago

This needs an extra sanity check that the result of ->getProperty is defined before calling ->toAttribute, or it would incur a Fatal error in the cases where no locator is available. You could for example write that as:

  if ($trailer) {
    if (my $locator = $trailer->getLocator) {
      $trailer = $locator->toAttribute; } }
dominique-unruh commented 5 years ago

Thanks. I had assumed that $trailer->getLocator is an always defined function (i.e., the "if ($trailer)" in the next line is only to catch the case where $trailer is undefined from the start).

I think there might be many more places where ->toAttribute should be added (for example, I see the spurious Info:provides theory: ::Common::Locator=HASH(0x562cac692?test in my output), but this one is the only one that lead to an error so far.

dginev commented 5 years ago

I've been away from the project space for quite some time, and was just doing quick reviews, but since you made two PRs I tried to investigate why the Travis CI builds now fail, which lead me down a rather deep rabbit hole...

Long-story short, some dust had settled w.r.t to the Travis deploys, which I just cleaned up in #122 . I incorporated your upgrades from this PR (and extended them to another routine of interest), so I will close this PR and merge there. I recommend rebasing your master to upstream, and we can continue removing what's rest of the cruft.

Since I recall some contributors may be new to GH, what you'd like to do from a terminal is:

git checkout master
git pull --rebase
git checkout your-local-dev-branch
git rebase master

to sync any local feature branches with master updates.