Named arguments not forced to one per line?

[hadley]

I had expected

new("Period", x@.Data[i],
  year = x@year[i], month = x@month[i],
  day = x@day[i], hour = x@hour[i], minute = x@minute[i]
)

to be restyled to

new("Period", x@.Data[i],
  year = x@year[i], 
  month = x@month[i],
  day = x@day[i], 
  hour = x@hour[i],
  minute = x@minute[i]
)

Since https://style.tidyverse.org/syntax.html#long-lines says "use one line each for the function name, each argument, and the closing )".

This rule should only apply to named arguments, and I'd expect the following code to be left as is:

show <- paste(
  x@year, "y ", x@month, "m ", x@day, "d ",
  x@hour, "H ", x@minute, "M ", x@.Data, "S",
  sep = ""
)

(I'm spotting these cases because I've started restyling lubridate, which has a mismash of code styles from years of development. styler has made a big improvement and I really appreciate all the work you have put in this package 😄)

[hadley]

A somewhat related situation is:

poslt <- as.POSIXlt("2010-02-03 13:45:59", tz = "GMT", format = "%Y-%m-%d %H:%M:%S")

which because is >80 characters wide, I'd expect to get restyled as:

poslt <- as.POSIXlt(
  "2010-02-03 13:45:59", 
  tz = "GMT", 
  format = "%Y-%m-%d %H:%M:%S"
)

# or from a strict reading of the style guide
poslt <- as.POSIXlt("2010-02-03 13:45:59", 
  tz = "GMT", 
  format = "%Y-%m-%d %H:%M:%S"
)

[hadley]

Another similar example is:

poslt <- as.POSIXlt(c("2010-02-14 01:59:59", "2010-02-15 01:59:59", "2010-02-16
  01:59:59"), tz = "UTC", format = "%Y-%m-%d %H:%M:%S")

which definitely has to become

poslt <- as.POSIXlt(
  c("2010-02-14 01:59:59", "2010-02-15 01:59:59", "2010-02-16 01:59:59"), 
  tz = "UTC", 
  format = "%Y-%m-%d %H:%M:%S"
)

since the c() can't fit on one line with as.POSIXct().

---

If I add a couple more date-times to get this:

poslt <- as.POSIXlt(c("2010-02-14 01:59:59", "2010-02-15 01:59:59", "2010-02-16 01:59:59",
  "2010-02-16 01:59:59", "2010-02-16 01:59:59"), tz = "UTC", format = "%Y-%m-%d %H:%M:%S")

Then restyling gives me:

poslt <- as.POSIXlt(c(
  "2010-02-14 01:59:59", "2010-02-15 01:59:59", "2010-02-16 01:59:59",
  "2010-02-16 01:59:59", "2010-02-16 01:59:59"
), tz = "UTC", format = "%Y-%m-%d %H:%M:%S")

whereas I would prefer:

poslt <- as.POSIXlt(
  c(
    "2010-02-14 01:59:59",
    "2010-02-15 01:59:59",
    "2010-02-16 01:59:59",
    "2010-02-16 01:59:59",
    "2010-02-16 01:59:59"
  ),
  tz = "UTC",
  format = "%Y-%m-%d %H:%M:%S"
)

[MichaelChirico]

doesn't this apply as a reason not to always restyle named arguments?

You may also place several arguments on the same line if they are closely related to each other..

this for example helps plot() calls from taking up a lot of vertical real estate by e.g. allowing xlim=, xlab=, xaxt=, las=, to be on the same line if they fit.

[hadley]

I'm pretty sure I meant that advice to only apply to unnamed arguments.

[lorenzwalthert]

Thanks @hadley. I agree on your first post. I was aware of this, but given other more urgent issues and my time constraints, it's not implemented yet.

On the other posts: They would in my opinion be resolved if the rule for 80 characters was implemented, which is the most upvoted unclosed issue that is quite complex to implement I anticipate: #247. I have not started with it yet and don't think I will soon.

[hadley]

Ah ok, makes sense. The other thing that I've been wondering about is maybe if it would make sense to supply styler with a few manual formatters that would (e.g.) allow to you toggle between all arguments on one line to each argument on its own line. Obviously that wouldn't help with restyling a complete package, but by binding it to a keyboard shortcut, it would make it very easy to restyle as needed.

[lorenzwalthert]

The other thing that I've been wondering about is maybe if it would make sense to supply styler with a few manual formatters that would (e.g.) allow to you toggle between all arguments on one line to each argument on its own line.

We aimed for a non-invasive formatter when building the package and allowed many configuration options, i.e. if you check the tidyverse_style signature:

tidyverse_style(
  scope = "tokens",
  strict = TRUE,
  indent_by = 2,
  start_comments_with_one_space = FALSE,
  reindention = tidyverse_reindention(),
  math_token_spacing = tidyverse_math_token_spacing()
)

More prominent styling aspects have their own argument, the rest is grouped into the residual strict, the effects of it are described in this vignette.

Hence, the most natural extension would just be to add one_named_argument_per_line or similar and set the default to FALSE (to maintain backward compatibility). Once styled with TRUE, each argument would be placed on one line and consequentially, styling with the default would not remove any line breaks (but also not enforce TRUE for newly contributed code). A related idea is to use a config file to make it easy for people to use a non-default config as proposed in #319, but that itself is quite an untertacking.

My main concern with these special formatters you suggested is that they are a bit of a one-way street, in particular for the case outlined in this issue, as long as there is no character limit implemented (e.g. with #247). I don't think anyone would want to force putting all arguments on the same line. Hence, I'd prefer to just implement another argument for tidyverse_style(). What do you think of that? If you feel really strongly about the one argument per line, we could also make TRUE the default at some point, maybe along with some other big changes as sketched in #769.

[hadley]

What I was thinking has been implemented in https://github.com/lionel-/codegrip/, so I'm going to close this issue.

[lorenzwalthert]

Ok, good to know. But since we want to be style guide compliant, I think we’ll eventually also implement that functionality.