Typst property output #9648

gordonwoodhull · 2024-04-10T15:37:08Z

Here is my work in progress on Typst property output. Supercedes #9623.

https://github.com/gordonwoodhull/pandoc/tree/typst-property-output

It handles 8 cases of attribute-bearing AST nodes:

span text (no element properties because text() is the closest element to span)
div and div text
table and table text
td and td text (3 cases)

Now that I have tests I would be comfortable implementing the rest, but I haven't needed them yet. Thought I'd check in to ask if I'm on track here. All suggestions are welcome!

I think the code is more idiomatic Haskell (but not perfect) and the usage is terse (except for the align case, see below). Some details of whitespace/line breaks probably need more work.

There aren't row elements in Typst; tr attributes would have to be underlaid on cell properties.

align is messy

There is one case that is ugly - the Typst reader extracts (horizontal) align, but this needs to be coalesced with vertical-align for the Typst cell align. An alternative design would be to not remove the style in the reader, and allow Lua to override the whole property. Technically I should also parse typst:align as addition and then dedupe, because Typst will error if there are duplicate terms in the align expression, but Lua only sees vertical-align currently, so it works out.

jgm · 2024-04-11T16:30:02Z

src/Text/Pandoc/Writers/Typst.hs

+
+formatTypstProps :: [(Text, Text)] -> [Text]
+formatTypstProps =
+  map (\(k,v) -> last (T.splitOn (T.pack ":") k) <> ": " <> v)


Avoid last, as it's a partial function.
Also you don't need T.pack ":"; ":" will work by itself because we have OverloadedStrings.

Good points. I had to improve validation to drop keys like typst:foo:bar, so I took the opportunity to pull the last element at input time and not worry about invalid input during output.

jgm · 2024-04-11T16:36:43Z

test/Tests/Writers/Typst.hs

+      , "  )"
+      ]
+    , "table text" =:
+      let headers = []
+          rows = []
+      in tableWith ("", [], [("typst:text:fill", "orange")])
+                    emptyCaption
+                    []
+                    (TableHead nullAttr $ toHeaderRow headers)
+                    [TableBody nullAttr 0 [] $ map toRow rows]
+                    (TableFoot nullAttr [])
+    =?> unlines [
+        "#figure("
+      , "  align(center)[#text(fill: orange)[#table("
+      , "    columns: 0,"
+      , "    align: (),"
+      , "  )]]"
+      , "  , kind: table"
+      , "  )"
+      ]
+    , "table cell" =:
+      let headers = []
+          rows = [Row nullAttr [
+              cellWith ("", [], [("typst:fill", "blue")]) AlignDefault (RowSpan 1) (ColSpan 1) $ para "Foo"
+            , simpleCell $ para "Bar"
+            ]]
+      in table
+                    emptyCaption
+                    [(AlignDefault,ColWidthDefault),(AlignDefault,ColWidthDefault)]
+                    (TableHead nullAttr $ toHeaderRow headers)
+                    [TableBody nullAttr 0 [] rows]
+                    (TableFoot nullAttr [])
+    =?> unlines [
+        "#figure("
+      , "  align(center)[#table("
+      , "    columns: 2,"
+      , "    align: (auto,auto,),"
+      , "    table.cell(fill: blue)[Foo"
+      , ""
+      , "    ], [Bar"
+      , ""
+      , "    ],"
+      , "  )]"
+      , "  , kind: table"
+      , "  )"
+      ]
+    , "table cell text no attr" =:
+      let headers = []
+          rows = [Row nullAttr [
+              cellWith ("", [], [("typst:text:fill", "orange")]) AlignDefault (RowSpan 1) (ColSpan 1) $ para "Foo"
+            , simpleCell $ para "Bar"
+            ]]
+      in table
+                    emptyCaption
+                    [(AlignDefault,ColWidthDefault),(AlignDefault,ColWidthDefault)]
+                    (TableHead nullAttr $ toHeaderRow headers)
+                    [TableBody nullAttr 0 [] rows]
+                    (TableFoot nullAttr [])
+    =?> unlines [
+        "#figure("
+      , "  align(center)[#table("
+      , "    columns: 2,"
+      , "    align: (auto,auto,),"
+      , "    [#set text(fill: orange); Foo"
+      , ""
+      , "    ], [Bar"
+      , ""
+      , "    ],"
+      , "  )]"
+      , "  , kind: table"
+      , "  )"
+      ]
+    , "table cell text w attr" =:
+      let headers = []
+          rows = [Row nullAttr [
+              cellWith ("", [], [("typst:text:fill", "orange")]) AlignCenter (RowSpan 1) (ColSpan 1) $ para "Foo"
+            , simpleCell $ para "Bar"
+            ]]
+      in table
+                    emptyCaption
+                    [(AlignDefault,ColWidthDefault),(AlignDefault,ColWidthDefault)]
+                    (TableHead nullAttr $ toHeaderRow headers)
+                    [TableBody nullAttr 0 [] rows]
+                    (TableFoot nullAttr [])
+    =?> unlines [
+        "#figure("
+      , "  align(center)[#table("
+      , "    columns: 2,"
+      , "    align: (auto,auto,),"
+      , "    table.cell(align: center)[#set text(fill: orange); Foo"
+      , ""
+      , "    ], [Bar"
+      , ""
+      , "    ],"
+      , "  )]"
+      , "  , kind: table"
+      , "  )"
+      ]
+    ]
+  ]


