Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Binary Operators: Pipe: Change erlfmt formatting to place pipes at the beginning of lines #167

Closed
awalterschulze opened this issue Sep 21, 2020 · 19 comments · Fixed by #191
Labels
enhancement New feature or request

Comments

@awalterschulze
Copy link
Contributor

awalterschulze commented Sep 21, 2020

This was a question brought up multiple times in an experimental diff on OTP:

This is some we should reconsider and provide a formatting decision document on.

@awalterschulze awalterschulze added documentation Improvements or additions to documentation discussion Issue requires further discussion before solving labels Sep 21, 2020
@michalmuskala
Copy link
Member

The main reason is very mundane: we place all operators at the end of the line. One of our design principles is avoiding special cases.

@awalterschulze
Copy link
Contributor Author

Do you think it is worth reconsidering for this very popular case?

@michalmuskala
Copy link
Member

Potentially yes. One way to settle this would be to analyse some existing codebases to see which style is more prevalent.

@paulo-ferraz-oliveira
Copy link

I actually prefer operator, pipes, etc. "before" code (rationale: I don't need to read until the end of the line to understand something will change in the next line). I understand the need to have a solid opinion (as a default), but am all for having the choice to "tweak" the output to suite what I prefer. 😄

@awalterschulze
Copy link
Contributor Author

Ha ha totally agree @paulo-ferraz-oliveira :D

I am also a prefix pipe person and even a prefix comma person, but I agree with @michalmuskala lets try to settle the issue using data.
I think conforming is more valuable than my own opinion.

I think we should focus this analysis on pipes specifically.
Volunteers welcome :D

@paulo-ferraz-oliveira
Copy link

a prefix comma person

Haven't gotten used to this one yet, but @elbrujohalcon is surely trying to make me like it 😛 (i.e. elvis_core likes it, so if you PR, you have to "like" it also)...

more valuable than my own opinion

A bit of a side track here: why are configuration options frowned upon? We use them all the time, everywhere (optional callback, optional map fields, optional elements in proplists, ...). Is it because of maintenance costs?

@michalmuskala
Copy link
Member

One of the main goals of this formatter is to remove style discussions from team dynamics. If you have options that can be tweaked this goal is not achieved - you'll now be discussing which options to use.

@elbrujohalcon
Copy link
Contributor

elbrujohalcon commented Sep 22, 2020

Not trying to advocate for adding options to erlfmt, but for completeness…

You can also avoid style discussions in team dynamics by going the other way as we did with rebar3_format, where every developer can read the code as they like it (i.e. using their own favorite options… even their own favorite formatter) as long as default formatting is used when pushing code to a central repo. Which is what the SmallTalk formatter used to do many years ago.

@michalmuskala
Copy link
Member

While I agree this could be a valid approach, I personally think it creates more issues than it solves. There a lots of cases where you read code not locally (e.g. github or in PRs) and there are also cases where having consistent line numbers between developers is important (e.g. for easily sharing stack traces).

@elbrujohalcon
Copy link
Contributor

elbrujohalcon commented Sep 22, 2020

I see. I guess it is a matter of what happens more often or what is a larger PITA... Having to work with code that you drastically dislike / can't read or understand... Or having to apply the canonical formatter before sharing the code you are working on with others....

@paulo-ferraz-oliveira
Copy link

One of the main goals of this formatter is to remove style discussions from team dynamics. If you have options that can be tweaked this goal is not achieved - you'll now be discussing which options to use.

Fair, but teams usually also have a "decision" committee, which decides which are the best options (and sticks to those). I understand your point, though.

@awalterschulze
Copy link
Contributor Author

We hope we can find a compromise, which means everyone will just slightly dislike some formatting and not drastically dislike / can't read code and eliminate the read for a decision committee to discuss formatting when they can be focusing on more important things.

@awalterschulze awalterschulze changed the title Why does erlfmt place pipe at the end of the line Pipe: Why does erlfmt place pipe at the end of the line Sep 22, 2020
@elbrujohalcon
Copy link
Contributor

elbrujohalcon commented Sep 22, 2020

My personal belief (emphasis on "belief" since I have 0 proof/data about it) is that it will be fine for new projects where, from the get-go, everybody will just use erlfmt and get used to it.
But for people with strong preferences on code style that already have huge codebases consistently formatted that way (without the assistance of a formatter), like @inaka or @klarna (where everything is formatted in ROK-style, i.e. comma-first, as @paulo-ferraz-oliveira noted when working in elvis), the decision will likely be to just not use the new formatter. Because the alternative is to reformat everything and start writing code in a way that they don't like or even consider detrimental to their usual workflows.
Then again, and closing the off-topic derailment here, this is an opinionated formatter that will respect the code formatting decisions of its authors and that's perfect.
The goal of my comment was to reduce the broad generalization presented by @michalmuskala where having options implied unquestionably having to discuss about them. That's just not true in all scenarios. That's all.

…and now… back to the show 🤪

@awalterschulze awalterschulze changed the title Pipe: Why does erlfmt place pipe at the end of the line Binary Operators: Pipe: Why does erlfmt place pipe at the end of the line Sep 22, 2020
@paulo-ferraz-oliveira
Copy link

paulo-ferraz-oliveira commented Sep 22, 2020

Edit: changing , to | in the example.

Another good point about using | (or ,, for that matter) to the left is the fact that in a diff only your changes are important.

You have

[
    a |
    b
]

now you add a c:

[
    a |
    b |   % changed here
    c     % changed here
]

(two lines changed).

On the other hand, you have:

[
    a
  | b
]

now you add a c:

[
    a
  | b
  | c    % changed here
]

(one line changed)

[I'm sorry if this was already discussed before]

@awalterschulze
Copy link
Contributor Author

awalterschulze commented Sep 23, 2020

@paulo-ferraz-oliveira is it possible to move you comments about , to the lists issue #150
Sorry I know these are hard to find, but it would be great to help us keep things semi organised.

Even if it is discussed before, it is still useful to see others agree with some point or run into the same issue.
So thank you for bringing this up :)

@paulo-ferraz-oliveira
Copy link

Another data point for |: #175 (comment).

@paulo-ferraz-oliveira
Copy link

@paulo-ferraz-oliveira is it possible to move you comments about , to the lists issue #150
Sorry I know these are hard to find, but it would be great to help us keep things semi organised.

Even if it is discussed before, it is still useful to see others agree with some point or run into the same issue.
So thank you for bringing this up :)

I'll change from , to | since the "problem" is the same.

@awalterschulze
Copy link
Contributor Author

We now have a proposal to move pipes to be a prefix with some data to back it up
#186

@paulo-ferraz-oliveira
Copy link

Hurrah for data. ❤️

@awalterschulze awalterschulze added enhancement New feature or request and removed discussion Issue requires further discussion before solving documentation Improvements or additions to documentation labels Sep 24, 2020
@awalterschulze awalterschulze changed the title Binary Operators: Pipe: Why does erlfmt place pipe at the end of the line Binary Operators: Pipe: Change erlfmt formatting to place pipes at the beginning of lines Sep 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants