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

Space in file path can not be processed by typst #10181

Closed
albert-ying opened this issue Jul 1, 2024 · 6 comments · Fixed by #10200
Closed

Space in file path can not be processed by typst #10181

albert-ying opened this issue Jul 1, 2024 · 6 comments · Fixed by #10200
Assignees
Labels
bug Something isn't working figures typst
Milestone

Comments

@albert-ying
Copy link

albert-ying commented Jul 1, 2024

Bug description

When adding image with space in filename, Typst gives error while other formats are fine

Update: it only happen in #box element (i.e., when specifying width

Steps to reproduce

---
format: typst
---

## This works

![](path/to/image with space in name.png)

## This also works

![](image.png){width="50%"}

## This does not work

![](path/to/image with space in name.png){width="50%"}

## This does not work

::: {layout-ncol=2}

![](path/to/image with space in name.png)

![](path/to/image with space in name.png)

:::

quarto render report.qmd --to typst

Expected behavior

Render with no error

Actual behavior

pandoc 
  to: typst
  output-file: report.typ
  standalone: true
  default-image-extension: svg
  wrap: none
  citeproc: false
  
[typst]: Compiling report.typ to report.pdf...error: file not found (searched at xxxxx)
    ┌─ report.typ:276:23
    │
276 │ #box(width: 100%,image("xxxx.png"))
    │                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Your environment

No response

Quarto check output

quarto check
Quarto 1.4.557
[✓] Checking versions of quarto binary dependencies...
      Pandoc version 3.1.12: OK
      Dart Sass version 1.69.5: OK
      Deno version 1.37.2: OK
[✓] Checking versions of quarto dependencies......OK
[✓] Checking Quarto installation......OK
      Version: 1.4.557
      Path: /Applications/quarto/bin

[✓] Checking tools....................OK
      TinyTeX: v2023.12
      Chromium: (not installed)

[✓] Checking LaTeX....................OK
      Using: TinyTex
      Path: /Users/A.Y/Library/TinyTeX/bin/universal-darwin
      Version: 2023

[✓] Checking basic markdown render....OK

[✓] Checking Python 3 installation....OK
      Version: 3.11.5 (Conda)
      Path: /opt/homebrew/anaconda3/bin/python
      Jupyter: 5.3.0
      Kernels: python3

[✓] Checking Jupyter engine render....OK
@albert-ying albert-ying added the bug Something isn't working label Jul 1, 2024
@cderv
Copy link
Collaborator

cderv commented Jul 1, 2024

---
output:
  pdf_document
---

This is not a YAML header for Quarto here. So the reproducible example is not ok. Can you share the real reproducible example you ran as I see to: typst in you sharedl log ?

I tried

---
format: typst
keep-typ: true
---

![](images/image with space.png)

And it is working ok.

@cderv cderv added the needs-repro Issues that are blocked until reporter provides an adequate reproduction label Jul 1, 2024
@albert-ying
Copy link
Author

Hi @cderv, thank you for the reminder -- I updated the post. It seems that the error only happen when the space is in the filepath AND the width is specified.

@cderv
Copy link
Collaborator

cderv commented Jul 1, 2024

Thanks for the new reproducible example.

The problem lies in the typ filep produced

276 │ #box(width: 100%,image("xxxx.png"))
   │                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

xxx is not helpful here. Real message is

281 │ #box(width: 50%,image("image%20with%20space.png"))
    │                       ^^^^^^^^^^^^^^^^^^^^^^^^^^

This shows that the spaces are encoded.

And it happens only for all except the first. THis is the intermediate .typ content

= This works
<this-works>
#box(image("image with space.png"))

= This also works
<this-also-works>
#box(width: 50%,image("image%20with%20space.png"))

= This does not work
<this-does-not-work>
#box(width: 50%,image("images/image%20with%20space.png"))

= This does not work
<this-does-not-work-1>
#block[
]
#grid(
columns: (50.0%, 50.0%), gutter: 1em, rows: 1,
  rect(stroke: none, width: 100%)[
#box(width: 100%,image("images/image%20with%20space.png"))

],
  rect(stroke: none, width: 100%)[
#box(width: 100%,image("images/image%20with%20space.png"))

],

It does not happen with bare pandoc. So it comes from our processing it seems.

