Skip to content
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

Newline after multiline arguments? #209

Closed
Tracked by #1231
krlmlr opened this issue Dec 2, 2023 · 4 comments
Closed
Tracked by #1231

Newline after multiline arguments? #209

krlmlr opened this issue Dec 2, 2023 · 4 comments

Comments

@krlmlr
Copy link
Member

krlmlr commented Dec 2, 2023

This looks fairly awkward to me:

styler::style_text("
list(
  c(
    a
  ), c(
    b
  )
)
")
#> list(
#>   c(
#>     a
#>   ), c(
#>     b
#>   )
#> )

Created on 2023-06-15 with reprex v2.0.2

I think there should be a newline after ), . Is there a rule in the style guide? Should there be one?

@MichaelChirico suggests that enforcing the newline creates wasted space. While true, I also find the comma between ) and c( difficult to spot, and I'm happy to trade the extra lines for readability here.

Downstream: r-lib/styler#1133 (which also suggests to discuss here first).

@MichaelChirico
Copy link
Collaborator

MichaelChirico commented Dec 3, 2023

This is the relevant section:

https://style.tidyverse.org/syntax.html#long-lines

Is your example really representative? In particular the second argument being unnamed. I find that often in practice examples like yours can be reflowed with explicit arguments instead:

# foo = function(descriptive_name1, descriptive_name2)
foo(
  c(
    a
  ), c(
    b
  )
)

foo(
  descriptive_name1 = c(
    a
  ),
  descriptive_name2 = c(
    b
  )
)

It might help to share some specific package code you've come across that'd be affected by the rule here.

@krlmlr
Copy link
Member Author

krlmlr commented Dec 3, 2023

If a function call is too long to fit on a single line, use one line each for the function name, each argument, and the closing ).

Does the ), c( violate this very rule because parts of two arguments are on the same line?

Occasionally, I create unnamed lists of vectors, with foo === list in your example. (+1 for getting === into base R.)

@hadley
Copy link
Member

hadley commented Aug 9, 2024

Yes, there definitely should be a new line there, yielding:

list(
  c(
    a
  ),
  c(
    b
  )
)

(and this is what codegrip generates)

I agree that this violates the "no more than one argument per line" rule.

@hadley
Copy link
Member

hadley commented Sep 30, 2024

So I think this is covered by the style guide (or at least is sufficiently esoteric we don't need to explicit mention it)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants