Skip to content

Conversation

hadley
Copy link
Member

@hadley hadley commented Apr 5, 2022

These new functions are powered by cli, given considerably better displays.

A few things to note:

  • markdown() now checks for incomplete braces; I think this is ok. Certainly it seems inconsistent to only check it for sectioned md, which is what we were doing before.
  • I overhauled the generated tags so that that include the file and line from the block. I think this is a nice (if minor) improvement; I did it mainly in order to simplify the warning code in try_find_topic_in_package().
  • Converting the tests to use snapshots revealed that a lot of the warning messages were poorly worded and/or ungrammatical.
  • Use warn(parent = err) makes it so much easier to forward on the underlying problem.

To do:

  • Add news bullet
  • Add RStudio hyperlinks

@hadley hadley requested a review from gaborcsardi April 5, 2022 16:24
@gaborcsardi
Copy link
Member

Looks great! Not only are the warnings better, the code that generates them is simpler as well. Left some comments, mostly cosmetics.

R/markdown.R Outdated
roxy_tag_warning(tag, "Unknown xml node: ", xml_name(xml))
warn_roxy_tag(tag, c(
"markdown translation failed",
x = "Internal error: unknown xml node {xml_name(xml)}"
Copy link
Member

Choose a reason for hiding this comment

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

We could be more helpful here as well, maybe suggest opening an issue?

@hadley hadley merged commit f94f43e into main Apr 8, 2022
@hadley hadley deleted the cli_warn branch April 8, 2022 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants