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.

Sign up for GitHub

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

Jump to bottom

Recommend single indent, not double indent #223

Merged
merged 4 commits into from
Oct 2, 2024
Merged

Recommend single indent, not double indent #223

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6c4026a
Recommend single indent, not double indent
hadley Sep 30, 2024
7bbacee
Respond to feedback; add S7 content
hadley Oct 1, 2024
b077972
Merge commit '5ffe2e3de85c1d3b5f6619d6619d9a466b36f902'
hadley Oct 1, 2024
4aac7aa
Feedback from review
hadley Oct 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions functions.Rmd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,23 @@ There are two options if the function name and definition can't fit on a single
}
hadley marked this conversation as resolved.
Show resolved Hide resolved
```

* Double-indent: Place each argument of its own **double** indented line.
The trailing `)` and leading `{` go on the same line as the last argument.

* Single-indent: Place each argument of its own **single** indented line.
hadley marked this conversation as resolved.
Show resolved Hide resolved

```{r, eval = FALSE}
long_function_name <- function(
a = "a long argument",
b = "another argument",
c = "another long argument") {
a = "a long argument",
b = "another argument",
c = "another long argument"
) {
# As usual code is indented by two spaces.
}
```

In both cases the trailing `)` and leading `{` should go on the same line as the last argument.
The trailing `)` and leading `{` go on a new line.

Prefer function-indent style to double-indent style when it fits.
Prefer function-indent style to single-indent style when it fits.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That might be one of the few formatting settings we would implement in Ark for legacy formatting: an option between function-style if it fits (legacy), or single-indent style everywhere ("modern"). Another very similar option would be call-indent style versus single-indent style for function calls, the former matching the RStudio default setting and the latter matching the tidyverse style.

But if we're bold enough, I now think the default / enforced style should be single-indent style to match JS/TS and Rust styling.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea so the "legacy" formatter option logic would be:

  • Legacy
    • If it fits, don't break the arguments over multiple lines at all, else
    • If it fits, use function-style line breaks, else
    • Fall back to single-indent line breaks
  • Modern
    • If it fits, don't break the arguments over multiple lines at all, else
    • Fall back to single-indent line breaks

I think that Modern (or whatever we call it) would probably result in less code diffs in the long term, at the cost of one big set of upfront diffs as it changes existing function-style -> single-indent style everywhere. I think I'll probably end up preferring this.

Maybe the style guide should just not set a preference between these two styles right now? We can add a preference back in as we feel out the formatter.


These styles are designed to clearly separate the function definition from its body.

Expand Down
Loading