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

quarto.doc.include_text does not add text into revealjs #5666

Closed
5 tasks done
coatless opened this issue May 24, 2023 · 14 comments
Closed
5 tasks done

quarto.doc.include_text does not add text into revealjs #5666

coatless opened this issue May 24, 2023 · 14 comments
Assignees
Labels
bug Something isn't working pandoc-lua Issues with our Lua helper functions, filters, etc. in Pandoc upstream Bug is in upstream library
Milestone

Comments

@coatless
Copy link
Contributor

Bug description

If we have the following code inside of a Lua filter that is included as a quarto extension:

  -- Sample initialization
  local example_text = [[
    <script>
    // the hello world program
    document.write('Hello, World!');
    </script>
  ]]

  -- Insert the web initialization
  -- https://quarto.org/docs/extensions/lua-api.html#includes
  quarto.doc.include_text("in-header", example_text)

The quarto.doc.include_text() fails to include the text inside of the header under the revealjs format. However, it works under the html format option. Per the Lua documentation at:

https://quarto.org/docs/extensions/lua-api.html#includes

I feel like this should be a bug as the revealjs format is a version of an html document.

You can experiment and see more details on the quarto.doc.include_text() use with the filter here:

https://github.com/coatless/quarto-ext-revealjs-bug

Checklist

  • Please include a minimal, fully reproducible example in a single .qmd file? Please provide the whole file rather than the snippet you believe is causing the issue.
  • Please format your issue so it is easier for us to read the bug report.
  • Please document the RStudio IDE version you're running (if applicable), by providing the value displayed in the "About RStudio" main menu dialog?
  • Please document the operating system you're running. If on Linux, please provide the specific distribution.
  • Please provide the output of quarto check so we know which version of quarto and its dependencies you're running.
@coatless coatless added the bug Something isn't working label May 24, 2023
@cderv
Copy link
Collaborator

cderv commented May 25, 2023

Thanks a lot for the great report!

This is indeed an issue in our processing here:

-- process a text dependency, placing it in the specified location
local function processTextDependency(dependency, meta)
local rawText = dependency.content
local textLoc = rawText.location
if meta[textLoc] == nil then
meta[textLoc] = {}
end
meta[textLoc]:insert(pandoc.RawBlock(FORMAT, rawText.text))
end

It seems FORMAT should be tweaked to html when revealjs as it is seems to be no valid for rawBlock()

I think this is why it gets ignored at the end.

Maybe this is a Pandoc issue where using the revealjs format as same name as output should work maybe.

@dragonstyle we can definitely patch and handle but what is your thoughts about what Pandoc should be doing with pandoc.RawBlock()

@cderv cderv added the pandoc-lua Issues with our Lua helper functions, filters, etc. in Pandoc label May 25, 2023
@dragonstyle
Copy link
Collaborator

Woah that is a surprising and cool bug. I'm honestly not sure the best approach to this- overall specifying a raw block of the current format seems correct, but perhaps we need to provide per format overrides here.

Have you confirmed that manually tweaking the FORMAT to html resolves the issue?

@cderv
Copy link
Collaborator

cderv commented May 25, 2023

Yes this is surprising - I asked in Pandoc discuss to confirm what Pandoc expects https://groups.google.com/g/pandoc-discuss/c/2ffxC1uLh2M

Have you confirmed that manually tweaking the FORMAT to html resolves the issue?

Yes I have confirm. I wonder if this is not related to the Writer .hs file in pandoc. Revealjs is part of html writer. This would mean the same for beamer and other format that are sub format of another.

I didn't find the code in Pandoc code where under which contidion a rawBlock is ignored or not. cc @tarleb as you could be interested in this.

This is a current fix that covers slides mainly

diff --git a/src/resources/pandoc/datadir/init.lua b/src/resources/pandoc/datadir/init.lua
index 533c8b742..dac5cdcf1 100644
--- a/src/resources/pandoc/datadir/init.lua
+++ b/src/resources/pandoc/datadir/init.lua
@@ -1540,7 +1540,14 @@ local function processTextDependency(dependency, meta)
    if meta[textLoc] == nil then
       meta[textLoc] = {}
    end
-   meta[textLoc]:insert(pandoc.RawBlock(FORMAT, rawText.text))
+   local rawFormat = FORMAT
+   -- Some output formats requires raw blocks in another format
+   if format.isHtmlSlideOutput() then 
+      rawFormat = "html"
+   elseif format.isLatexOutput() then
+      rawFormat = "latex"
+   end
+   meta[textLoc]:insert(pandoc.RawBlock(rawFormat, rawText.text))
  end

@cderv
Copy link
Collaborator

cderv commented May 25, 2023

I didn't find the code in Pandoc code where under which contidion a rawBlock is ignored or not. cc @tarleb as you could be interested in this.

Ok found it.

https://github.com/jgm/pandoc/blob/4a6950200ff148b44b38c20f015100bb4d7e5033/src/Text/Pandoc/Writers/HTML.hs#L908-L920

This check for html only and not revealjs.
https://github.com/jgm/pandoc/blob/4a6950200ff148b44b38c20f015100bb4d7e5033/src/Text/Pandoc/Writers/HTML.hs#L1737-L1741

LaTeX handle beamer format ok
https://github.com/jgm/pandoc/blob/4a6950200ff148b44b38c20f015100bb4d7e5033/src/Text/Pandoc/Writers/LaTeX.hs#L467-L474

I would think this is an issue in Pandoc. We'll see what they answer.

@cderv
Copy link
Collaborator

cderv commented May 25, 2023

So @dragonstyle this will be an issue for sure with current pandoc with all HTML slide formats that are defined in the writer.hs

https://github.com/jgm/pandoc/blob/4a6950200ff148b44b38c20f015100bb4d7e5033/src/Text/Pandoc/Writers/HTML.hs#L26-L30

  writeS5,
  writeSlidy,
  writeSlideous,
  writeDZSlides,
  writeRevealJs,

We could patch only those, or look into all the writers to find all the possible format that are not correctly handle as raw block.

I believe epub would also expect html for example. So we may need a broad handling to provide per format overrides 🤔

@cscheid
Copy link
Collaborator

cscheid commented May 25, 2023

We can add a quarto-post filter pass that takes the "sub-html" raw blocks like revealjs and re-issues them as html. Would that solve the problem?

@cderv
Copy link
Collaborator

cderv commented May 25, 2023

if we do the association between FORMAT and raw block equivalent, why not solve that when creating the RawBlock directly ?
Is it easier to do it in one post processing Lua step later ?

In both case we need find all the mismatches and cover them .

Edit: Understood now having the broader picture of what you are suggesting after the meeting.

@cderv
Copy link
Collaborator

cderv commented May 30, 2023

Issue has been opened upstream, so it could be fixed in next Pandoc maybe

@cderv cderv added the upstream Bug is in upstream library label May 30, 2023
@dragonstyle dragonstyle added this to the v1.4 milestone Jul 7, 2023
@coatless
Copy link
Contributor Author

Submitted a PR to Pandoc (jgm/pandoc#9110) that covers just the HTML writer for the moment. We'll see what happens next.

@dlsun
Copy link

dlsun commented Sep 29, 2023

The upstream fix has been merged into Pandoc. So are we now just waiting for the next version of Pandoc to be released and then for Quarto to re-build with the new Pandoc?

@mcanouil
Copy link
Collaborator

Basically yes, but note that this will only be available in the 1.4 pre-release.

@coatless
Copy link
Contributor Author

Pandoc v3.1.9 was just released yesterday and included a fix for this.

https://github.com/jgm/pandoc/releases/tag/3.1.9

Can the included pandoc version in Quarto get upgraded this week? Unfortunately, I can't bump the version in an official way given dependencies are cached in an RStudio/Posit S3 bucket.

@cscheid
Copy link
Collaborator

cscheid commented Oct 29, 2023

Can the included pandoc version in Quarto get upgraded this week?

Hopefully. We'll try to get it done quickly (and certainly before 1.4 is out), but in general we can't promise a specific time.

@coatless
Copy link
Contributor Author

coatless commented Nov 8, 2023

I think this was just taken care of in PR #7419. So, I'm going to close out this issue and eagerly awaiting for the pre-release binaries to be updated.

Thank you for fast-tracking an update @cscheid!

@coatless coatless closed this as completed Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pandoc-lua Issues with our Lua helper functions, filters, etc. in Pandoc upstream Bug is in upstream library
Projects
None yet
Development

No branches or pull requests

6 participants