quarto-ext / fontawesome

Use Font Awesome icons in HTML and PDF documents.
https://quarto-ext.github.io/fontawesome/
MIT License
109 stars 10 forks source link

lua errors when trying to render revealjs with fontawesome extension #20

Closed morrisLuke closed 2 years ago

morrisLuke commented 2 years ago

Error running filter C:/Program Files/Quarto/share/filters/quarto-pre/quarto-pre.lua: _extensions/quarto-ext/fontawesome/fontawesome.lua:56: attempt to call a nil value (field 'is_format') stack traceback: ...ram Files/Quarto/share/filters/quarto-pre/quarto-pre.lua:2943: in function 'callShortcodeHandler' ...ram Files/Quarto/share/filters/quarto-pre/quarto-pre.lua:3024: in function 'transformShortcodeInlines'

reprex follows

---
title: "A"
subtitle: "B"
author: "C"
institute: "D"
format: 
  revealjs:
    theme:
      - moon
    self-contained: true
from: markdown+emoji
---

## [Thank you]{style="font-weight:500"}

{{< fa envelope >}} [address@email.net](mailto:address@email.net)

 

{{< fa brands github >}} [https://github.com](username)

 

{{< fa brands linkedin >}} [https://linkedin.com](handle)
morrisLuke commented 2 years ago

P.S. new to issue reporting, so let me know if anything else is needed

cscheid commented 2 years ago

Are you running a v1.2 prerelease version of quarto?

mcanouil commented 2 years ago

This is related to #19 and the issue comes from the upgrade performed based on Quarto 1.2 code change.

The solution would be to add tags/releases for the following commits:

With tags, it would be possible to use:

quarto install extension quarto-ext/fontawesome@v0.0.1

@cscheid Should the extensions in https://github.com/quarto-ext be following Quarto version, at least for Major and Minor, then the patch part can be used for fixes?

morrisLuke commented 2 years ago

ahhh.... Looks like I've got 1.1.251, the stable on quarto's get started page.

mcanouil commented 2 years ago

It's fine, but the extension is currently requiring the pre-release, but the latest tag is way too old to be used.

morrisLuke commented 2 years ago

Just updated to the 1.2.231 prerelease and things are resolved. Thanks for tolerating a noob

mcanouil commented 2 years ago

@morrisLuke The extension should not be asking users to use the pre-release / not stable version without a working solution with the stable version.

cscheid commented 2 years ago

@mcanouil No, what should have happened is that we should update the quarto version requirement for this package.

cscheid commented 2 years ago

@morrisLuke Glad you got it sorted out, but no need to apologize, this is clearly a bug on the advertised version requirement.

... Oh, I see the problem now, 🤦 . The issue is that 1.1 does not have the code that checks for version requirements. That's annoying. At least we are soon going to have a stable 1.2 version, and that should solve this problem.

mcanouil commented 2 years ago

Meanwhile, tag/release can help @cscheid 😉

cscheid commented 2 years ago

@mcanouil no, it cannot, not in this case! the issue is with quarto's version. Quarto 1.1 lacks the code that checks extension versions.

Releases will not help here because we are not ever going to have the HEAD of this repo expecting an old version of quarto. The right solution would be to backport the version checking code in 1.2 back to 1.1. But that doesn't solve the issue that the current installed base of RStudio etc still will have the old, non-checking code. And the new releases of RStudio will be getting a v1.2 release anyway, like I mentioned.

mcanouil commented 2 years ago

The idea is to tag/release 63f2000b431e0af5d8b374636de965ea5ec62383 with for example vO.O.2 Then one can quarto install extension quarto-ext/fontawesome@v0.0.2 if using Quarto < v1.2

cscheid commented 2 years ago

The extension support in 1.1 is poor enough that I truly would rather this not be easy to do. v1.2 is going to be out in a couple of weeks. I'm sorry, but this discussion is not particularly productive at this point.

mcanouil commented 2 years ago

It's fine 😉 I thought that the idea to allow to target tags when installing/using extensions/formats was for these situations among others. I am most likely missing some "long run" infos. 🤷‍♂️

koliajaykr commented 2 years ago

Just updated to the 1.2.231 prerelease and things are resolved. Thanks for tolerating a noob

Please, share the code you used to resolve this issue.

mcanouil commented 2 years ago

He updated Quarto to v1.2 pre-release: https://quarto.org/docs/download/

eitsupi commented 2 years ago

Hmmm, does this mean that there are no plans to release version 1.0.0? (I understand that quarto 1.1 does not check the required version, but isn't that unrelated to not creating a release or tag?)

morrisLuke commented 2 years ago

Same code as reprex, only change was downloading and installing pre-release of Quarto CLI

On Fri, Oct 21, 2022, 12:54 AM Ajay Koli @.***> wrote:

Just updated to the 1.2.231 prerelease and things are resolved. Thanks for tolerating a noob

Please, share the code you used to resolve this issue.

— Reply to this email directly, view it on GitHub https://github.com/quarto-ext/fontawesome/issues/20#issuecomment-1286596732, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC7GGHI3XPWZA4MSBMAJPTLWEJD3XANCNFSM6AAAAAARKPOLXE . You are receiving this because you were mentioned.Message ID: @.***>

cscheid commented 2 years ago

@eitsupi No, there will be a 1.0. I just don't want to encourage people to install an old version of quarto.

koliajaykr commented 2 years ago

He updated Quarto to v1.2 pre-release: quarto.org/docs/download

Thank you, updating Quarto to the latest version (pre-release in this case) helped to solve the error related to the fontawesome.

eitsupi commented 2 years ago

@cscheid Now that quarto 1.2 has been released, how about creating the 1.0.0 release of this repo?

cscheid commented 2 years ago

@eitsupi That's a great idea.