Fix default metadata addition for pandoc 2.8 change of behavior

[cderv]

This fixes #1740

I began with the easy way, just prepending if pandoc >= 2.8
This seem to work well

@yihui what do you prefer ?
This sort of simple way to deal with this or the use of --metadata-file ?

In fact, I tried the later, but at these lines in render.R, where R file is spinned, it does not seems that we can access yet the output format to add some pandoc args directly. We can add it later like post_knit_handler or just add it anywhere if a temporary yaml file has been created I guess. Even if it seems the way to go, it seems maybe to complicated to implement (more lines and logic to add)

What do you think ?
I'll modify or complete this PR based on your input.

[cderv]

I answered my own question : Prepending the document is really not a good idea as yaml_front_matter is useful in a lot of place. I tried to implement the use of --metadata-file pandoc argument. I used a test of existence with exists, not sure it is in your good practice. You'll tell me.

You may have another way to do this.

I was thinking of adding a args <- c() to append to, from the start of render to the run_pandoc step, but this does not seem the way you took currently, adding pandoc arg when required.

Also the use of --metadata-file could be activated from 2.3 but I left 2.8 for preventing any unnecessary changes.

[yihui]

Thanks! I'd just examine the YAML metadata provided in the R script, exclude the fields that have already been provided, and only append the fields that do not exist in the R script. In other words, we just resolve the possible conflicts on our side, instead of relying on Pandoc's overriding mechanism. This should work for any version of Pandoc.

[yihui]

BTW, I forgot to mention one important thing: I have noticed that you have the excellent habit of adding tests in PRs, and that's really great! In this PR, although I changed your implementation, your contribution of the tests was still extremely valuable to make me feel more confident in my changes. By comparison, sometimes I'm too lazy to add tests... Whenever you feel any tests are missing, please feel free to add them. Thank you!

[cderv]

Thanks !

I think tests are really important, all the more when your more than one on the same code. It helps me feel safer when I tend to change existing code that impacts a current behaviour. We should have test for every feature we have and add. It is why on this type of PR, I often write test first for the current behavior and the one we want, and then make change verifying it does not break and have the correct behavior. I am glad it helped you too.

testthat is a really powerful framework to write clean dans small test so it helps a lot. I don't mind writing tests so I'll add some when I find they are missing. 👍

And I completely understand the change - Part of the work on an issue is identifying the root core of the problem and come up with a first solution. So if you have a better implementation (as in this case 👍 ), it is more than welcome to change mine 😉

[yihui]

Tests are great, until they are run on Solaris 😁

https://cran.r-project.org/web/checks/check_results_rmarkdown.html

Don't worry about it. It was my oversight. With 6f4fbd5, it won't happen again.

[cderv]

Oh good to know about Solaris.
With this fix, yeah it won't be an issue anymore 😄