@cderv cderv added typst figures and removed needs-repro Issues that are blocked until reporter provides an adequate reproduction labels Jul 1, 2024
@cderv
Copy link
Collaborator

cderv commented Jul 1, 2024

It does not happen with bare pandoc. So it comes from our processing it seems.

This is interesting ! This is because Pandoc will unEscape strings in the writer.
https://github.com/jgm/pandoc/blob/217adafcb5e056090e66b3fe2c7b4d94ec9bcf39/src/Text/Pandoc/Writers/Typst.hs#L429

src' = T.pack $ unEscapeString $ T.unpack src -- #9389

It was reported in pandoc in January:

and added in 3.1.12

They use a function from Haskell Network.Uri package for this so there is not API exposed by Pandoc. (cc @tarleb - could be an idea to expose this function in pandoc's Lua api as it seems Images will have escaped String as source ...)

But we have our own, so we should probably use this in the right place...

function urldecode(url)
if url == nil then
return
end
url = url:gsub("+", " ")
url = url:gsub("%%(%x%x)", function(x)
return string.char(tonumber(x, 16))
end)
return url
end
function fullyUrlDecode(url)
-- decode the url until it is fully decoded (not a single pass,
-- but repeated until it decodes no further)
result = urldecode(url)
if result == url then
return result
else
return fullyUrlDecode(result)
end
end

@cderv
Copy link
Collaborator

cderv commented Jul 1, 2024

diff --git a/src/resources/filters/quarto-post/typst.lua b/src/resources/filters/quarto-post/typst.lua
index 3ceefe686..6e0aba7ac 100644
--- a/src/resources/filters/quarto-post/typst.lua
+++ b/src/resources/filters/quarto-post/typst.lua
@@ -121,7 +121,7 @@ function render_typst_fixups()
           attr_str = attr_str .. "height: " .. height .. ","
         end
         local escaped_src = image.src:gsub("\\", "\\\\"):gsub("\"", "\\\"")
-        return pandoc.RawInline("typst", "#box(" .. attr_str .. "image(\"" .. escaped_src .. "\"))")
+        return pandoc.RawInline("typst", "#box(" .. attr_str .. "image(\"" .. fullyUrlDecode(escaped_src) .. "\"))")
       end
 
       return image

@cderv
Copy link
Collaborator

cderv commented Jul 1, 2024

The diff above fixes it. Though it modifies a block that I am not sure we still need based on the comment

Image = function(image)
image = _quarto.modules.mediabag.resolve_image_from_url(image) or image
-- if the width or height are "ratio" or "relative", in typst parlance,
-- then we currently need to hide it from Pandoc 3.1.9 until
-- https://github.com/jgm/pandoc/issues/9104 is properly fixed
if is_ratio_or_relative(image.attributes["width"]) or is_ratio_or_relative(image.attributes["height"]) then
local width = image.attributes["width"]
local height = image.attributes["height"]
image.attributes["width"] = nil
image.attributes["height"] = nil
local attr_str = ""
if width ~= nil then
attr_str = attr_str .. "width: " .. width .. ","
end
if height ~= nil then
attr_str = attr_str .. "height: " .. height .. ","
end
local escaped_src = image.src:gsub("\\", "\\\\"):gsub("\"", "\\\"")
return pandoc.RawInline("typst", "#box(" .. attr_str .. "image(\"" .. escaped_src .. "\"))")
end

So maybe the real fix is to just rely on Pandoc processing

❯ quarto pandoc --to typst
![](image with space.png){width=50%}
^Z
#box(image("image with space.png", width: 50%))

❯ quarto pandoc --to typst
![](image with space.png){height=50%}
^Z
#box(image("image with space.png", height: 50%))

So it seems it is doing the right thing now and d56ea70 (#7577) could be reverted

@cscheid I defer to you for decision

@cderv cderv added the triaged-to Issues that were not self-assigned, signals that an issue was assigned to someone. label Jul 1, 2024
@cscheid cscheid added this to the v1.6 milestone Jul 1, 2024
@cscheid cscheid assigned cscheid and unassigned cscheid Jul 1, 2024
@cscheid cscheid removed the triaged-to Issues that were not self-assigned, signals that an issue was assigned to someone. label Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working figures typst
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants