Issue with persistent line break with fun("", {})

[ColinFay]

Description

Let's assume the following code with a persistent line break

list(
foo, bar)
# Which gets formatted as
list(
  foo,
  bar
)

If I switch bar for an expression, it's still formatted as expected:

list(
foo, {bar})
# gets formatted as
list(
  foo,
  {
    bar
  }
)

But if ever foo is a text string, the persistent line break is removed:

list(
"foo", {bar})
# gets formatted as
list("foo", {
  bar
})

only in cases where {} is the second argument:

list(
  "foo","bar",{bar}
)
# gets formatted as
list(
  "foo",
  "bar",
  {
    bar
  }
)

Is this an intended behavior?

Thanks.

[lionel-]

This is our handling for test_that()-like calls. We didn't hardcode testthat and instead used a general rule.

See #4 for more context.

[DavisVaughan]

I suppose we could extend

# We never break with test_that calls, specifically the rule for this is:
# - A function call
# - With 2 arguments
# - Where the first is a string
# - And the second is a braced expression

with additional rules of

- And the first argument doesn't have an existing leading line break
- And the second argument doesn't have an existing leading line break

because those are unlikely to be present in test_that() calls. And if they are present in a test_that() call, that's a super explicit request to do something weird.

That would retain the persistent line break here

list(
"foo", {bar})

And also here (another test case)

list("foo",
{bar})

I am mostly convinced this is a good idea?

[ColinFay]

Regarding test_that calls, the template from usethis::use_test() won't add a line break by default, and all the examples from the test chapter are written that way so I suppose most of the time a persistent line break will be a deliberate choice from the user ?

[lbm364dl]

This is our handling for test_that()-like calls

Is there anything written about cases where it's preferable to ignore the line width as in these?

air/crates/air_r_formatter/tests/specs/r/test_that.R.snap

Line 16 in 7125f07

test_that("description that is super long and actually exceeds the line limit but we arent going to break!", {

I haven't found any mention in the Tidyverse style guide, e.g. in the Braced expressions section.

I think this style decision should be somehow mentioned. In my case, I was also using the lintr line width check in CI, so I guess the correct suggestion would be to stop checking line width and rely on air format for too long but acceptable lines? Is this something worth mentioning here?

air/docs/configuration.qmd

Line 100 in 7125f07

While the formatter will attempt to format lines such that they remain within the `line-width`, it isn't a hard upper bound, and formatted lines may exceed the `line-width`.

[DavisVaughan]

As a general rule I do think you should remove lintr's line width check if you use Air, and let Air handle that for you. Otherwise there are just going to be places where these tools differ.

test_that() style blocks are the only ones that get this "never break" feature, but I don't think we mention it. We should add it to the Air docs in formatter.md and the tidyverse style guide

[lbm364dl]

For Tidyverse style guide, I just saw there's already an open issue from you at tidyverse/style#241.