Code style #663
Code style #663
Conversation
| @@ -48,7 +48,6 @@ NULL | |||
| #' @param ... other values passed to [all.equal()] | |||
| expect_equal <- function(object, expected, ..., info = NULL, label = NULL, | |||
| expected.label = NULL) { | |||
|
|
|||
There was a problem hiding this comment.
Ideally styler wouldn't strip this new line. It doesn't matter here, but in general it seems like a bad idea
There was a problem hiding this comment.
Can we define a rule when to add/keep a newline after an opening brace? Is this just for top-level functions or for blocks in general?
There was a problem hiding this comment.
I think I generally prefer a newline if the function def span multiple lines as it more clearly distinguishes the body from the args.
There was a problem hiding this comment.
So should we
- Add a section to the tidyverse style guide to leave one line blank after a multi-line function declaration and
- make styler in general preserving line breaks
?
Not sure whether for the second bullet, we could also make some blank line formatting optional, but I could not think of anything particularly meaningful. Maybe one can choose to either to not touch blank lines, or to reduce the amount of blank lines to one if there is at least two blank lines next to each other.
There was a problem hiding this comment.
I'm not sure how consistently I adhere to this style. There's also the question of wrapping multiline formals - we should probably be consistent with other calls, and if all arguments don't fit on one line, each argument gets it's own line.
R/expect-self-test.R
Outdated
| @@ -67,7 +67,8 @@ show_failure <- function(expr) { | |||
| #' @rdname expect_success | |||
| #' @param path Path to save failure output | |||
| expect_known_failure <- function(path, expr) { | |||
| FailureReporter <- R6::R6Class("FailureReporter", inherit = CheckReporter, | |||
| FailureReporter <- R6::R6Class( | |||
| "FailureReporter", inherit = CheckReporter, | |||
There was a problem hiding this comment.
Why doesn't "FailureReporter" stay on the first line? (Given that it's an unnamed argument that fits)
There was a problem hiding this comment.
Because that is not yet implemented in styler. See r-lib/styler#246. For now, the line is broken after the first parenthesis with strict = TRUE if the whole call does not fit on one line.
R/reporter-junit.R
Outdated
| timer = NULL, | ||
| doc = NULL, | ||
| errors = NULL, | ||
| results = NULL, |
There was a problem hiding this comment.
I wonder if there's a way that styler could detect if the = were aligned, and if so, leave as is
R/stack.R
Outdated
| @@ -43,8 +42,8 @@ Stack <- R6Class( | |||
| ), | |||
|
|
|||
| private = list( | |||
| stack = NULL, # A list that holds the items | |||
There was a problem hiding this comment.
Preserving this alignment would also be nice
tests/testthat/test-test-that.R
Outdated
| test_that('simple', { # line 2 | ||
| expect_true(FALSE) # line 3 | ||
| }) # line 4 | ||
| context("testing testFile") # line 1 |
There was a problem hiding this comment.
Can you manually revert the changes to this file? The indenting of the comments is particularly important here
|
Thanks. I've changed alignment and applied a few more tweaks. The code of this PR doesn't change when running |
|
Reference: r-lib/styler#240 |
Each commit represents a styling level of styler. Some pieces are converted incorrectly, I have added issues to the styler repo and left the code untouched in these cases. One commit is a manual change, all other changes are automatic.
Reference:
CC @lorenzwalthert.