quarto-dev / quarto-cli

Open-source scientific and technical publishing system built on Pandoc.
https://quarto.org
Other
3.99k stars 328 forks source link

nil protection for `caption_long` for lightbox, especially used in manuscript type #9477

Closed juliantao closed 7 months ago

juliantao commented 7 months ago

Bug description

Since commit bef2e84, when a caption is missing in a multi-figure environment of a manuscript file, the preview/render function is no longer working. The following error is given:

Error running filter /home/julian/quarto-cli/src/resources/filters/main.lua:
...n/quarto-cli/src/resources/filters/./layout/lightbox.lua:147: attempt to index a ni
l value (field 'caption_long')
stack traceback:
        [C]: in ?
        [C]: in method 'walk'
        ...n/quarto-cli/src/resources/filters/./ast/customnodes.lua:76: in function <.
..n/quarto-cli/src/resources/filters/./ast/customnodes.lua:65>
        (...tail calls...)
        ...n/quarto-cli/src/resources/filters/./layout/lightbox.lua:144: in upvalue 'p
rocessSubFloat'
        ...n/quarto-cli/src/resources/filters/./layout/lightbox.lua:272: in local 'fil
ter_fn'
        ...n/quarto-cli/src/resources/filters/./ast/customnodes.lua:154: in function <
...n/quarto-cli/src/resources/filters/./ast/customnodes.lua:144>
        (...tail calls...)
        [C]: in ?
        [C]: in method 'walk'
        ...n/quarto-cli/src/resources/filters/./ast/customnodes.lua:76: in function <.
..n/quarto-cli/src/resources/filters/./ast/customnodes.lua:65>
        (...tail calls...)
        ...n/quarto-cli/src/resources/filters/./layout/lightbox.lua:258: in local 'fil
ter_fn'
        ...n/quarto-cli/src/resources/filters/./ast/customnodes.lua:154: in function <
...n/quarto-cli/src/resources/filters/./ast/customnodes.lua:144>
        (...tail calls...)
        [C]: in ?
        [C]: in method 'walk'
        ...n/quarto-cli/src/resources/filters/./ast/customnodes.lua:76: in function <.
..n/quarto-cli/src/resources/filters/./ast/customnodes.lua:65>
        (...tail calls...)
        .../quarto-cli/src/resources/filters/./ast/runemulation.lua:82: in local 'call
back'
        .../quarto-cli/src/resources/filters/./ast/runemulation.lua:100: in upvalue 'r
un_emulated_filter_chain'
        .../quarto-cli/src/resources/filters/./ast/runemulation.lua:136: in function <
.../quarto-cli/src/resources/filters/./ast/runemulation.lua:133>
stack traceback:
        ...n/quarto-cli/src/resources/filters/./ast/customnodes.lua:76: in function <.
..n/quarto-cli/src/resources/filters/./ast/customnodes.lua:65>
        (...tail calls...)
        ...n/quarto-cli/src/resources/filters/./layout/lightbox.lua:144: in upvalue 'p
rocessSubFloat'
        ...n/quarto-cli/src/resources/filters/./layout/lightbox.lua:272: in local 'fil
ter_fn'
        ...n/quarto-cli/src/resources/filters/./ast/customnodes.lua:154: in function <
...n/quarto-cli/src/resources/filters/./ast/customnodes.lua:144>
        (...tail calls...)
        [C]: in ?
        [C]: in method 'walk'
        ...n/quarto-cli/src/resources/filters/./ast/customnodes.lua:76: in function <.
..n/quarto-cli/src/resources/filters/./ast/customnodes.lua:65>
        (...tail calls...)
        ...n/quarto-cli/src/resources/filters/./layout/lightbox.lua:258: in local 'fil
ter_fn'
        ...n/quarto-cli/src/resources/filters/./ast/customnodes.lua:154: in function <
...n/quarto-cli/src/resources/filters/./ast/customnodes.lua:144>
        (...tail calls...)
        [C]: in ?
        [C]: in method 'walk'
        ...n/quarto-cli/src/resources/filters/./ast/customnodes.lua:76: in function <.
..n/quarto-cli/src/resources/filters/./ast/customnodes.lua:65>
        (...tail calls...)
        .../quarto-cli/src/resources/filters/./ast/runemulation.lua:82: in local 'call
back'
        .../quarto-cli/src/resources/filters/./ast/runemulation.lua:100: in upvalue 'r
un_emulated_filter_chain'
        .../quarto-cli/src/resources/filters/./ast/runemulation.lua:136: in function <
.../quarto-cli/src/resources/filters/./ast/runemulation.lua:133>
stack traceback:
        ...n/quarto-cli/src/resources/filters/./ast/customnodes.lua:76: in function <.
..n/quarto-cli/src/resources/filters/./ast/customnodes.lua:65>
        (...tail calls...)
        ...n/quarto-cli/src/resources/filters/./layout/lightbox.lua:258: in local 'fil
ter_fn'
        ...n/quarto-cli/src/resources/filters/./ast/customnodes.lua:154: in function <
...n/quarto-cli/src/resources/filters/./ast/customnodes.lua:144>
        (...tail calls...)
        [C]: in ?
        [C]: in method 'walk'
        ...n/quarto-cli/src/resources/filters/./ast/customnodes.lua:76: in function <.
..n/quarto-cli/src/resources/filters/./ast/customnodes.lua:65>
        (...tail calls...)
        .../quarto-cli/src/resources/filters/./ast/runemulation.lua:82: in local 'call
back'
        .../quarto-cli/src/resources/filters/./ast/runemulation.lua:100: in upvalue 'r
un_emulated_filter_chain'
        .../quarto-cli/src/resources/filters/./ast/runemulation.lua:136: in function <
.../quarto-cli/src/resources/filters/./ast/runemulation.lua:133>
stack traceback:
        ...n/quarto-cli/src/resources/filters/./ast/customnodes.lua:76: in function <.
..n/quarto-cli/src/resources/filters/./ast/customnodes.lua:65>
        (...tail calls...)
        .../quarto-cli/src/resources/filters/./ast/runemulation.lua:82: in local 'call
back'
        .../quarto-cli/src/resources/filters/./ast/runemulation.lua:100: in upvalue 'r
un_emulated_filter_chain'
        .../quarto-cli/src/resources/filters/./ast/runemulation.lua:136: in function <
.../quarto-cli/src/resources/filters/./ast/runemulation.lua:133>
ERROR: Error
    at renderFiles (file:///home/julian/quarto-cli/src/command/render/render-files.ts:
350:23)
    at eventLoopTick (ext:core/01_core.js:153:7)
    at async renderProject (file:///home/julian/quarto-cli/src/command/render/project.
ts:391:23)
    at async Command.actionHandler (file:///home/julian/quarto-cli/src/command/preview
/cmd.ts:357:36)
    at async Command.execute (file:///home/julian/quarto-cli/src/vendor/deno.land/x/cl
iffy@v1.0.0-rc.3/command/command.ts:1948:7)
    at async Command.parseCommand (file:///home/julian/quarto-cli/src/vendor/deno.land
/x/cliffy@v1.0.0-rc.3/command/command.ts:1780:14)
    at async quarto (file:///home/julian/quarto-cli/src/quarto.ts:156:3)
    at async file:///home/julian/quarto-cli/src/quarto.ts:170:5
    at async mainRunner (file:///home/julian/quarto-cli/src/core/main.ts:35:5)
    at async file:///home/julian/quarto-cli/src/quarto.ts:160:3

Steps to reproduce

  1. Use the official manuscript template https://github.com/quarto-ext/manuscript-template-vscode.git
  2. At the end of the index.qmd file, add the following figure div:
::: {#fig-multi layout-nrow=3}

![](./images/la-palma-map.png){#fig-a}

![](./images/la-palma-map.png){#fig-b}

Captions (a) bala; (b) bala
:::
  1. Now try to preview, the above error will appear
  2. We can modify the above div by including captions for each figure:
::: {#fig-multi layout-nrow=3}

![a](./images/la-palma-map.png){#fig-a}

![b](./images/la-palma-map.png){#fig-b}

Captions (a) bala; (b) bala
:::
  1. Now this index.qmd document can be rendered successfully.

image

Expected behavior

I noticed that the team provided nil protection to some of the environments, such as callout and float, but not for lightbox. See commit b8c82350851d53b1ab73c54c1800284b16af1046

Note that if we only have a single figure, even if a caption is missing, a document can still be rendered. The problem occurs when we have multiple figures in a manuscript type document.

Actual behavior

No response

Your environment

No response

Quarto check output

No response

juliantao commented 7 months ago

I just noticed that this issue is related to #9444 @cderv @gordonwoodhull

gordonwoodhull commented 7 months ago

Nod, probably the Pandoc upgrade and its nils replacing erroneous empty Inlines.

Thanks for the report & bisect!

gordonwoodhull commented 7 months ago

I can repro as described. After a "mechanical" fix of the crashing line, another similar bug is exposed in layout/docx.lua which can also be fixed mechanically.

diff --git a/src/resources/filters/layout/lightbox.lua b/src/resources/filters/layout/lightbox.lua
index a6ca0d0f7..a0479c856 100644
--- a/src/resources/filters/layout/lightbox.lua
+++ b/src/resources/filters/layout/lightbox.lua
@@ -144,7 +144,12 @@ function lightbox()
     subFloatEl = _quarto.ast.walk(subFloatEl, {
       traverse = 'topdown',
       Image = function(imgEl)
-        local caption_content = subFloatEl.caption_long.content or subFloatEl.caption_long
+        local caption_content
+        if subFloatEl.caption_long then
+          caption_content = subFloatEl.caption_long.content or subFloatEl.caption_long
+        else
+          caption_content = pandoc.Inlines({})
+        end
         local caption = full_caption_prefix(parentFloat, subFloatEl)
         tappend(caption, caption_content)
         local subImgModified = processImg(imgEl, { automatic = true, caption = caption, gallery = gallery })

diff --git a/src/resources/filters/layout/docx.lua b/src/resources/filters/layout/docx.lua
index 52b2491c1..8fb65c458 100644
--- a/src/resources/filters/layout/docx.lua
+++ b/src/resources/filters/layout/docx.lua
@@ -90,7 +90,7 @@ function docxDivCaption(captionEl, align)
   local caption = pandoc.Para({
     pandoc.RawInline("openxml", docxParaStyles(align))
   })
-  tappend(caption.content, captionEl.content)
+  tappend(caption.content, captionEl and captionEl.content or pandoc.Inlines({}))
   return caption
 end

However, I am not sure how to isolate this for a test, or alternately, how to add a manuscript project test, so leaving this for now. The described code in an isolated single file does not err.

I don't think this is a dupe of #9444, which encounters a null caption_long at a different code location. Does appear to a dupe as pointed out by @cderv! Location is different because of bundling.

cscheid commented 7 months ago

@gordonwoodhull I think we'd use our older testing procedures, which were to add the test document to tests/docs/... (not smoke-all). and then create an explicit render test testRender() call with the verification code in typescript. For example, https://github.com/quarto-dev/quarto-cli/blob/main/tests/smoke/engine/include-engine-detection.test.ts

gordonwoodhull commented 7 months ago

Taking this as @cderv showed me how to make a minimal reprex.