Enter-tainer / typstyle

Beautiful and reliable typst code formatter
https://enter-tainer.github.io/typstyle
Apache License 2.0
298 stars 10 forks source link

Feature request: inline functions with `if` statements #76

Open ntjess opened 4 months ago

ntjess commented 4 months ago

Consider this case:

#table(
  fill: (x, y) => if y == 0 { white.darken(15%) } else { none },
  align: (x, y) => if y == 0 { center } else { horizon },
  [Hi],
  [there],
)

The function definition is short (< line length) and clearly indicates parameter usage. typstyle reformats to:

#table(
  fill: (x, y) => if y == 0 {
    white.darken(15%)
  } else {
    none
  },
  align: (x, y) => if y == 0 {
    center
  } else {
    horizon
  },
  [Hi],
  [there],
)

which is arguably less readable.

My request is for inline ternary functions shorter than the user-specified line length to remain one-line.

Enter-tainer commented 4 months ago

it is related to how we format code blocks. currently code blocks are always take at least 3 lines. i think we can make a special case for this.

Enter-tainer commented 4 months ago

I think it is a little bit hard to determine when to fold these into a single line. I did some initial test but the result doesn't look good

image image

ntjess commented 4 months ago

Fair point; perhaps only when an "else" clause is also specified?

Enter-tainer commented 4 months ago

Fair point; perhaps only when an "else" clause is also specified?

I propose a way to determine whether to inline if or not:

  1. Never inline if statement. That is, ifs that directly appears in code blocks
  2. For other ifs, if it has an else clause and all its subclause only has a single item(and the item is not commet!), and if all these thing can fit in the column width, we put it in a single line.
ntjess commented 4 months ago

That makes sense to me!

For other ifs

Are there additional ifs besides assignment and control statement?

Enter-tainer commented 4 months ago

I mean we may only inline ifs that appears in expression, function args, rhs of assignment ... But we shouldn't inline ifs that is directly in a code block.

{
  // DO NOT
  if ... {
  ...
  } else {
  ..
  }
  let a = if ... { } else { } // DO
  let a = f(if ... { } else { }, ...) // DO
  let a = (1, 2, if ... { } else { }) // DO
}
Enter-tainer commented 4 months ago

I just realize that the body of if can be content blocks as well, like

#if 1 > 2 [ 111 ] else [ 222 ]

So maybe we should have a special code path for "simple if"s. If we have a if,

  1. not a statement(direct parent is not code block)
  2. has 2 sub clause
  3. both of sub clause is code block
  4. each sub clause has only one item inside, and the item is not line comment

We try to inline it.

ntjess commented 4 months ago

Content blocks should be acceptable too, right? IMO As long as they are short enough, the cognitive complexity is still lower when inlined.

All the other conditions make sense to me.

Myriad-Dreamin commented 4 months ago
  1. Never inline if statement. That is, ifs that directly appears in code blocks
  2. For other ifs, if it has an else clause and all its subclause only has a single item(and the item is not commet!), and if all these thing can fit in the column width, we put it in a single line.

From my point of view, the first point is good but the second point is not quite good. I don't think it good and robust to check so many conditions: if it has an else clause and all its subclause only has a single item(and the item is not commet!). Why don't we change the perspective for this issue to simplify the rule? inline if expression only if

  1. any expression is on the value position of a named item, such as:
    #table( // named item can be as a function argument
    fill: (x, y) => if y == 0 { white.darken(15%) } else { none },
    )
    #let paint = ( // named item can be as an item of some dictionary literal
    stroke: if y == 0 { white.darken(15%) } else { none },
    )
  2. the entire expression's width can fit in the column width.

I think it is a generally great enough rule to make beautiful named items, and we can find other good rules when someone make another issue.

ntjess commented 4 months ago

I think the reduced complexity makes sense, but I would advocate for let declarations to also be considered. To me, readability decreases with spacing for the following example:

#let x = if cond {
  5
} else {
  6
}

However, let introduces all the edge cases from above. So I can understand avoiding it in favor or straightforward rules.

Enter-tainer commented 3 months ago

https://doc.rust-lang.org/nightly/style-guide/expressions.html#single-line-if-else