Pandoc: Lua filters: make pandoc.Attr more user friendly

[fiapps]

I was unable to figure out on my own how to pass attributes to pandoc.Attr, and in the reply to my StackExchange question I was invited to open an issue to discuss a better interface.

I tried to do this: attrs = pandoc.Attr("", {}, {lang = "es-SP"}).
The correct usage is (as of Pandoc 2.0.1.1): attrs = pandoc.Attr("", {}, {{"lang", "es-SP"}}). It was explained that the reason for this non-intuitive syntax is that it could be important to preserve the order of elements, and Lua provides no guarantees about the order of keys in a table.

[fiapps]

My first comment on this is that the non-intuitive syntax wouldn't have been a problem for me if it had been documented, but "table containing string keys and values" wasn't clear enough for me to understand that I was meant to pass {{"k1", "v1"}, {"k2", "v2"}, …}. Sample code would have also been sufficient.

As for a more intuitive interface, I'm not sure what to suggest. My first thought was that an ordered list of {key, value} pairs is fine as an internal representation, but the constructor pandoc.Attr should accept a table like {k1 = "v1", k2 = "v2"}. This would work perfectly well for me, but I soon realized it would fail to preserve attribute order in other use cases. Suppose one is adding an attribute to elements with existing attributes. If the attributes are converted from an internal ordered table to a table indexed by key at the API level, and then back to an ordered table for internal purposes, the original order will be lost.

I suppose pandoc.Attr could accept either a table indexed by string keys or an ordered table of pairs, but that might be complicated to implement.

Thinking about users potentially manipulating attribute lists makes me think that with the current representation, it would be cumbersome for users to perform operations like getting the value of an attribute or setting an attribute that may already exist. Perhaps helper functions would be appropriate if these turn out to be common operations.

[pkulchenko]

Maybe accept a regular (unordered) table and another (optional) table that only includes the keys in the right order?

[tarleb]

Thanks for raising the issue, @fiapps. You are right about documentation, an example would be helpful.

Maybe we can assign a metatable to the list of attributes: this would allow us to provide key-value based access while still keeping the underlying structure as-is. This should leave serialization/deserialization performance mostly unaffected and backwards compatible. This should make attributes much more user friendly, especially when combined with @pkulchenko's suggestion.

[jgm]

An easier way to access key-value attributes would be great.

E.g., it should be a simple process to check the 'style'
attribute of a Span.

It might also be nice to include a helper function to check
if a string is one of the classes of an attribute. (Without
that, one has to iterate through the table, which is ugly
and un-functional.)

+++ Albert Krewinkel [Nov 15 17 19:46 ]:

Thanks for raising the issue, [1]@fiapps. You are right about
documentation, an example would be helpful.

Maybe we can assign a metatable to the list of attributes: this would
allow us to provide key-value based access while still keeping the
underlying structure as-is. This should leave
serialization/deserialization performance mostly unaffected and
backwards compatible. This should make attributes much more user
friendly, especially when combined with [2]@pkulchenko's suggestion.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, [3]view it on GitHub, or [4]mute the
thread.

References

1. https://github.com/fiapps
2. https://github.com/pkulchenko
3. https://github.com/jgm/pandoc/issues/4071#issuecomment-344706860
4. https://github.com/notifications/unsubscribe-auth/AAAL5EHVD6uHaQO8P30Ia5bQPZeYvXhrks5s2z-BgaJpZM4Qd_XI

[tarleb]

Adding a List metatable to the class attributes is simple enough and shouldn't affect performance significantly. We might want to think about setting that metatable for other lists as well (MetaList, [Blocks], [Inlines]).

I currently, after some experimenting, see three options for attributes:

