Functional coding vs. AST munching in commands: a design issue and many questions

[Omikhleia]

When working on "generic" styling, inputters and programmatic command creation, I got some design issues which I'll try to explain here...

SILE.call() can take, as third argument, an AST tree (which is what content usually is) or a function (which seems handy when implementing some complex typesetting).

We use the latter quite a bit in the code base. Let's illustrate this strategy with some dumb and naive commands ("approach 1"):

\begin{document}
\lua{
-- Emulating what we could do in some package or class
local class = SILE.documentState.documentClass
class:registerCommand("see", function (options, content)
  SILE.call("seestyle", options, function ()
    SILE.typesetter:typeset("see ")
    SILE.process(content)
  end)
end)
class:registerCommand("seestyle", function (_, content)
  -- Default style hook typesets in italic, override it for something different.
  SILE.call("em", {}, content)
end)
}
\see{my little example}
\end{document}

So far so good, or so it seems: It outputs "see my little example", with my "default" style hook applied, so I get "see my little example" which was expected...

But say I now want a different style to be applied, so I override the hook as follows:

\use[module=packages.textcase]
\define[command=seestyle]{\uppercase{\process}}
\see{my little example}

Guess what happens... It errors.

Using code at: ./core/utilities.lua:191: bad argument #1 to 'for iterator' (table expected, got function)
(...)
stack traceback:
	[C]: in function 'next'
	./core/utilities.lua:191: in function 'core.utilities.sortedpairs'
	./packages/inputfilter/init.lua:8: in function 'packages.inputfilter.transformContent'
        (...)

Text casing uses an input filter, and those choke on functions. They always expect a structure AST (i.e. tables).
Let's call this issue A. How can we address it? It's not obvious and it's unlikely we'd get the expected output (so now "SEE MY LITTLE EXAMPLE") here...

Well, back to what I said in the very first sentence, maybe we should have used a structured AST table, rather than a functional call? Let's try it and re-implement our command ("approach 2"):

class:registerCommand("see", function (options, content)
  local recontent = SU.subContent(content)
  recontent.id = nil
  SILE.call("seestyle", options, {
    "see ",
    recontent
  })
end)

All of a sudden, it works with the initial italicizing hook or my overridden upper-casing hook as well. Yay!?

But here we need to know a bit more about what SILE expects as an AST.

• It can be a table, containing text string or other AST sub trees...
• But if we want to pass the content we have to extract (= re-copy) its array items (the "child" sub-tree).

Regarding the latter point, it's because the content as an AST tree is actually:

{
  "my little example",
  command = "see",
  id = "texlike_command",
  options = {
  },
  col = ...,
  lno = ...,
  pos = ...
}

So yeah, we have to extract the sub content tree (i.e. the array elements, ignoring the toplevel key-value elements), since we already are processing the "see" command... Let's call this issue B.
First, note that if we had a slightly different AST the "extraction" issue could have been avoided (e.g. consider the Pandoc AST, it splits tags, attributes and child nodes in different sub-objects). But we cannot just decide to use a different AST now, or many things will break and would need a complete re-implementation...
Thus, let's stick to it. It's not a very efficient way of doing things (since we'll create a shallow copy of the tree...), but heh... And we do have the SU.subContent convenience utility, after all, right? Ahem... Except that its implementation is real weird.

• It does some sorting on the table keys, but then just uses the numerical (indexed) entries... This doesn't seem to make sense here... Issue B1.
• It adds a id = "stuff" entry to the extracted content... Which we have to remove if we want our above approach not to generate another obscure error... So it's not even obvious what the intent was here. Issue B2

In my packages, I realize I am more and more moving from "approach 1" (functional calls) to "approach 2" (AST hacking)... But I am not sure how else the issue may be addressed.

• A doesn't seem to have a good solution
• B is made complex and possibly fragile, depending a lot on SILE's AST at its lowest level...
• B1 and B2 are weird, so I ended up using a much simpler sub-content extraction (just looping with ipairs on the content...)

EDIT: References for cases where I used "approach 2" recently:

• Command cascades should build an AST instead of functions Omikhleia/markdown.sile#80 (my initial issue with commands that expect a tree)
• feat: add character style support for text casing Omikhleia/resilient.sile#41 (so messy that I wonder where this is heading and am frightened merging!)

[Omikhleia]

I don't really expect an answer now - I'll definitively move towards removing as many SILE.call(function () ...) as I can from my own code base, and to avoid the dubious SU.subContent()...

Additional question of the day: why ...registerCommand("xxx", function(options, content) since, as the AST is made, content.options == options)? No I don't expect an informed answer either. 😁

[alerque]

Additional question of the day: why ...registerCommand("xxx", function(options, content) since, as the AST is made, content.options == options)? No I don't expect an informed answer either. 😁

Because content as passed to commands may not be a table, it can be nil or a string or a table or a function. Not all content being passed to our functions came out of our AST builder that also stuffed (the probably redundant) copy of options on AST nodes.

[Omikhleia]

Let's admit that fact.

I am still puzzle by many things, such as the role and intent of the id = "stuff" (B2)

EDIT: And the weird option sorting (B1).

EDIT: And the obscure error (A) although things could possibly be handled more nicely

[alerque]

To the best of my recollection the "stuff" naming started in the LPEG parser which attaches the matching rule as the ID of the output, herse we got id = "texlike_stuff" and several other variants. Because of some assumptions elsewhere when we iterate over AST content, we do need all content nodes to have an id. Not having an ID puts them in a different handling mode. I think the id = "stuff" was just to give them some id key and don't think it has any particular meaning.

As @simoncozens will admit, Naming Stuff Is Hard™, and the LPEG grammar was rife with bad naming out of the gate. I've tried to clean it up a little each time I've overhauled it, but it still isn't great. One of the outcomes of #1435 should be more sensible names for each part of the grammar and parser.

Also from memory, the option sorting is just so we can both process the numbered keys in order (content) and have reproducible ordering of the named options. Lua doesn't grantee pairs() returns in any particular order, and in order to pass our regression tests we need to make sure we always handle them in the same order, whatever that is.

The obscure error is because the subContent() function has a bug: it is deficient in that it only handles some types of nodes we allow in the AST. In particular we allow content nodes to be tables, strings, or functions. The subContent function makes the assumption that it will only be run on table content. It should test for that and error or do some sensible passthrough in the other cases.

[Omikhleia]

I think the id = "stuff" was just to give them some id key and don't think it has any particular meaning.

#1914 proposes its removal (addressing B1 and B2 concerns)