Commas trailing #2104
Commas trailing #2104
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2104 +/- ##
=======================================
Coverage 99.65% 99.65%
=======================================
Files 118 118
Lines 5206 5208 +2
=======================================
+ Hits 5188 5190 +2
Misses 18 18
|
R/commas_linter.R
Outdated
| xpath_after <- "//OP-COMMA[@line1 = following-sibling::*[1]/@line1 and @col1 = following-sibling::*[1]/@col1 - 1]" | ||
| xpath_after <- paste0( | ||
| "//OP-COMMA[@line1 = following-sibling::*[1]/@line1 and @col1 = following-sibling::*[1]/@col1 - 1", | ||
| if (allow_trailing_comma) " and not(following-sibling::*[1]/self::OP-RIGHT-BRACKET)", |
There was a problem hiding this comment.
Maybe also allow RBB (a[[1,]]) and OP-RIGHT-PAREN (a(1,)) if the argument is set?
NEWS.md
Outdated
| @@ -40,6 +40,7 @@ | |||
| * `keyword_quote_linter()` for finding unnecessary or discouraged quoting of symbols in assignment, function arguments, or extraction (part of #884, @MichaelChirico). Quoting is unnecessary when the target is a valid R name, e.g. `c("a" = 1)` can be `c(a = 1)`. The same goes to assignment (`"a" <- 1`) and extraction (`x$"a"`). Where quoting is necessary, the linter encourages doing so with backticks (e.g. `` x$`a b` `` instead of `x$"a b"`). | |||
| * `length_levels_linter()` for using the specific function `nlevels()` instead of checking `length(levels(x))` (part of #884, @MichaelChirico). | |||
| * `if_not_else_linter()` for encouraging `if` statements to be structured as `if (A) x else y` instead of `if (!A) y else x` (part of #884, @MichaelChirico). | |||
| * `commas_linter()` add parameter `allow_trailing_comma` to allow trailing commas while indexing. | |||
There was a problem hiding this comment.
I'd suggest to use the wording "gains an option", see the bullet regarding fixed_regex_linter().
And please reference the issue number and give yourself credit for it.
R/commas_linter.R
Outdated
| #' @evalRd rd_tags("commas_linter") | ||
| #' @seealso | ||
| #' - [linters] for a complete list of linters available in lintr. | ||
| #' - <https://style.tidyverse.org/syntax.html#commas> | ||
| #' @export | ||
| commas_linter <- function() { | ||
| commas_linter <- function(allow_trailing_comma = FALSE) { |
There was a problem hiding this comment.
allow_trailing seems more concise to me since comma is already part of the linter name.
There was a problem hiding this comment.
it's also more consistent with missing_argument_linter()
|
Line 76 of I found a minimally invasive solution, but I would also be interested in the question in principle |
|
There is no tolerance, 120 is an upper limit. if (allow_trailing) {
xp_trailing_cond <- "and not(following-sibling::*[1][
self::OP-RIGHT-PAREN or self::OP-RIGHT-BRACKET or self::RBB
]"
} else {
xp_trailing_cond <- ""
}
xpath_after <- glue(
"//OP-COMMA[@line1 = following-sibling::*[1]/@line1 and @col1 = following-sibling::*[1]/@col1 - 1 {xp_trailing_cond}]"
) |
@AshesITR I have already implemented another solution, should I still implement the one suggested? |
|
It's fine, thanks! |
This is the pull request for issue #2102
This is my first PR in your project, please don't hold back with comments