1. Create a meatable for attributes such that they behave like tables for string keys, but keep the current associated list as-is. Resulting syntax: div.attributes.style = 'color:red'; advantages: mostly[^1] backwards compatible, pleasant to use, fast serialization/deserialization; disadvantage: inefficient access.
2. Provide an AssocList 'class' and make attributes an instance of it. Syntax: div.attributes:set('style', 'color:red'); advantages: fully backwards compatibel, transparent; disadvantage: performance as above, slightly less convenient.
3. Use normal table instead of a list of pairs. Not really feasible, as it would either require changes to Attr.

Right now I'm leaning towards 2., but 1. is tempting as well.

[jgm]

I'd go for 1, but I don't understand fully what is not
included in "mostly backwards compatible".

Note: in addition to convenient setters, we need convenient
getters: something like if div.attributes.style == 'color:red'.

+++ Albert Krewinkel [Nov 18 17 19:50 ]:

Adding a List metatable to the class attributes is simple enough and
shouldn't saffect performance significantly. We might want to think
about setting that metatable for other lists as well (MetaList,
[Blocks], [Inlines]).

I currently, after some experimenting, see three options for
attributes:

1. Create a meatable for attributes such that they behave like tables
   for string keys, but keep the current associated list as-is.
   Resulting syntax: div.attributes.style = 'color:red'; advantages:
   mostly backwards compatible, pleasant to use, fast
   serialization/deserialization; disadvantage: inefficient key
   access.
2. Provide an AssocList 'class' and make attributes an instance of it.
   Syntax: div.attributes.set('style', 'color:red'); advantages: fully
   backwards compatibel, transparent; disadvantage: performance as
   above, slightly less convenient.
3. Use normal table instead of a list of pairs. Not really feasible,
   as it would either require changes to Attr.

Right now I'm leaning towards 2., but 1. is tempting as well.

—
You are receiving this because you commented.
Reply to this email directly, [1]view it on GitHub, or [2]mute the
thread.

References

1. https://github.com/jgm/pandoc/issues/4071#issuecomment-345466611
2. https://github.com/notifications/unsubscribe-auth/AAAL5KcVEISebIAoXK6PGMUDN6hl_7Opks5s3zUCgaJpZM4Qd_XI

[tarleb]

I had just (slightly too late) finished editing the comment to explain "mostly backwards compatible".

The only interesting change here would be pairs.

for k, v in pairs(attributes) do
  print(k, v)
end

Given attributes style="color:red" lang="de", the current implementation gives

1       table …
2       table …

while it would feel more natural to adapt the __pairs metamethod such that it would print

style   color:red
lang    de

[tarleb]

Proof of Concept for 1

[jgm]

I think there aren't enough filters yet that we should let
this small backwards-compatibility issue worry us. Let's
try to get the most intuitive interface we can.

+++ Albert Krewinkel [Nov 18 17 21:33 ]:

I had just (slightly too late) finished editing the comment to explain
"mostly backwards compatible".

The only interesting change here would be pairs.

for k, v in pairs(attributes) do
print(k, v)
end

Given attributes style="color:red" data-language="de", the current
implementation gives
1 table …
2 table …

while it would feel more natural to adapt the __pairs metamethod such
that it would print
style color:red
lang de

—
You are receiving this because you commented.
Reply to this email directly, [1]view it on GitHub, or [2]mute the
thread.

References

1. https://github.com/jgm/pandoc/issues/4071#issuecomment-345472895
2. https://github.com/notifications/unsubscribe-auth/AAAL5JQlFONm5MkjFcUPPn038jFeTMvPks5s300hgaJpZM4Qd_XI

[tarleb]

I should add that a __pairs metamethod, AFAIK, only works with Lua 5.2 and 5.3, but with neither LuaJIT nor Lua 5.1.

[fiapps]

I would prefer option 1 to option 2.

As for efficiency, I agree with your choices (options 1 and 2): give the priority to serialization and deserialization performance, with the expectation that many filters will not even look at attributes. I would also expect the common case to be small numbers of attributes on elements, so that O(n) access time would not be a problem.

[tarleb]

I opened #4080 to fix attribute behavior and #4081 to keep track of the additional improvements for working with lists.