only set info attribute for language blocks

[zkamvar]

This will fix #23 via https://github.com/ropenscilabs/tinkr/pull/24/files#diff-cf43f9aae06899fecb008822d7d986c4R113, which sets the "info" attribute instead of resetting ALL attributes. The only potential drawback is if any of the chunk-derived attributes interact with the stylesheet, but I can't see any pathways so far.

I've also added some documentation in the README about the process for adding new elements (which really should be a vignette on its own):

path <- system.file("extdata", "example2.Rmd", package = "tinkr")
yaml_xml_list <- tinkr::to_xml(path)

# Namespace comes from commonmark
xml2::xml_ns(yaml_xml_list$body)
#> d1 <-> http://commonmark.org/xml/1.0

new_chunk <- xml2::read_xml("<code_block language='r' name='xml-block'></code_block>")

# set the namespace of the new chunk
xml2::xml_set_attr(new_chunk, "xmlns", xml2::xml_ns(yaml_xml_list$body))

# add code in the new chunk
# NOTE: code MUST end in a newline character
xml2::xml_set_text(new_chunk, "cat('this is a new chunk from {tinkr}')\n")
#> {xml_document}
#> <code_block language="r" name="xml-block" xmlns="http://commonmark.org/xml/1.0">

# Add chunk into document
xml2::xml_add_child(yaml_xml_list$body, new_chunk, .where = 1L)

newfile <- tempfile(fileext = ".Rmd")
tinkr::to_md(yaml_xml_list, newfile)
cat(readLines(newfile, 16), sep = "\n")
#> ---
#> title: "Untitled"
#> author: "M. Salmon"
#> date: "September 6, 2018"
#> output: html_document
#> ---
#> 
#> ```{r setup, include=FALSE, eval=TRUE}
#> knitr::opts_chunk$set(echo = TRUE)
#> ```
#> 
#> ```{r xml-block}
#> cat('this is a new chunk from {tinkr}')
#> ```
#> 
#> ## R Markdown

[maelle]

why, btw?

[zkamvar]

I'm not sure! I think it has something to do with a fencepost problem in the stylesheet because the last line of code is accessible before it hits xslt::xml_xslt(). I think it may be worthwhile to open an issue to figure out what's going on, but probably not too high priority IMO.

[maelle]

Not sure if helpful but I used your edit of to_md() and the code below

path <- system.file("extdata", "example2.Rmd", package = "tinkr")
yaml_xml_list <- tinkr::to_xml(path)
# Add chunk into document
xml2::xml_add_child(yaml_xml_list$body, 
                    "code_block",
                    "this is a new chunk from {tinkr}\n",
                    language='r', name='xml-block', 
                    xmlns=xml2::xml_ns(yaml_xml_list$body),
                    .where = 1L)
tou <- tempfile(fileext = ".Rmd")
tinkr::to_md(yaml_xml_list, tou)
file.edit(tou)

I was wondering whether it was more natural to add a chunk this way? I.e. not creating a node, but instead using the "..." of xml2::xml_add_child(). I can't remember where I saw that, I used GitHub code search to look at examples.

The namespace thing is tricky, should your PR actually add a helper function for adding childs (and siblings and parents?)? The arguments would be everything but not the namespace, that tinkr would set. The function would also check the text ends with a newline, if there's text.

I saw you're into R6 and I wonder whether that's the route we should go in tinkr too instead of using a list?

[zkamvar]

I was wondering whether it was more natural to add a chunk this way? I.e. not creating a node, but instead using the "..." of xml2::xml_add_child(). I can't remember where I saw that, I used GitHub code search to look at examples.

YES! 💯 I think the method you proposed is a MUCH better way of doing things. I admit that I'm always a bit perplexed by the {xml2} documentation, and never really understood what was going on with the ..., but now it makes sense!

The namespace thing is tricky, should your PR actually add a helper function for adding childs (and siblings and parents?)? The arguments would be everything but not the namespace, that tinkr would set. The function would also check the text ends with a newline, if there's text.

I think adding helper functions for these situations would be quite nice. The only thing is that the code block example is one of the simplest situations because you only need the code block and the text that goes inside. It gets more complex with almost any other element.

One avenue we might explore is having a helper function that inserts user markdown as nodes by passing it through commonmark::markdown_xml() and then taking the children of that new document. That's was one of the strategies I found to work with dealing with list elements in {pegboard}

I saw you're into R6 and I wonder whether that's the route we should go in tinkr too instead of using a list?

I'm ambivalent about this. I've found it to be quite useful for my own tasks and it reinforces the concept that {xml2} objects work by side-effect, but on the other hand, it may not be strictly necessary. I guess it depends on what the perceived audience will do with it. 🤷

[maelle]

Well for now the perceived audience is you and me 🤣

[zkamvar]

It gets more complex with almost any other element.

This may not necessarily be the case AND I just realized that this could tie into #20 because if the intention is to allow users to insert any random markdown, then it can be inserted into a text node and it would come out in the rendered document as markdown. The only things we would need to do is to add a tag to that text that would prevent it from be escaped on its way out. I used this strategy in {pegboard}.

[zkamvar]

@maelle, do you think this can be merged? This bugfix would really help me refactor {pegboard}.

[zkamvar]

Now that <SNAP! voice> I've got the power </SNAP! voice> to merge PRs, I will do this self-merge since I have addressed @maelle's requests.