Don't overwrite but merge keys for pandoc

[jkrumbiegel]

Description

This PR fixes an issue I noticed when writing a typst template that was supposed to use metadata defined under the params key, so that both the template and some julia code in the notebook could inspect the relevant values. I could not access values defined in _quarto.yml and first thought it was an issue about params itself, which turned out not to be the case.

Here's a repo where I pushed an MWE with a basic qmd file and typst template https://github.com/jkrumbiegel/quarto-mwe-pandoc-metadata

When rendering this with quarto, I got the following result:

You can see that only the data from _quarto.yml that does not share keys with the file's frontmatter actually reaches the template. I logged two relevant bits of data from pandoc.ts and you can see that this would be the behavior when replacing the first object's keys with those present in the second object:

// options.format.metadata

{
  "revealjs-plugins": [],
  params: { nested: { B: "quarto.yml", A: "frontmatter" } },
  nested_merged: { B: "quarto.yml", A: "frontmatter" },
  nested_unmerged_B: { B: "quarto.yml" },
  unnestedB: "quarto.yml",
  project: {},
  format: { "nested-typst": {} },
  nested_unmerged_A: { A: "frontmatter" },
  unnestedA: "frontmatter",
  "toc-title": "Table of contents",
  logo: { light: undefined, dark: undefined },
  "font-paths": []
}


// engineMetadata

{
  engine: "julia",
  format: { "nested-typst": "default" },
  params: { nested: { A: "frontmatter" } },
  nested_merged: { A: "frontmatter" },
  nested_unmerged_A: { A: "frontmatter" },
  unnestedA: "frontmatter"
}

I don't really understand what problem the code that does the overwriting was supposed to solve, but I thought if data from the engineMetadata is needed, maybe we should at least just merge it in, not overwrite. I assume the nested case was not considered when writing this.

With the change from this PR, I get the following render:

I need help in adding a minimal test, as the MWE I have seems too extensive and a more targeted unit test might be better suited.

I would also ask for this fix to be considered for backporting to 1.7, as we cannot update to the 1.8 series quickly and the extent of the fix is very small while its impact is larger.

Checklist

I have (if applicable):

• filed a contributor agreement.
• referenced the GitHub issue this PR closes
• updated the appropriate changelog in the PR
• ensured the present test suite passes
• added new tests
• created a separate documentation PR in Quarto's website repo and linked it to this PR

[posit-snyk-bot]

🎉 Snyk checks have passed. No issues have been found so far.

✅ security/snyk check is complete. No issues have been found. (View Details)

✅ license/snyk check is complete. No issues have been found. (View Details)

[jkrumbiegel]

After simplifying the mechanism to also handle arrays, some examples with evaluated entries break, but at this point I can't tell anymore how this is supposed to work at all :)

[cscheid]

Merging is supposed to be behavior we use, but params is a special yaml key (and yes, it's a problem that we have too many of these).

We are working on fixing some things that will (fix some things that will)* eventually let us reason more precisely about the behavior of YAML configuration. Stay tuned for the behavior of YAML blocks in our new markdown variant!

[jkrumbiegel]

This problem is not limited to params though, it applies to all nested keys. The params key was just what I found it first with, as you can only meaningfully use it in a nested way.

I would appreciate if we could find a quicker intermediate solution than a full overhaul of the markdown parsing system. (The parsing is also not the problem here, all attributes are correctly merged and available at first, and then some info is thrown away).

As you say, there are too many special keys, so the only way to avoid problems seems to be to namespace all custom keys under one key that's unlikely to conflict with anything quarto might add. But this you cannot do due to this overwriting bug.

[cscheid]

Do you have an MWE of this happening outside of params? That would make it easier for us to discuss.

[jkrumbiegel]

The MWE I posted contains both, params and no params. Consider the entry nested_merged: { B: "quarto.yml", A: "frontmatter"}, this is still correctly merged in options.format.metadata but then gets overwritten with nested_merged: { A: "frontmatter" } from engineMetadata which contains only the key A from the frontmatter and not B from _quarto.yml. I also checked that this is not engine specific, it also works incorrectly when engine: julia and the executable cell are removed.

[cscheid]

We are about to release 1.8 (hopefully tomorrow), so this isn't a PR we can merge right now. But I agree that we should try to do a faster fix early in 1.9 (the bigger changes I'm alluding to will start coming in at 1.9 as well)