Sort keys when encoding tables as JSON

[winniehell]

Description

Sorts object keys when encoding a table as JSON. This makes JSON objects deterministic which is especially useful for the GLightbox parameters which are part of the rendered HTML. This change may have unintended side effects such as performance degradation which I hope will be revealed by a review.

closes #11122

Checklist

I have (if applicable):

• filed a contributor agreement.
• referenced the GitHub issue this PR closes
• updated the appropriate changelog

[winniehell]

I verified locally that this change resolves #11122 but of course it has larger impact on the code base which I'm not sure is wanted.

[cscheid]

Thanks for the PR. The fix makes sense, but json.lua is an upstream dependency, see the header:

--
-- json.lua
--
-- Copyright (c) 2020 rxi
-- https://github.com/rxi/json.lua
--

I have a slight preference towards making a fix upstream first.

[winniehell]

@cscheid I can try but that library has not seen any activity in the last four years, so I'm not optimistic it will get merged any time soon.

[cscheid]

I'd be more willing to take a PR here if the fix were mergeable upstream first. Your fix creates a dependency between json.lua and Quarto, and that's not really wanted in this context.

[winniehell]

yep, I understand. 👍 the alternative however implies that sortedPairs is duplicated upstream which leads to at least two copies in Quarto. There are already multiple anyway so this may not be a big deal.

[cscheid]

We're not a go shop but the appropriate proverb applies here: A little copying is better than a little dependency.

[winniehell]

We are in agreement here and for this situation and I also dislike big dependency trees. But I have also made bad experience with legacy code bases that had a lot of duplication and some of the duplicates were adjusted to behave slightly different. So there is probably a sweet spot somewhere in the middle (and just to be absolutely clear: I'm not suggesting any change in strategy for Quarto here, just sharing my point of view).

[winniehell]

upstream pull request: rxi/json.lua#51

[cscheid]

Great! Do you want to amend this PR with those changes? We can take this locally now, and monitor the upstream repo to get the changes when they merge it. Thank you!

[winniehell]

@cscheid yep, I have also included the other unreleased changes from upstream and created a pull request for your earlier changes: rxi/json.lua#52

[cscheid]

This looks great, thank you.

We're still missing the changelog update, right?

[cscheid]

Merged - thanks for the PR and followup!