I'd suggest just using a "command test" (see test/commands/*.md).
They are easier to create and much easier to maintain (since we can just do make test TESTARGS=--accept to update the expected output instead of doing it manually.

Aha, much simpler to write tests that way!

I threw in negative cases: elements that don't have typst:* attributes should not affect the output. This was covered by previous tests but worth it to be somewhat complete.

jgm · 2024-04-11T16:40:43Z

src/Text/Pandoc/Writers/Typst.hs

+-- there is probably a more idiomatic way to do this
+ifNotEmpty ::  ([a2] -> Doc Text) -> [a2] -> Doc Text
+ifNotEmpty f list =
+  case list of
+    [] -> ""
+    _ -> f list
+


I think it would be more elegant to use a fold. E.g. foldl' addTypstProperty mempty props.
If the list props is empty, you get mempty, so you wouldn't need this ugly function.
It should be possible to make this code simpler.

It's a little bit more complicated, because we have this conditional text we want to add around the lists, like we want to emit either #set text(prop:value); or the empty string at the top of a block where we are applying typst:text:* attributes. We don't want to emit a useless #set text(); (if that's even valid).

And likewise everywhere else: if a block has no attributes, we want to emit #block[content] and not #block()[content], etc.

@cscheid and I looked at it together and agreed that most clear thing to do is not to try to abstract it, but just write functions that both check for the empty list and generate text. This section is about half as many lines of code now.

gordonwoodhull · 2024-04-12T04:51:16Z

Okay, I think I have addressed your concerns, as well as adding solid validation of typst:* keys.

Glad to make any other changes! Should I add documentation somewhere?

I think I'd prefer to wait to implement output of properties from other elements until we get more experience, unless you have any requests.

<span>, <div>, <table> and <td> are the most important elements as far as Quarto is concerned.

I can do another PR later with other elements as our users inevitably test the boundaries of this, or we find more uses for Typst properties.

Thanks for all your helpful comments so far.

jgm · 2024-04-12T05:43:41Z

As for documentation, one possibility would be to have a separate document along the lines of
https://pandoc.org/org.html
whose source is doc/org.md. Alternatively it could be a section in the manual like the current section for EPUBs, but my thinking is that this concerns a pretty niche feature that is only accessible through filters, so it probably is best to document it separately...

jgm · 2024-04-12T17:04:01Z

src/Text/Pandoc/Writers/Typst.hs

+    if length(spl) < 2 || length(spl) > 3
+      || head spl /= "typst"
+      || (length(spl) == 3 && spl !! 1 /= "text")


We like to avoid using partial functions like head. I think it's cleaner to use pattern matching for this kind of thing. Could you do something like

case T.splitOn ":" k of "typst":"text":x -> Right (x,v) "typst":x -> Left (x, v) _ -> Nothing

Second thought: instead of using partitionEithers . catMaybes . map, why not just use a fold that directly constructs the tuple? This way you also avoid needing to use Maybe or Either.

pickTypstAttrs = foldr go ([],[]) where go (k,v) (eltProps, textProps) = case T.splitOn ":" k of "typst":"text":x -> (eltProps, (x,v):textProps) "typst":x -> ((x,v):eltProps, textProps) _ -> (eltProps, textProps)

Or even more compactly with Data.Bifunctor.(first,second)

pickTypstAttrs = foldr go ([],[]) where go (k,v) case T.splitOn ":" k of "typst":"text":x -> second ((x,v):) "typst":x -> first ((x,v):) _ -> id

This is very nice!

All five of my logical conditions covered by three patterns, and the meaning is very clear with the bifunctor approach.

jgm · 2024-04-12T17:08:49Z

src/Text/Pandoc/Writers/Typst.hs

+formatTypstProps :: [(Text, Text)] -> [Text]
+formatTypstProps =
+  map (\(k,v) -> k <> ": " <> v)
+
+toTypstPropsListSep :: [(Text, Text)] -> Doc Text
+toTypstPropsListSep typstAttrs =
+   literal (T.intercalate ", " $ formatTypstProps typstAttrs)
+


With this approach the attribute lists will be unbreakable.
That's probably not what we want. Better:

formatTypstProps = map (\(k,v) -> literal (k <> ": " <> v)) toTypstPropsListSep = hsep . intersperse "," . formatTypstProps

Great, knew I was missing something about line breaks. Thanks!

jgm · 2024-04-12T17:11:25Z

src/Text/Pandoc/Writers/Typst.hs

+
+toTypstTextElement :: [(Text, Text)] -> Doc Text -> Doc Text
+toTypstTextElement [] content = content
+toTypstTextElement typstTextAttrs content = "#text" <> parens (toTypstPropsListSep typstTextAttrs) <> brackets content


Could simplify by using toTypstPropsListParens here instead of the spelled out version

gordonwoodhull · 2024-04-12T21:37:23Z

Excellent feedback - I think it is very clean and simple now.

I've pushed a first draft of the docs. The wording is a little rough but I think they are complete.

Will take another pass over the weekend and clean up.

iandol · 2024-04-13T07:29:28Z

Amazingly useful contribution @gordonwoodhull!!! 🥳

jgm · 2024-04-13T16:33:55Z

doc/typst-property-output.md

+the command
+
+```sh
+quarto pandoc -f html -t typst --lua-filter ./typst-property-example.lua


Since this is pandoc's documentation, not quarto, shouldn't this just be pandoc?

This implements output of Typst properties from attributes of the forms * `typst:prop` - the value should be interpreted as raw Typst code to set a parameter on the corresponding Typst element * `typst:text:prop` - the value should be interpreted as raw Typst code to set a parameter on a Typst `#set text()` rule, or text element as appropriate. This commit covers the following elements: * span text * div and div text * table and table text * td and td text It would be possible to support more elements in the future.

gordonwoodhull · 2024-04-13T23:30:07Z

I fixed the example, did a few more copy edits, and squashed everything into one commit.

I think this is ready! Please let me know if you want any further changes.

jgm · 2024-04-14T23:03:33Z

test/command/typst-property-output.md

+```
+
+```
+% pandoc -f html -t typst


Is there a difference between this test and the one at line 76?

Did you mean to have a test of an attribute on table?

Hmm, I guess I added negative tests for both table and cell without somehow realizing they are the same. Removed the redundant test.

this is the annoying case where we have to concatenate align and i forgot to update it when cleaning up attr validation

Because it's already inside figure & align, so these settings won't pollute anything

gordonwoodhull · 2024-04-15T08:15:21Z

I found that I had failed to update the ugly special case for align when I improved validation, so I pushed a fix and associated test.

Also I noticed that we can use a set-text rule instead of text element for table, since the table is inside the figure & align scopes and the set-text rule won't leak out.

jgm · 2024-04-15T16:32:48Z

I think it's good to go! Thanks.

gordonwoodhull · 2024-04-15T17:07:48Z

Thank you!

gordonwoodhull · 2024-05-09T22:06:13Z

This is working great and we'd like to include "Typst CSS" in Quarto 1.5.

When you're around and have a minute, please release 3.1.14!

gordonwoodhull mentioned this pull request Apr 10, 2024

poc: typst output properties #9623

Closed

jgm reviewed Apr 11, 2024

View reviewed changes

gordonwoodhull force-pushed the typst-property-output branch from 38d0a75 to c6a21ca Compare April 12, 2024 04:27

gordonwoodhull marked this pull request as ready for review April 12, 2024 04:32

jgm reviewed Apr 12, 2024

View reviewed changes

jgm reviewed Apr 13, 2024

View reviewed changes

gordonwoodhull force-pushed the typst-property-output branch from cfb080c to 62b7cec Compare April 13, 2024 23:26

jgm reviewed Apr 14, 2024

View reviewed changes

gordonwoodhull added 3 commits April 15, 2024 02:18

remove redundant test

515b414

use align not typst:align, add test for align

ad38fce

this is the annoying case where we have to concatenate align and i forgot to update it when cleaning up attr validation

Use set-text rule for table

edd2657

Because it's already inside figure & align, so these settings won't pollute anything

jgm merged commit 99a4591 into jgm:main Apr 15, 2024
8 of 12 checks passed

gordonwoodhull deleted the typst-property-output branch April 15, 2024 17:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Typst property output #9648

Typst property output #9648

gordonwoodhull commented Apr 10, 2024 •

edited

Loading

jgm Apr 11, 2024

gordonwoodhull Apr 12, 2024

jgm Apr 11, 2024

gordonwoodhull Apr 12, 2024

jgm Apr 11, 2024

gordonwoodhull Apr 12, 2024 •

edited

Loading

gordonwoodhull commented Apr 12, 2024

jgm commented Apr 12, 2024

jgm Apr 12, 2024

gordonwoodhull Apr 12, 2024

jgm Apr 12, 2024

gordonwoodhull Apr 12, 2024

jgm Apr 12, 2024

gordonwoodhull commented Apr 12, 2024 •

edited

Loading

iandol commented Apr 13, 2024

jgm Apr 13, 2024

gordonwoodhull Apr 13, 2024

gordonwoodhull commented Apr 13, 2024

jgm Apr 14, 2024

jgm Apr 14, 2024

gordonwoodhull Apr 15, 2024 •

edited

Loading

gordonwoodhull commented Apr 15, 2024

jgm commented Apr 15, 2024

gordonwoodhull commented Apr 15, 2024

gordonwoodhull commented May 9, 2024

Typst property output #9648

Typst property output #9648

Conversation

gordonwoodhull commented Apr 10, 2024 • edited Loading

align is messy

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gordonwoodhull Apr 12, 2024 • edited Loading

Choose a reason for hiding this comment

gordonwoodhull commented Apr 12, 2024

jgm commented Apr 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gordonwoodhull commented Apr 12, 2024 • edited Loading

iandol commented Apr 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gordonwoodhull commented Apr 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gordonwoodhull Apr 15, 2024 • edited Loading

Choose a reason for hiding this comment

gordonwoodhull commented Apr 15, 2024

jgm commented Apr 15, 2024

gordonwoodhull commented Apr 15, 2024

gordonwoodhull commented May 9, 2024

gordonwoodhull commented Apr 10, 2024 •

edited

Loading

gordonwoodhull Apr 12, 2024 •

edited

Loading

gordonwoodhull commented Apr 12, 2024 •

edited

Loading

gordonwoodhull Apr 15, 2024 •

edited

Loading