if/else formatting can lead to unexpected breaks

[aronatkins]

I just reformatted some written-long-ago code frequently included if/else statements without braces. Those statements are reformatted into a shape that leads to a less readable result (to my eye).

These examples show the spirit of what happened:

Input:

foo <- function() {
  if (first_long_variable && second_long_variable)
    cat("ahoy there mates!")
  else
    cat("arr! i be shivering.")
}

Formatted:

foo <- function() {
  if (first_long_variable && second_long_variable) cat("ahoy there mates!") else
    cat("arr! i be shivering.")
}

The input form made it obvious that this was an if/else statement. The reformatted form makes it less clear.

Things are also complicated with if/else-if chains.

Input:

foo <- function() {
  if (first)
    cat("first")
  else if (second)
    cat("second")
  else if (third)
    cat("third")
  else
    cat("last")
}

Formatted:

foo <- function() {
  if (first) cat("first") else if (second) cat("second") else if (third)
    cat("third") else cat("last")
}

The formatted form is much more challenging to skim to see the different expressions.

In each of these examples, I introduced braces in order to avoid letting air collapse them.

Using air 0.4.1

[DavisVaughan]

Yea, this isn't great. I'm actually actively working on a feature called autobracing that will address this.

It implements the adjustments to the tidyverse style guide as noted in
tidyverse/style#240

Notably, if you have a multiline if statement it must have braces

Here's what my dev build gives me for your examples:

foo <- function() {
  if (first_long_variable && second_long_variable) {
    cat("ahoy there mates!")
  } else {
    cat("arr! i be shivering.")
  }
}

foo <- function() {
  if (first) {
    cat("first")
  } else if (second) {
    cat("second")
  } else if (third) {
    cat("third")
  } else {
    cat("last")
  }
}

[aronatkins]

I favor braces.

The one type of example I encountered where the lack of braces was (subjectively) better is in the rare instance where everything fit onto one line.

query <- paste0("force=", if (force) "1" else "0")

list(
  value = if (is.na(value)) NULL else value
)

This is all from old code, and if writing from scratch today, we would probably use a slightly different approach (more temporary variables, possibly).

[aronatkins]

The examples in this issue were extracted from rstudio/rsconnect#1154 (the first commit contains the automatic reformatting).

[DavisVaughan]

We will allow both of those "one liner" forms!