Skip to content

Decrease syntax noise by changing if-then-else 2-level layout to 1-level#457

Closed
cblp wants to merge 1 commit intotweag:masterfrom
cblp:if-then-else
Closed

Decrease syntax noise by changing if-then-else 2-level layout to 1-level#457
cblp wants to merge 1 commit intotweag:masterfrom
cblp:if-then-else

Conversation

@cblp
Copy link
Copy Markdown
Contributor

@cblp cblp commented Nov 8, 2019

Motivation

Before:

if condition
  then do
    2nd level of indentation
  else do
    2nd level of indentation
-- 5 lines, 2 of them are purely syntactical, with no semantics

After:

if condition then do
  1st level of indentation
else do
  1st level of indentation
-- 4 lines
  1. Exactly 1 indentation for 1 code block => Lesser code depth. Currently, the number of indentation levels is twice greater than the number of semantic layers.
  2. Closer to if-statement layout in other languages. => More friendly to newcomers with experience in other languages.

Drawbacks (not problems in my opinion)

  1. In Haskell98 mode, usage in do-blocks needs NonDecreasingIndentation extension.
    Usage in pure expressions works out of the box.
    In Haskell2010 mode, it works out of the box.

  2. Longer lines in some cases due to then ending.

  3. More lines in short cases:

    -- before: 3 lines
    if condition
      then x
      else y

    vs.

    -- after: 4 lines
    if condition then
      x
    else
      y
  4. Reformatting code formatted with ormolu 0.0.1.0. This is not a problem since 0.0.1.0 doesn't commit being stable.

    Compensated by the decrease in longer cases, see example in Motivation section.

Motivation
----------

Before:

    if condition
      then do
        2nd level of indentation
      else do
        2nd level of indentation
    -- 5 lines

After:

    if condition then do
      1st level of indentation
    else do
      1st level of indentation
    -- 4 lines

1. Less levels of indentation.
2. Closer to if-statement layout in other languages.

Drawbacks (not problems in my opinions)
---------------------------------------

1.  In Haskell98 mode, usage in do-blocks needs NonDecreasingIndentation extension.
    Usage in pure expressions works out of the box.
    In Haskell2010 mode, it works out of the box.

2.  Longer lines in some cases due to ` then` ending.

3.  More lines in short cases:

	-- 3 lines before
	if condition
	  then x
	  else y

    vs.

	-- 4 lines after
	if condition then
	  x
	else
	  y

    Compensated by the decrease in longer cases, see example in Motivation section.
@cblp
Copy link
Copy Markdown
Contributor Author

cblp commented Nov 8, 2019

Hm, CI found a bug in case

  -- comment
  else do

but this bug is in master, is not introduced by my patch.

@mrkkrp
Copy link
Copy Markdown
Member

mrkkrp commented Nov 8, 2019

Another subjective issue. Personally, I'd be very uncomfortable with this. I don't think we need to go out of our way to save a level of indentation. Also, I have never seen this formatting in Haskell code. Maybe in JavaScript or C++.

With current layout you have a condition, then you have clearly two branches formatted quite symmetrically which makes sense to me.

In general now that we released the initial version we're going to need very good reasons to change any part of formatting. Those should better be technical and non-subjective.

@mrkkrp
Copy link
Copy Markdown
Member

mrkkrp commented Nov 8, 2019

@cblp Please make sure to first open an issue for changes like this so we could discuss before you spend time implementing something.

@cblp
Copy link
Copy Markdown
Contributor Author

cblp commented Nov 8, 2019

The issue is #299.

@cblp
Copy link
Copy Markdown
Contributor Author

cblp commented Nov 8, 2019

I don't think we need to go out of our way...

What is your way?

@cblp
Copy link
Copy Markdown
Contributor Author

cblp commented Nov 8, 2019

I have never seen this formatting in Haskell code

I have no data, but I think this style is so common, that an extension was created for it, and is has been made default in Haskell2010.

@cblp
Copy link
Copy Markdown
Contributor Author

cblp commented Nov 8, 2019

The technical reason is decreasing syntactical noise.

@cblp cblp changed the title Change if-then-else 2-level layout to 1-level Decrease syntax noise by changing if-then-else 2-level layout to 1-level Nov 8, 2019
@mrkkrp
Copy link
Copy Markdown
Member

mrkkrp commented Nov 8, 2019

The issue is #299.

That issue resulted in bumping indentation of then and else. I don't think we agreed on going with the version B there and there has been no new comments for a while.

What is your way?

"Go out of your way" is an idiom.

The technical reason is decreasing syntactical noise.

Syntactical noise is hard to define precisely, so it's hardly a technical reason. I perceive your layout as more noisy, and this is just my opinion. Given two conflicting opinions on a subjective issue such as this I see no reason to change the status quo.

I do not want to discourage you from contributing, but if we start accepting changes that tweak existing formatting just because of taste preferences, the style will never get fixed as there are so many opinions as there are people. It's not a very productive use of our time, too.

@cblp
Copy link
Copy Markdown
Contributor Author

cblp commented Nov 8, 2019

just because of taste preferences

No, I really hope this can help people.

When I look to

if...
  then
    a
  else
    b

I see 5 syntax nodes: a fork, two branch selectors, two branch bodies.

And in

if...
  a
else
  b

I see only 3 syntax nodes: a fork and two branch bodies.

Well, it comes I'm the only one.

@mrkkrp
Copy link
Copy Markdown
Member

mrkkrp commented Nov 9, 2019

I see 4 syntax nodes: a fork, one branch selector, and two branch nodes.

@cblp
Copy link
Copy Markdown
Contributor Author

cblp commented Nov 10, 2019

Well, it seems to be purely subjective.

@cblp
Copy link
Copy Markdown
Contributor Author

cblp commented Nov 10, 2019

The only metric applicable is the indentation depth but is it good or bad is unclear.

@mrkkrp
Copy link
Copy Markdown
Member

mrkkrp commented Nov 18, 2019

As discussed, it is not obvious that this change is in fact an improvement. The current Ormolu policy regarding formatting is that we're going to keep the style we have right now unless there is a good objective reason to change it, so that the style doesn't change a lot between releases for people who already started using the tool.

@mrkkrp mrkkrp closed this Nov 18, 2019
wldhx pushed a commit to wldhx/fourmolu that referenced this pull request Apr 16, 2021
wldhx added a commit to wldhx/fourmolu that referenced this pull request Apr 16, 2021
wldhx added a commit to wldhx/fourmolu that referenced this pull request Apr 16, 2021
wldhx added a commit to wldhx/fourmolu that referenced this pull request Sep 9, 2021
wldhx added a commit to wldhx/fourmolu that referenced this pull request Mar 21, 2022
wldhx added a commit to wldhx/fourmolu that referenced this pull request Mar 21, 2022
wldhx added a commit to wldhx/fourmolu that referenced this pull request Mar 25, 2022
wldhx added a commit to wldhx/fourmolu that referenced this pull request Mar 31, 2022
cblp pushed a commit to cblp/fourmolu that referenced this pull request Jul 12, 2025
cblp pushed a commit to cblp/fourmolu that referenced this pull request Jul 12, 2025
cblp pushed a commit to cblp/fourmolu that referenced this pull request Jul 20, 2025
brandonchinn178 pushed a commit to fourmolu/fourmolu that referenced this pull request Aug 10, 2025
* Add option for if-then-else 1-level layout

Port of tweag/ormolu#457
Original issue tweag/ormolu#299

Closes #80

* Add tests for indent=2,4

* fixup! Add tests for indent=2,4

* Use `locatedToken` instead of `txt`

* Solve tall conditions

* fixup! Solve tall conditions

* fixup! fixup! Solve tall conditions

* fixup! fixup! fixup! Solve tall conditions

* fixup! fixup! fixup! fixup! Solve tall conditions

* Style

* Rename one-level-ifs -> shifted-ifs

* Reformat closer to upstream Ormolu, by @brandonchinn178

* Add if-else-if chains to tests and docs

* Change test file name encoding

---------

Co-authored-by: wldhx <wldhx@wldhx.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants