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

More general handling of { blocks in argument lists #142

Open
lionel- opened this issue Jan 10, 2025 · 2 comments
Open

More general handling of { blocks in argument lists #142

lionel- opened this issue Jan 10, 2025 · 2 comments

Comments

@lionel-
Copy link
Collaborator

lionel- commented Jan 10, 2025

We currently have special support for trailing { blocks and trailing lambdas. The goal is to prevent multi-line { blocks from causing expansion of arguments in cases like these:

with(data, {
  x + y
})

map(xs, function(x) {
  x + 1
})

The implementation is currently borrowed from Biome which has to deal with similar cases such as JS lambda functions (#4). It is rather complicated and involves a best fitting algorithm. It supposes that the { blocks are either leading (unused in Air) or trailing. However it's been pointed out to us that the same behaviour would still be desirable in non-trailing case, for instance if optional arguments are trailing:

expect_snapshot({
  ...
}, error = TRUE)

And in fact there are important classes of functions that rely on trailing arguments in this way, e.g. the base higer order functions like Find() or Reduce().

Find(function(x) {
  ...
}, xs, right = FALSE)

We could leverage Biome's leading group feature to implement this. But I think it's worth exploring the feasibility of a simpler approach where we'd simply treat all { blocks as "resetting" the line width. This would handle all blocks in the same manner, whether they are leading, trailing, or not. E.g. it'd be allowed to write the following without causing an expansion:

f(1, {
  ...
}, 2, 3, {
  ...
}, 4)

Expansion would still be possible by leveraging magic line breaks, e.g. in the above example you'd add a newline before 1 to force expansion, or remove it to request the flat layout.

@DavisVaughan
Copy link
Collaborator

I'm on board with first-argument { support, but I'm still not quite sure about this one

f(1, {
  ...
}, 2, 3, {
  ...
}, 4)

I feel like you visually "lose" the 2 and 3 arguments pretty easily there. It's also probably not very common of a style.

If it makes the algorithm wayyyyyy simpler then I guess it doesn't matter too much, but if not then I'd also be ok with just adding in first-argument support to mirror our last-argument support.

@lionel-
Copy link
Collaborator Author

lionel- commented Jan 14, 2025

There are specific (and very rare) applications where you chain { blocks and I figured we might as well support those consistently.

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

2 participants