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

Space between function right parenthesis and function open curly #242

Closed
jennybc opened this issue May 12, 2017 · 7 comments
Closed

Space between function right parenthesis and function open curly #242

jennybc opened this issue May 12, 2017 · 7 comments
Labels
feature a feature request or enhancement good first issue ❤️ good issue for first-time contributors

Comments

@jennybc
Copy link
Member

jennybc commented May 12, 2017

It feels like lintr should complain about the lack of space between the right paren and opening curly brace here (occurs twice):

if (length(foo) == 0L){
    if (verbose){
      message("message")
    }
    invisible(NULL)
  } else
    req_tbl[keep_names, ]
@jennybc
Copy link
Member Author

jennybc commented May 12, 2017

Another issue brought up by that snippet: seems like the else statement should have brackets. Either because it always should or because the if used brackets. If that's correct, I can open a second issue or someone else can.

@jimhester
Copy link
Member

I agree about the spacing before the {, but I don't think any of the linters currently check for that.

My personal style is to always use braces for all conditionals, but some people feel the opposite and the tidyverse style is mostly silent on this matter (http://style.tidyverse.org/syntax.html#indenting).

I guess tidyverse style guide does say to use braces on all multiline conditionals, so it probably should be linted.

@jennybc
Copy link
Member Author

jennybc commented May 12, 2017

I say: he who controls the default linters has the power 👊.

@xanderdunn
Copy link

I'd like to see a linter that checks for this as well

@ashiklom
Copy link

Seconding (thirding? fourthing?) this. It could it its own linter, and maybe even one that isn't enabled by default. E.g. paren_brace_space_linter.

@russHyde russHyde added feature a feature request or enhancement good first issue ❤️ good issue for first-time contributors labels Jan 11, 2019
@dschlaep
Copy link

I would like to see this implemented as well, either as stand-alone linter as previously suggested or folded into an existing one, e.g., open_curly_linter.

Interestingly, Rstudio (Version 1.1.463) shows above example as a lint case even though lintr::lint() does not, see attached screenshot.
Screen Shot 2019-03-11 at 15 42 58

Thanks!

@jimhester
Copy link
Member

RStudio does not use lintr, it uses a completely different codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement good first issue ❤️ good issue for first-time contributors
Projects
None yet
Development

No branches or pull requests

6 participants