Fix apparent typos in sample.lua

[wlupton]

% pandoc --version // pandoc 2.9.2.1 (but not really relevant)

This PR fixes a few obvious (I hope) typos.

Notes on the changes:

• The escape() function's in_attribute argument wasn't used. I assume it was intended to enable escaping of quote characters
• The html_align() function was global but I think it's a local helper. Maybe it should be moved to the top of the file, along with the other helpers?

There are a few other things that I could fix here (or elsewhere), but I wanted to check first:

• I think that sample.lua should aim to generate the same (or very similar) output to that generated by the built-in HTML writer (although obviously it might lag a bit). Is this agreed?
• I think it would be useful to mention all the available global variables. Only PANDOC_DOCUMENT is mentioned, but I think that these are also available: PANDOC_API_VERSION, PANDOC_SCRIPT_FILE, PANDOC_STATE, PANDOC_VERSION
• I had to change the CaptionedImage() function. The sample.lua one generates a div with a p for the caption, whereas the HTML writer generates a figure and figurecaption so I changed it to do this. I also changed it to generate a bare img if the caption is empty
• The Image() function doesn't include the supplied attributes in the HTML
• The Table() function defines a global 'head' variable that isn't used and might be a hangover?
• The generated HTML uses a mixture of quote types; mostly double but some single, e.g., Link() and Image() use single quotes. I think it would be better to use only one type of quote in the generated HTML (I'd choose double quotes)
• The attributes() function uses pairs() to iterate through the attributes, which means that they're in an undefined order. I think it would be better if the order was predictable, e.g., 'id' first followed by the rest in alphabetical order? I wrote an spairs() (sorted pairs) function that has the same interface as pairs() and ipairs(), and could contribute that if it would be useful

[kysko]

I think that sample.lua should aim to generate the same (or very similar) output to that generated by the built-in HTML writer (although obviously it might lag a bit). Is this agreed?

I see it more as an example of how we can write a custom writer, in this case a HTML writer. If it can do all or most of pandoc's internal writer, good, but might not be necessary.
What should be pointed out though is that a custom writer for HTML has the advantage that a lot will be handled by the browsers, and a custom writer for markdown (for example) would need a bit more work (for lists, tables, etc). (Making a custom lua markdown writer is good practice!)

I think that these are also available: PANDOC_API_VERSION, PANDOC_SCRIPT_FILE, PANDOC_STATE, PANDOC_VERSION

Yes, I can confirm.

One could also point out that things like pandoc.List can be used, like in the lua filters; and also pandoc.system... and what else?

The Table() function defines a global 'head' variable that isn't used and might be a hangover?

I suggest you work on the sample.lua from the nightlies, since a lot has been done for Tables, and this particular function has changed very recently.
/Edit: oops! My mistake, I was thinking about the HTML table writer! See #6573 for open issue.

The attributes() function uses pairs() to iterate through the attributes, which means that they're in an undefined order. I think it would be better if the order was predictable, e.g., 'id' first followed by the rest in alphabetical order?

Agree. My own version begins with id, then class, which for me were the most important, but I leave the rest in undefined order. But maybe you're right and the rest might be in alphabetical order if A-B comparisons have to be made.

[wlupton]

Thanks for the feedback @kysko. Some follow-ups. I'll leave this for a bit to allow more discussion and then will push some more changes that reflect consensus (unless someone else wants to take this on, which would be fine by me!).

I see it more as an example of how we can write a custom writer, in this case a HTML writer. If it can do all or most of pandoc's internal writer, good, but might not be necessary.

OK. However, I think that migration from using the built-in HTML writer to using a custom HTML writer will be a common use case, so I think it would be really good if the sample lua writer produced similar HTML. Otherwise, everyone will need to go through the same process in realising and fixing the differences. Perhaps I'm really suggesting that, rather than being sample.lua, this file might become html-writer-sample.lua or even html-writer.lua (then there could also be markdown-writer.lua etc.).

My own version begins with id, then class, which for me were the most important, but I leave the rest in undefined order. But maybe you're right and the rest might be in alphabetical order if A-B comparisons have to be made.

My spairs() function takes an optional comp argument (and passes it to table.sort()) so we could image using a custom comp() that implements the "id, class, rest" ordering.

(from my original post) The Image() function doesn't include the supplied attributes in the HTML

I subsequently realised that this should be done intelligently (because of the special width and height processing). In my case I passed it naively but I was generating a fragment that was then passed into a second pandoc invocation, and the standard HTML writer did the right thing.

[wlupton]

@jgm I note that this is merged and closed. Should I move the open discussion to a separate issue (not sure what the convention is here)?

[jgm]

Thanks for these improvements.

The html_align() function was global but I think it's a local helper. Maybe it should be moved to the top of the file, along with the other helpers?

That seems right.

I think that sample.lua should aim to generate the same (or very similar) output to that generated by the built-in HTML writer (although obviously it might lag a bit). Is this agreed?

I think we need to balance two goals: reproducing the HTML writer's behavior, and functioning as a clean and relatively easy to understand example. I'm happy to accept changes to bring the filter's behavior closer to the HTML writer, but I might think twice if they introduce a lot of additional complexity.

I think it would be useful to mention all the available global variables. Only PANDOC_DOCUMENT is mentioned, but I think that these are also available: PANDOC_API_VERSION, PANDOC_SCRIPT_FILE, PANDOC_STATE, PANDOC_VERSION

Yes.

The Table() function defines a global 'head' variable that isn't used and might be a hangover?

Probably. The original sample.lua goes pretty far back.

The generated HTML uses a mixture of quote types; mostly double but some single, e.g., Link() and Image() use single quotes. I think it would be better to use only one type of quote in the generated HTML (I'd choose double quotes)

Agreed.

The attributes() function uses pairs() to iterate through the attributes, which means that they're in an undefined order. I think it would be better if the order was predictable, e.g., 'id' first followed by the rest in alphabetical order? I wrote an spairs() (sorted pairs) function that has the same interface as pairs() and ipairs(), and could contribute that if it would be useful

I don't feel strongly about this. I think it's fine. It does add a bit of complexity but people can just copy the spairs function.