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

Typst writer: add #box around image to make it inline. #9149

Merged
merged 3 commits into from
Oct 20, 2023
Merged

Typst writer: add #box around image to make it inline. #9149

merged 3 commits into from
Oct 20, 2023

Conversation

jgm
Copy link
Owner

@jgm jgm commented Oct 20, 2023

An #image by itself in typst is a block-level element. To force images to be inline (as they are in pandoc), we need to add a box with an explicit width. When a width is not given in image attributes, we compute one from the image itself, when possible.

Closes #9104, supersedes #9105.

An `#image` by itself in typst is a block-level element.
To force images to be inline (as they are in pandoc), we need
to add a box with an explicit width. When a width is not given
in image attributes, we compute one from the image itself, when
possible.

Closes #9104, supersedes #9105.
@cscheid
Copy link
Contributor

cscheid commented Oct 20, 2023

As far as typst is concerned, I think it's enough to provide either of width or height. It looks from my reading of the code that the logic around imageSize depends only on the presence of width. If that's true, then pandoc would compute and provide width in the case that only height appears in the document.

Assuming I'm right, I think this is not ideal. I think the imageSize logic should be triggered only if neither of width and height is provided.

@jgm
Copy link
Owner Author

jgm commented Oct 20, 2023

ok, I've made a small change, see how that looks

@cscheid
Copy link
Contributor

cscheid commented Oct 20, 2023

I think that works.

It took me a bit because my non-Haskell brain has a hard time reading the block inside the case statement width' <- case lookup "width" kvs... and accepting it will not be evaluated if the other case statement matches (_, Just h). But I think it all works out correctly. Thank you!

@jgm
Copy link
Owner Author

jgm commented Oct 20, 2023

No, your instinct was right, the code isn't right. I'll fix.

@jgm
Copy link
Owner Author

jgm commented Oct 20, 2023

I think this is cleaner, but I've been writing in haste, so it needs a close look and testing.

@cscheid
Copy link
Contributor

cscheid commented Oct 20, 2023

Yes - this looks symmetric wrt exchanging width and height, which makes me think it's more obviously right.

@jgm jgm merged commit a67223c into main Oct 20, 2023
18 of 21 checks passed
@jgm jgm deleted the issue9104 branch October 20, 2023 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

typst images are block elements unless there's a box() around them
2 participants