Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vignette for markdown #499

Merged
merged 1 commit into from Sep 6, 2016
Merged

Vignette for markdown #499

merged 1 commit into from Sep 6, 2016

Conversation

@gaborcsardi
Copy link
Collaborator

@gaborcsardi gaborcsardi commented Sep 3, 2016

First version, comments / updates welcome.

@hadley hadley merged commit 7b3786a into r-lib:master Sep 6, 2016
3 checks passed
@hadley
Copy link
Member

@hadley hadley commented Sep 6, 2016

Looks great!

@gaborcsardi
Copy link
Collaborator Author

@gaborcsardi gaborcsardi commented Sep 6, 2016

\o/

```r
#' Use roxygen to document a package.
#'
#' This function is a wrapper for the [](roxygen2::roxygenize) function from
Copy link
Member

@jimhester jimhester Sep 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does commonmark support autolink syntax (e.g. <roxygen2::roxygenize>)? I would find it less awkward than [](roxygen2::roxygenize)

Copy link
Collaborator Author

@gaborcsardi gaborcsardi Sep 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, this is a good idea. I agree that the current syntax is awkward, but it was the best we could come up with. THere is a long discussion about it here: gaborcsardi/maxygen#3

commonmark supports autolink, but the contents are restricted:

x <- markdown_xml(text = "this is an <http://roxygen2::roxygenize> autolink")
❯ cat(x)
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE document SYSTEM "CommonMark.dtd">
<document xmlns="http://commonmark.org/xml/1.0">
  <paragraph>
    <text>this is an </text>
    <link destination="http://roxygen2::roxygenize" title="">
      <text>http://roxygen2::roxygenize</text>
    </link>
    <text> autolink</text>
  </paragraph>
</document>x <- markdown_xml(text = "this is an <roxygen2::roxygenize> autolink")
❯ cat(x)
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE document SYSTEM "CommonMark.dtd">
<document xmlns="http://commonmark.org/xml/1.0">
  <paragraph>
    <text>this is an </text>
    <text>&lt;</text>
    <text>roxygen2::roxygenize&gt; autolink</text>
  </paragraph>
</document>

So in practice we cannot really use this to create links. It is a bummer, I agree, and any more ideas are welcome.

In theory we can modify the parser as well, it is Jeroen's package. :) But that seems like a slippery slope.....

Copy link
Collaborator Author

@gaborcsardi gaborcsardi Sep 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about bringing back the backtick notation, e.g. whenever the backticks contain a single instance of ::, it is picked up as a link to a function. E.g.:

The function `roxygen2::roygenize`.
Or just simply `::roxygenize`.

We would also keep the current forms, to have a consistent notation, and the backtick forms would be just shortcuts.

Another alternative would be to use the autolink form as a shortcut, but this will not allow the :: qualified form, at least not always:

This one is fine: <roxygenize>.
This one, too: <pkg::func>. 
But this one is not: <roxygen2::roxyigenize>. Probably because of the number in the tag?

I'll dig a bit more.....

Copy link
Collaborator Author

@gaborcsardi gaborcsardi Sep 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the best alternative is the one that @hadley suggested in that thread: use bracket links, and provide the references to commonmark. E.g. this works:

This will be a link here: [roxygen2::roxygenize].

[roxygen2::roxygenize]: blah

and we would provide the reference definitions at the end, the user would not have to type them. Not sure how, but we could pre-parse the text, and stick there everything we could find?

Copy link
Member

@jimhester jimhester Sep 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From http://spec.commonmark.org/0.25/#autolinks looks like it just looks for a scheme followed by a :. So you could use any two or more letters followed by a :, e.g.

cat(commonmark::markdown_xml("<ref:roxygen2::roxygenize>"))
#> <?xml version="1.0" encoding="UTF-8"?>
#> <!DOCTYPE document SYSTEM "CommonMark.dtd">
#> <document xmlns="http://commonmark.org/xml/1.0">
#>   <paragraph>
#>     <link destination="ref:roxygen2::roxygenize" title="">
#>       <text>ref:roxygen2::roxygenize</text>
#>     </link>
#>   </paragraph>
#> </document>

roxygen2 not working as a scheme actually seems to be a bug in commonmark to me. According to the spec a scheme is supposed to be any alphanumeric so I believe it should be valid.

Copy link
Collaborator Author

@gaborcsardi gaborcsardi Sep 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed looks like a bug: commonmark/cmark#153

So if this is fixed, it is almost good:

  • <package::function> works
  • <function> works (it parses as inline HMTL, but we can work with that)

Unfortunately

  • <foo.bar> and
  • <foo_bar> will not work

:(

Copy link
Member

@jimhester jimhester Sep 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for sending the PR. For the latter two cases people could always use pkg:: syntax even within the package they are documenting, but I agree that isn't great :(.

Another alternative is to have a pre-processing step where we add a fake scheme to any <> links that don't already have a common scheme https?, ftp etc. Could use http://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml or a subset as the list of common schemes and just add e.g. xyz:, which lets you have (almost) arbitrary text in the autolink.

Copy link
Collaborator Author

@gaborcsardi gaborcsardi Sep 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preprocessing could be possible, but it could also be fragile. E.g. you would not want to pre-process within code, and there could be other exceptions as well. Even just determining whether you are inside code or not would basically need a markdown parser....

I think, however, that the following could work to get the [pkg::fun] and also [fun] notation:

  1. parse the text, extract the [pkg::fun] and [fun] forms. All of them, even if they are in code, or wherever.
  2. add them to the end of the text, after two newlines, so that cmark can use them as anchors. E.g. [pkg::fun]: dummy.
  3. the destination must be some random marker, instead of the dummy here, and then when see the marker in the parsed xml, we'll know that this is a link to R documentation.
  4. run the parser
  5. find the <link> tags with the random marker as the destination, and replace them with \code{\link{...}}.

The unused reference definitions at the end are just ignored, so if we picked up something from a code block, nothing happens:

❯ cat(markdown_xml("foo [pkg::fun] bar `[code]` bar bar\n\n[pkg::fun]: dummy\n[code]: dummy2"))
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE document SYSTEM "CommonMark.dtd">
<document xmlns="http://commonmark.org/xml/1.0">
  <paragraph>
    <text>foo </text>
    <link destination="dummy" title="">
      <text>pkg::fun</text>
    </link>
    <text> bar </text>
    <code>[code]</code>
    <text> bar bar</text>
  </paragraph>
</document>

Copy link
Collaborator Author

@gaborcsardi gaborcsardi Sep 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened an issue for this: #505 FYI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants