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

Add code diagnostics for missing/extra commas, and for unmatched }, ), and ] #1126

Merged
merged 2 commits into from Mar 8, 2016

Conversation

wch
Copy link
Collaborator

@wch wch commented Mar 4, 2016

For example, with this ui.R:

bootstrapPage(
  div("foo"),
  ,
  div(
    "foo"
    "bar"
  )
  123, NA, x[1],
  { 45 },
  mtcars[1,,FALSE]
))

It outputs:

> runApp('temp/commas')

Listening on http://127.0.0.1:6775
Error in parse(file, keep.source = FALSE, srcfile = src, encoding = enc) : 
  /home/winston/shiny/temp/commas/ui.R:8:5: unexpected string constant
7:     "foo"
8:     "bar"
       ^
Possible extra comma at:
5:  ,
    ^
Possible missing comma at:
8:    "bar"
      ^
Possible missing comma at:
10:  123, NA, x[1],
     ^
Possible unmatched ')' at:
13:))
    ^
Warning: Error in sourceUTF8: Error sourcing /home/winston/shiny/temp/commas/ui.R
Stack trace (innermost first):
    1: runApp [/home/winston/shiny/R/server.R#696]

If an error occurs sourcing ui.R, server.R, app.R, or global.R (anything that's sourced via sourceUTF8), shiny will run diagnoseCode on that file and print out this information.

Some issues that are worth considering:

  • The first error message (with "unexpected string constant") is from R failing to parse the code; the remaining messages are from the new diagnoseCode function. It would be nice to not show R's error message and only show the messages from diagnoseCode, but I don't think that's doable in the general case because the code could have errored in any number of ways, not just the ones that the diagnoseCode function will identify.
  • The diagnostics will be used for ui.R, server.R, and app.R (if an error occurs), but if any of those files source another file, that other file won't have diagnostics.
  • The same applies to modules that are sourced.

if (!is.na(err)) {
return(err)
}
if (is.na(scope)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This could be pulled out into a separate function.

wch added a commit that referenced this pull request Mar 8, 2016
Add code diagnostics for missing/extra commas, and for unmatched }, ), and ]
@wch wch merged commit 0622326 into master Mar 8, 2016
@jcheng5
Copy link
Member

jcheng5 commented Mar 9, 2016

(For the record, @wch and I code-reviewed this in person this morning)

So lovely to get this in!

@kevinushey
Copy link
Contributor

Awesome 😄

@wch wch deleted the commas branch March 9, 2016 21:39
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.

None yet

3 participants