add box() around image() when it's safe to do so. #9105
Conversation
src/Text/Pandoc/Writers/Typst.hs
Outdated
| let widthV = lookup "width" kvs | ||
| let heightV = lookup "height" kvs | ||
| let (boxOpen, boxClose) = case (widthV, heightV) of | ||
| (Nothing, Nothing) -> mempty | ||
| _ -> ("box(", ")") | ||
| let width' = maybe mempty ((", width: " <>) . literal) $ widthV | ||
| let height' = maybe mempty ((", height: " <>) . literal) $ heightV | ||
| return $ "#" <> boxOpen <> "image(" <> doubleQuoted src <> width' <> height' <> ")" <> boxClose |
There was a problem hiding this comment.
Sorry for the delay in reviewing. This looks good but I'm thinking it might be worth fixing all of #9104 by always using a box, and supplying some default dimensions in the case where dimensions are missing. We might then get an oddly sized inline image, but it would be better than having it omitted altogether. We could even fetch the image with fetchItem and compute its size using imageSize; then the size we provide would match the image. What do you think?
There was a problem hiding this comment.
by always using a box
That would make the behavior more consistent, which I like. I just don't know the consequences it would have for typst's layout in all settings. If you go that way, I think it would be prudent to have some override that disables it, in case it behaves badly.
We could even fetch the image with fetchItem and compute its size using imageSize; then the size we provide would match the image.
I think that would be excellent, and consistent with the rest of Pandoc. I thought about that solution but it went beyond my current knowledge of the code base. I'd be happy to give it a shot but I could use some pointers (if it's faster for you to do it, then of course that's fine too - just looking to save you time)
There was a problem hiding this comment.
I could make the code changes if you'd like. But what would help me is some clarity about the rules for typst. I don't want to add an option. If adding a box to all images is going to be a problem, it would be good to identify up front the cases where it would.
There was a problem hiding this comment.
According to the typst docs,
All elements except inline math, text, and boxes are block-level and cannot occur inside of a paragraph.
I suppose, then, that the "Pandoc-native" resolution to this problem would be to wrap all Pandoc inlines that are not typst "inlines" in a box. After going through the Typst docs, I think that the only problematic element really is Image. Other typst elements like Link, Cite, and Ref, are ultimately rendered as text, and (in my limited experiments!) behave as text for the purposes of block-vs-inline distinctions.
There was a problem hiding this comment.
To add some more to the fetchItem and imageSize idea, I do think it remains important to allow the image to be sized explicitly, because the typst rendering algorithm allows for some fancy sizing options like "fraction of the remaining space" (fr).
Nevertheless, I agree that by default, it would be excellent for Pandoc to provide sizing automatically whenever possible (I looked into having quarto do it but it didn't seem possible to do it from Lua; in passing, imageSize would be a really useful entry point for the pandoc.mediabag Lua API, @tarleb!)
There was a problem hiding this comment.
Yes, we would still use explicit sizes if they are provided; we'd only look at the image's actual size if we needed to provide our own size.
|
See alternative PR #9149. Not really tested yet. |
This should ammeliorate #9104.
I don't know what's the preferred Haskell style in Pandoc, and I speak Haskell with a heavy early-2000s accent. Let me know if there's better, more
<>-y ways to write that case statement!This passes local
cabal testtests, but I'm not sure if that's all I should have done.I added a
test/command/9014.mdentry, but I'm not sure I got the syntax right.