Skip to content

perf: Use non-lazy AST traversal for filters when possible #11052

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 25 commits into from

Conversation

tarleb
Copy link
Collaborator

@tarleb tarleb commented Oct 13, 2024

  • Add jog Lua module
  • Use jog instead of walk
  • Force use of pandoc walk for some filters
  • Add mechanism to check jog results

Welcome to the quarto GitHub repo!

We are always happy to hear feedback from our users.

To file a pull request, please follow these instructions carefully: https://yihui.org/issue/#bug-reports

If you're a collaborator from outside quarto-dev making changes larger than a typo, please make sure you have filed an individual or corporate contributor agreement. You can send the signed copy to jj@rstudio.com.

Also, please complete and keep the checklist below.

Description

Please describe your PR here.

```py
print("Hello Quarto!")
```

Checklist

I have (if applicable):

  • filed a contributor agreement.
  • referenced the GitHub issue this PR closes
  • updated the appropriate changelog

@tarleb tarleb marked this pull request as draft October 13, 2024 16:18
@tarleb tarleb force-pushed the jog branch 6 times, most recently from 8a04c2a to d7651cc Compare October 15, 2024 16:16
@tarleb tarleb force-pushed the jog branch 4 times, most recently from f1b9896 to 22ce9b0 Compare November 18, 2024 15:23
@cscheid cscheid added this to the v1.7 milestone Nov 18, 2024
@tarleb tarleb force-pushed the jog branch 6 times, most recently from 4d683c6 to 51c8800 Compare November 19, 2024 16:28
@gordonwoodhull
Copy link
Contributor

gordonwoodhull commented Nov 19, 2024

Hi @tarleb, the root cause of the Typst callout failures is that both with jog and without, the following

parse = function(div)
quarto_global_state.hasCallouts = true
local title = string_to_quarto_ast_inlines(div.attr.attributes["title"] or "")
if not title or #title == 0 then
title = resolveHeadingCaption(div)
end

will initialize callout.title to Inlines({})

However, when the custom node is read later on to generate Typst, content.title is nil on main, but

Plain {
  clone: function: 0x600002f273c0
  content: Inlines {}
  show: function: 0x600002f27450
  walk: function: 0x600002f27420
}

on jog.

return _quarto.format.typst.function_call("callout", {
{ "body", _quarto.format.typst.as_typst_content(callout.content) },
{ "title", _quarto.format.typst.as_typst_content(
callout.title or pandoc.Plain(_quarto.modules.callouts.displayName(callout.type))
)},
{ "background_color", pandoc.RawInline("typst", background_color) },
{ "icon_color", pandoc.RawInline("typst", icon_color) },
{ "icon", pandoc.RawInline("typst", "" .. icon .. "()")}
})

It's not the right solution, because we'd like to understand why it is converting (?) the title differently, but the following diff gets these tests passing:

diff --git a/src/resources/filters/customnodes/callout.lua b/src/resources/filters/customnodes/callout.lua
index 8a9f4c17d..e9deaa2d2 100644
--- a/src/resources/filters/customnodes/callout.lua
+++ b/src/resources/filters/customnodes/callout.lua
@@ -261,10 +261,14 @@ function _callout_main()
       end
     end
     if callout.attr.identifier == "" then
+      local title = callout.title
+      if not title or #title.content == 0 then
+        title = pandoc.Plain(_quarto.modules.callouts.displayName(callout.type))
+      end
       return _quarto.format.typst.function_call("callout", { 
         { "body", _quarto.format.typst.as_typst_content(callout.content) },
         { "title", _quarto.format.typst.as_typst_content(
-          callout.title or pandoc.Plain(_quarto.modules.callouts.displayName(callout.type))
+          title
         )},
         { "background_color", pandoc.RawInline("typst", background_color) },
         { "icon_color", pandoc.RawInline("typst", icon_color) },

This also fixes unwanted behavior of `as_blocks`, which would treat a
list of Inline elements as a list of singleton Plain elements, leading
to bad results.
This also fixes unwanted behavior of `as_blocks`, which would treat a
list of Inline elements as a list of singleton Plain elements, leading
to bad results.
This allows to get reproducible output when running the filter multiple
times.
@tarleb tarleb force-pushed the jog branch 2 times, most recently from 67c4ca7 to 7ca4ad5 Compare November 26, 2024 20:03
Jog can be enable on a per-filter basis by setting the `traverse` field
to `jog`.
@tarleb
Copy link
Collaborator Author

tarleb commented Nov 26, 2024

I've split this into multiple PRs, including #11525, #11526, #11538, and #11540.

@tarleb tarleb closed this Nov 26, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants