-
Notifications
You must be signed in to change notification settings - Fork 111
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
Need clarity on named vs. unnamed arguments and line breaks #212
Comments
The confusion likely stems from the brevity of the conds <- concatenate_elements_to_vector(
conds,
second_argument,
third_argument
) This format distinctly shows that conds <- concatenate_element_to_vector(conds,
second_argument,
third_argument
) The separation emphasizes |
I don't disagree, though But note that your case is far more exceptional -- at 29 characters it's at the limit of
about 0.5% of calls have such a long name, while other common 1-char calls include all_call_names <- function(f) {
f |>
parse() |>
xmlparsedata::xml_parse_data() |>
xml2::read_xml() |>
xml2::xml_find_all("//SYMBOL_FUNCTION_CALL") |>
xml2::xml_text()
}
# inside dplyr/R
list.files() |>
lapply(all_call_names) |>
unlist() |>
nchar() |>
table() |
I agree that the current advice is suboptimal. There are three cases that I think would be good to consider. (1) You have one "primary" argument, or you're mutating an object, as @MichaelChirico's (2) Sometimes, particularly in paste(
"Once upon a time, there was a", adjective, noun,
"who loved to", verb, adverb, ". ",
"It was the funniest thing ever!"
) (3) I also don't love the case where you have a short unnamed argument and then a couple of long named arguments, e.g. x <- map(
x,
f,
a_long_argument_name = 1
) But I don't have a good sense of how to fix the advice to handle all these cases. |
See also #189 |
(2) is already mentioned in the guide. And (3) is the topic of #189. But I'm not sure I can figure out what the rule is for (1) because I'd never write this: df <- mutate(df,
y = y + a_very_very_very_very_very_very_very_very_very_very_long_function_name(z)
) And looking again at the motivating example, I'd be tempted to rewrite like: extra_cond <- "self::OP-RIGHT-PAREN[preceding-sibling::*[not(self::COMMENT)][1][self::OP-LEFT-PAREN or self::OP-COMMA]]"
conds <- c(conds, extra_cond) So I don't think this is quite ready to become official style advice. |
In #39 we have this quote:
Downthread it's pointed out this is not very clear in the style guide as of now.
I'd also like to point out that I find it to be less-than-optimal advice. This came up in {lintr} here, where we have:
https://github.com/r-lib/lintr/pull/2543/files#diff-f32b4985bf0c6dd105640050567eb5e0d6d097c1e6e365a69a0e5a4c24dc2b13R40
I think it's very good to put
conds
on the same line asc(
here because it makes it very clear we're overwriting the same object in a way that's not as transparent if we forceconds
to come on the next line.Our usage is also in line with the current style guide, but not the text of the issue linked, so I'm hoping this discussion can be revisited.
The text was updated successfully, but these errors were encountered: