r-lib / styler

Non-invasive pretty printing of R code
https://styler.r-lib.org
Other
713 stars 70 forks source link

Indendation of low-precedence infix operators #1150

Open AshesITR opened 11 months ago

AshesITR commented 11 months ago

Related to: https://github.com/r-lib/lintr/issues/1718

y ~
  x1 +
    x2

Has parse tree ~(y, +(x1, x2)) so the ~ ends in line 3 and should thus cause an indentation for lines 2 and 3. + ends in line 3 and should cause another level of indentation on line 3.

styler::style_text("
y ~
  x1 +
    x2
")
#> y ~
#>   x1 +
#>   x2

Created on 2023-09-15 with reprex v2.0.2

MichaelChirico commented 11 months ago

The style guide doesn't really say much about indentation. Perhaps we could raise that; meanwhile, editing code that doesn't violate anything in the style guide generally goes against styler's intention, right?

AshesITR commented 11 months ago

Note that a + b + c is (a + b) + c whereas a ~ b + c is a ~ (b + c). Therefor, indentation shouldn't be the same for the two.

styler::style_text("
y +
  x1 +
  x2
")
#> y +
#>   x1 +
#>   x2

Created on 2023-09-15 with reprex v2.0.2

MichaelChirico commented 11 months ago

I think "should"/"shouldn't" can only be ruled by the style guide (for our defaults, at least).

If the style guide is silent and we're trying to be consistent, we should come to an agreement after discussion.

Reasoning in terms of "mixed precedence" makes sense to me. There's a related styler issue I've raised: https://github.com/tidyverse/style/issues/160. ?Syntax lists precedence groups, we could use that as an objective measure.

HaHeho commented 7 months ago

I can add the following similar example that yields malformed code (the input is the correctly formatted code that I would expect).

> styler::style_text("
+ stats::model.matrix(
+   ~ a
+   + b
+   + c,
+   dd
+ )
+ ")
stats::model.matrix(
  ~ a
  + b
    + c,
  dd
)

Note that considerations around the operator precedence do not apply here since they don't actually function as mathematical operators in this context. I believe something similar may arise around the ggplot2::facet_grid function.

EDIT: For reference, the built-in formatter of R Studio yields

stats::model.matrix(~ a
                    + b
                    + c,
                    dd)
lorenzwalthert commented 6 months ago

Note that considerations around the operator precedence do not apply here since they don't actually function as mathematical operators in this context

I don’t think we can differentiate these cases here in styler. Also, the issue caused some headache already in the past and I can’t recall the reasons for tie current implementation and if your representative was intentional or not (but it could probably be for d searching the issues) and given that the style guide is silent on it and the syntax is generally not used very often, this has low priority for me.