Indentation issue in multi-line conditional expressions

[IndrajeetPatil]

Currently, the multi-line conditional expressions don't have the same indentation.

E.g., the following code, when styled with {styler} remains unchanged:

communicate_warning <- function(changed, transformers) {
  if (any(changed, na.rm = TRUE) &&
    !parse_tree_must_be_identical(transformers) &&
    !getOption("styler.quiet", FALSE)
  ) {
    cat("Please review the changes carefully!", fill = TRUE)
  }
}

Shouldn't it instead be styled to the following?

communicate_warning <- function(changed, transformers) {
  if (any(changed, na.rm = TRUE) &&
      !parse_tree_must_be_identical(transformers) &&
      !getOption("styler.quiet", FALSE)
  ) {
    cat("Please review the changes carefully!", fill = TRUE)
  }
}

---

As an aside, the current behaviour in {styler} conflicts with {lintr}, which produces lint for the styled code (cc @AshesITR):

library(lintr)

code <- 'communicate_warning <- function(changed, transformers) {
  if (any(changed, na.rm = TRUE) &&
    !parse_tree_must_be_identical(transformers) &&
    !getOption("styler.quiet", FALSE)
  ) {
    cat("Please review the changes carefully!", fill = TRUE)
  }
}'

lint(
  text = code,
  linters = indentation_linter()
)
#> <text>:3:4: style: [indentation_linter] Indentation should be 6 spaces but is 4 spaces.
#>     !parse_tree_must_be_identical(transformers) &&
#>    ^~~

^{Created on 2022-11-21 with reprex v2.0.2}

[lorenzwalthert]

This is intentional (not the {lintr} conflict). There is at most one level of indention triggered per line. The style guide does not specify that clearly IIRC, so there's some ambiguity. When you type the code in RStudio, I think it's compliant with what {styler} does. See also #549, where a {lintr} conflict was mentioned as well. I think it would be greate if {lintr} could be configured to accept current behaviour of {styler} here.

[AshesITR]

The reason we do it like this is to disambiguate, especially such cases:

fun(arg1 &&
      still_arg1,
    arg2)

fun(arg1,
    arg2,
    arg3)

If lines get longer, the && is easy to overlook. The extra indent makes it obvious that we're still in the first argument.

[IndrajeetPatil]

Thanks for adding the clarification, @AshesITR.

@lorenzwalthert TBH, I also personally prefer an extra indent in such contexts, and if the style guide is ambiguous in this regard, maybe we should try to be consistent with {lintr}? WDYT?

[lorenzwalthert]

I think we should file an issue in https://github.com/tidyverse/style and get it clarified. That's how we solved these conflicts in the past. As I said, I think it's easier if the IDE already has styling aligned and if we keep the rule that there is at most one additional level of indention per line. That's why we also have

test_that('x', {
  # one level 
)} # edit: }) I mean 

[IndrajeetPatil]

I think we should file an issue in https://github.com/tidyverse/style and get it clarified.

@AshesITR Would you be willing to file an issue? You are the indentation expert now 🙃

[AshesITR]

I think we should file an issue in https://github.com/tidyverse/style and get it clarified. That's how we solved these conflicts in the past. As I said, I think it's easier if the IDE already has styling aligned and if we keep the rule that there is at most one additional level of indention per line. That's why we also have

test_that('x', {
  # one level 
)}

That code, when fixed by swapping )} to }), is also correctly indented by lintr standards.
We special case that parenthesized blocks for the same range (here it's (...{ ... })) don't double-trigger.

[IndrajeetPatil]

Thanks for creating the issue, @AshesITR!