styler icon indicating copy to clipboard operation
styler copied to clipboard

Don't force the first switch argument in-line with switch(

Open MichaelChirico opened this issue 2 years ago • 8 comments

I think we've hewed too closely to the style guide after #714:

library(styler)

styler::style_text('
switch(
  x,
  a = 1,
  b = 2,
  c = 3
)')
# switch(x,
#   a = 1,
#   b = 2,
#   c = 3
# )

Understand the switch() example in the style guide is like that, but I think both having x on its own line and moving it to switch(x are acceptable per the overall style guide, so styler shouldn't override valid syntax.

My reasoning is per: https://style.tidyverse.org/syntax.html#long-lines

As described under Named arguments, you can omit the argument names for very common arguments (i.e. for arguments that are used in almost every invocation of the function). Short unnamed arguments can also go on the same line as the function name, even if the whole function call spans multiple lines.

This becomes worse when the first argument is an expression, not just a symbol:

library(styler)

styler::style_text('
switch(
  tolower(x$switch_column),
  a = 1,
  b = 2,
  c = 3
)')
# switch(tolower(x$switch_column),
#   a = 1,
#   b = 2,
#   c = 3
# )

Here I think the original version is much clearer.

MichaelChirico avatar Oct 25 '21 21:10 MichaelChirico

I am happy to keep it if on a separate line. Cn you file a PR?

lorenzwalthert avatar Oct 26 '21 11:10 lorenzwalthert

Sure, I will give it a go. It will take a while to get to it.

MichaelChirico avatar Oct 26 '21 17:10 MichaelChirico

Let me know how I can help. Most likely the diff for #714 is a good starting point.

lorenzwalthert avatar Oct 26 '21 18:10 lorenzwalthert

It also seems like styler insists switch() be split across multiple lines, even when it fits cleanly in just one, e.g.:

styler::style_text("switch(tolower(suffix), k = 1, m = 2, g = 3, t = 4)")
# switch(tolower(suffix),
#   k = 1,
#   m = 2,
#   g = 3,
#   t = 4
# )

MichaelChirico avatar Oct 26 '21 22:10 MichaelChirico

One liners are explicitly discouraged in the type guide, see https://github.com/r-lib/styler/issues/714#issue-785207731.

lorenzwalthert avatar Oct 27 '21 06:10 lorenzwalthert

Thanks, that's unfortunate, it really is a huge waste of space for cases when it can fit inline. I'll file upstream -- the one-line examples in the guide all use fall-through, where I can see the case for splitting it out more easily.

MichaelChirico avatar Oct 27 '21 06:10 MichaelChirico

@MichaelChirico can you add the reference to the issue in https://github.com/tidyverse/style here?

lorenzwalthert avatar Dec 16 '21 22:12 lorenzwalthert

That's https://github.com/tidyverse/style/issues/181

MichaelChirico avatar Dec 19 '21 05:12 MichaelChirico