Could code formatter leave spaces in multiline keyword lists? #6647

pragdave · 2017-10-09T00:44:38Z

I like to make multiline keyword lists easier to read by aligning the values into a column:

  def project do
    in_production = Mix.env == :prod
     [
      app:     "fred",
      version: "1.2",
      elixir:  ">= 1.4.0",
      deps:    deps()
     ]
end

The code formatter removes these extra spaces:

  def project do
    in_production = Mix.env() == :prod

    [
      app: "fred",
      version: "1.2",
      elixir: ">= 1.4.0",
      deps: deps()
    ]
  end

I would be really, really nice if it kept them. (And even nicer still if it added them to do the alignment for you... :)

fishcakez · 2017-10-09T04:17:24Z

Hi @pragdave. I believe the Elixir code base used to be aligned more then half the time, and then moved away from aligning about a year ago. The formatter is following the standard style used by Elixir.

Note that aligning like this can cause many lines to be altered when one entry becomes longer. This hurt the review process, history and maintenance. Of course the later point is much less significant now we have a tool to handle it. Unfortunately allowing alignment when all are aligned would hit these issues, even with maintenance as lines need to be done by hand and its possible mistake would unalign. Hopefully developer environments can provide enough features, such as syntax highlighting to keep the unaligned way clear.

With the new formatter there is going to be compromises for everyone (especially amongst those who worked on it so far), and where possible follows Elixir's existing style. I hope people will stay objective about this and see that while not everything is exactly how they personally write code the consistency will benefit everyone.

pragdave · 2017-10-09T05:58:42Z

Well, as José can attest, I've never been a fan of the current Elixir style, and have many times proposed different ways of doing it. The code created by the standard generators, which was then in the standard "Elixir style" was eventually changed into something more reasonable as a result of my suggestions. Elixir is a lovely language, but the way it is often written makes it look ugly, which is a shame. Everything is packed together, with little whitespace, no alignment of related things, and long line after long line. The fact that the new formatter preserves most line breaks is great. In fact, I have no problems with 99%of what it does. Two of my three issues were simple bugs I found while trying it on a typical mix.exs file. The plea to preserve whitespace in multi line lists is different. It's a request to start a discussion. I'd like to have a formatter that reveals the structure and beauty of the code, rather than one that tries to save on whitespace. I agree it is a personal thing, but ultimately that's not bad if it is based on experience and not just opinion. I also knew that this suggestion would generate exactly this kind of vaguely ad hominem response, although I was surprised that it came from you, James. Like all of you, I have a lot riding on the success of Elixir. But I also am coming at it from the perspective of making it an approachable and long term language. When I make suggestions here, it's because my experience teaching Elixir tells me that people are confused by things, or that a particular feature could be easier to understand if written this way. You talk of compromise. My suggestion is a total compromise. Preserve whitespace if it is present. Sure I'd like to have it added automatically too, but that was just a pie in the sky ideal. The original suggestion is my way of compromising. If you don't write aligned kv lists, then the formatter agrees with you. If you do write them, that's peachy too. I'm not forcing my layout ideas on anyone with this suggestion. I'm suggesting a heuristic that would make the formatter just a little more humane. Dave

fishcakez · 2017-10-09T06:10:38Z

@pragdave I didn't mean to be ad hominem and was trying to be fair. I was very keen to keep discussion on track and explain why it is how it is. I think code style is more likely than most to trigger a discussion that can get derailed. Sorry I let that get in the way of fluid discussion.

josevalim · 2017-10-09T07:37:41Z

We (the core team) know from experience that discussion on styles can be tough. :) And, similar to our previous discussions, I don't believe anyone here intended to get personal or derail the discussion. ❤️

As I mentioned in the other issue, our goal with the formatter is to mandate a style, reducing the amount of discussion on each project or change and also ensuring everyone in the community follows the same guidelines.

We will likely end-up adding more flexibility but it is still too early to tell which ones. Everyone is going to have preferences on which formatting rules they would like to keep and if we adopt all of them then the formatter loses its goal of unifying the styles across the community.

@pragdave himself mentioned that we had already changed the style once (before we even started working on the formatter) and the style he proposed back then is the one adopted by the formatter now. This shows we do listen and we will continue improving based on the general feedback.

The formatter doesn’t respect whitespace in keyword lists or maps, and that makes the hardware byte lookup tables hard to read. Those parts of the formatter output were reverted, and I’m hoping that Dave’s GitHub issue elixir-lang/elixir#6647 gets future consideration.

RichMorin · 2018-01-18T17:07:27Z

I strongly support @pragdave on this issue and would extend it to any number of situations where the programmer uses spacing to clarify parallel construction, emulate a tabular format, etc. There are lots of examples in "Programming Elixir" that show effective use of inline spacing to clarify intent. Please allow Elixir programmers more flexibility in "hacking beautiful code".

josevalim · 2018-01-19T08:38:43Z

Thanks everyone for the feedback.

I will reopen this issue for discussion. The only way we can support this feature is by having some sort of opt-in approach. Otherwise, any project with multiple contributors will end-up with an inconsistent style, as some developers will align some entries and others won't. Having this would completely defeat the point of the formatter.

I have some ideas in mind but I would like to learn more about how inline whitespace is used before proposing them. Therefore, to move this forward, I would like to know:

in which scenarios do you use inline whitespace for alignment? The examples I have seen so far aligned the values in maps and keyword lists. Are there any others? Please provide code samples.
for the code samples you provided above , do you always use inline whitespace or it depends on the case? For example, do you always align the values in a multi-line map or a multi-line keyword list?

From experience, I know those discussions can be very opinionated and I would like to avoid that. Otherwise, if this turns out to be a matter of personal preferences, then I pick mine. ;) So I would appreciate if everyone could focus on answering the two questions above so we can better understand of how this feature would be used across the community.

OvermindDL1 · 2018-01-19T17:29:10Z

for the code samples you provided above , do you always use inline whitespace or it depends on the case? For example, do you always align the values in a multi-line map or a multi-line keyword list?

I try to whenever all the values are single-line. If any are multi-line then I tend to not to, so for example:

# All are single-line values, so aligning tends to make them look better
blah = %{
  boop:     42,
  vwallaba: 16,
  a:        :b, # Always trailing comma!
}

# Multi-line values, so I don't align these
commands = %{
  "list" =>
    fn(...) ->
      ...
    end,

  "ping" =>
    "pong",

  "help" =>
    fn(...) ->
      ...
    end, # Always trailing comma!

  ...
}

pragdave · 2018-01-19T20:07:07Z

redacted *

  stats = %{ 
    player_id: player_id, 
    word:      tally.letters |> Enum.join(), 
    won?:      won?,
    incorrect: tally.incorrect
  }

Fairly straightforward. But what happens if I add comments:

responses = %{
  won:          [ "success", "You Won!" ],                 # no change to rest of state
  lost:         [ "danger",  "You Lost!" ],                #            -"-
  good_guess:   [ "success", "Good guess!" ],              # one less move
  bad_guess:    [ "warning", "Bad guess!" ],               #      -"-
  already_used: [ "info",    "You already guessed that" ], # no change
  initializing: [ "info",    "Let's Play!" ]               # before game starts
}

This is effectively a table, where the first columns are code and the last column is commentary. Moving the comments before the entries makes it harder to read. Removing whitespace within the lines makes it way harder to read:

responses = %{
  won: [ "success", "You Won!" ], # no change to rest of state
  lost: [ "danger",  "You Lost!" ], #            -"-
  good_guess: [ "success", "Good guess!" ], # one less move
  bad_guess: [ "warning", "Bad guess!" ],  #      -"-
  already_used: [ "info",    "You already guessed that" ], # no change
  initializing: [ "info",    "Let's Play!" ] # before game starts
}

Sometimes, the whitespace doesn't follow rules. For example, I prefer:

defp deps do
  [
    { :arcade,  github: "pragdave/mae-arcade" },
    { :hangman, github: "pragdave/mae-hangman" },
    { :phoenix_pubsub,      "~> 1.0", path: "~/Play/phoenix_pubsub", override: true},
    { :phoenix,             "~> 1.3.0-rc" },
    { :phoenix_html,        "~> 2.6" },
    { :phoenix_live_reload, "~> 1.0", only: :dev },
    # ...

to

defp deps do
  [
    { :arcade,              github: "pragdave/mae-arcade" },
    { :hangman,             github: "pragdave/mae-hangman" },
    { :phoenix_pubsub,      "~> 1.0", path: "~/Play/phoenix_pubsub", override: true},
    { :phoenix,             "~> 1.3.0-rc" },
    { :phoenix_html,        "~> 2.6" },
    { :phoenix_live_reload, "~> 1.0", only: :dev },

and sometimes I'm just totally irrational, preferring

new_game = %{ game |
              game_state: :bad_guess,
              turns:       turns_left - 1
            }

to

new_game = %{ game |
              game_state: :bad_guess,
              turns:      turns_left - 1
           }

josevalim · 2018-01-19T20:28:17Z

Could we please keep the discussion focused? This is not an issue about the usefulness of the formatter. I am glad to discuss it but not here. If we could focus on the two questions so we can move this issue forward, it will be very appreciated. PS: I am aware we can’t support white space generally. That’s exactly why I asked for code samples. Let’s see some use cases and discuss that.

pragdave · 2018-01-19T20:36:11Z

I moved my stuff about removing the formatter out to #7233.

I put it here only because I thought it might short-circuit the need to discuss the lower level issues raised.

josevalim · 2018-01-19T20:40:07Z

Thanks Dave, very appreciated!

fertapric · 2018-01-26T11:02:18Z

If it helps, here is a real life example I spotted today: https://github.com/michalmuskala/jason/blob/master/bench/encode.exs#L2-L9

PragTob · 2018-01-31T17:50:13Z

👋 Thanks for having this discussion!

Adding to the voices here, for me this is the one thing that I really would like to change/allow in the formatter (so far). Aligning keys and values imo improves the readability a lot.

Generally speaking I do it always when I have a keyword list, map or struct with values that fit on a single line (and if they don't I often make them fit :D)

Example from benchee:

  # I'm ok/happy with this being changed so that `parallel` starts on the next line and the
  # closing brackets match up
  @config %Configuration{parallel: 1,
                         time:     40_000,
                         warmup:   20_000,
                         inputs:   nil,
                         print:    %{fast_warning: false, configuration: true}}
  @system %{
    elixir:           "1.4.0",
    erlang:           "19.1",
    num_cores:        "4",
    os:               "Super Duper",
    available_memory: "8 Trillion",
    cpu_speed:        "light speed"
  }

When using => syntax I align on =>:

Benchee.run(%{
  "Integer addition"          => fn -> 1 + 1 end,
  "String concatention"       => fn -> "1" <> "1" end,
  "adding a head to an array" => fn -> [1 | [1]] end,
  "++ array concat"           => fn -> [1] ++ [1] end,
  "noop"                      => fn -> 0 end,
  "Enum.map(10)"              => fn -> Enum.map(range, fn(i) -> i end) end
}, time: 3)

Oh one other thing, I also usually do it with = signs but I don't feel as strongly about:

    total_time         = Enum.sum(run_times)
    iterations         = Enum.count(run_times)
    average            = total_time / iterations
    ips                = iterations_per_second(average)
    deviation          = standard_deviation(run_times, average, iterations)
    standard_dev_ratio = deviation / average
    standard_dev_ips   = ips * standard_dev_ratio
    percentiles        = Percentile.percentiles(run_times, [50, 99])
    median             = Map.fetch!(percentiles, 50)
    mode               = Mode.mode(run_times)
    minimum            = Enum.min run_times
    maximum            = Enum.max run_times

Thanks a lot everyone! 💚 🎉 💞

josevalim · 2018-02-16T09:13:09Z

Thanks everyone for the examples. Now that we have examples, I would like us to move this discussion towards possible solutions. Once again, let's continue focused on the issue at hand, and avoid general comments about the formatter or other formatter features. :)

My biggest concern here is how to signal intent. Many have said they align under certain conditions, some of those conditions are programatic, others are not. This rules out a project-wide configuration.

Auto-detection is also tricky because it opens up space for accidental inconsistency: you have a developer that aligns and the other does not and there is nothing signaling a difference in intent. The worst part is what would happen when you add a new expression that changes the whole alignment. Imagine you had this:

  @system %{
    elixir: "1.4.0",
    erlang: "19.1",
    os:     "Super Duper"
  }

and then you add this:

  @system %{
    elixir: "1.4.0",
    erlang: "19.1",
    os:     "Super Duper",
    available_memory: "8 Trillion",
  }

If we are auto-detecting, it means the developer now needs to align all keys, instead of having the formatter do the job.

Speaking about maps and key-aligning, others have also requested auto-sorting of keys. How would you tell a future contributor that keys must be sorted alphabetically? In the cases I have done so, it was usually via a comment. If there is no comment, then an inconsistency would probably be found only during code reviews.

Given all of those requirements, it appears to me we have no other option besides being explicit. One mechanism to do so that came to mind is by using comments as annotations. For example:

  @system %{ # aligned, sorted
    elixir: "1.4.0",
    erlang: "19.1",
    os:     "Super Duper",
    available_memory: "8 Trillion",
  }

Now when the formatter runs, it will always align and sort the keys. If you add a new entry, regardless of the initial shape, the formatter end result is the same. Initially we could start by supporting this annotation in maps and keyword lists.

Thoughts? Other solutions?

fertapric · 2018-02-16T09:28:13Z

To bring some edge cases, what should the expected output be with nested keywords/maps?

Example:

@system %{ # aligned, sorted
  elixir: "1.4.0",
  erlang: "19.1",
  os:     "Super Duper",
  memory: %{
    available: "8 Trillion",
    used: "4 Trillion"
  }
}

josevalim · 2018-02-16T09:31:49Z

Excellent question, let's wait for replies. To add one other example:

@system %{ # aligned, sorted
  elixir: "1.4.0",
  erlang: "19.1",
  operating_system: "Super Duper",
  memory: %{
    available: "8 Trillion",
    used: "4 Trillion"
  }
}

I think the one that makes the most sense though is nesting everything:

@system %{ # aligned, sorted
  elixir:           "1.4.0",
  erlang:           "19.1",
  operating_system: "Super Duper",
  memory:           %{
                      available: "8 Trillion",
                      used: "4 Trillion"
                    }
}

devonestes · 2018-02-16T10:56:11Z

Here is a very rough algorithm for one possible solution:

if (all values of a multi-line keyword list or map begin at the same column) do
assume the human wants it this way and don't make any changes to this keyword list or map
else
format it according to the normal rules
end

Basically, if you want to completely exempt this from formatting by the formatter, you're welcome to do it, and the formatter will get out of your way. Yes, you still need to the the fancy formatting yourself, but that's not a change from what folks are currently doing. I think it's important to note that in the original issue description Dave and the other folks who gave examples aren't asking for all keyword lists or maps to be aligned, but for the formatter to not override their preference for alignment when they've already done that themselves.

I also think it's reasonable that if you want to exempt a particular map or keyword list from formatting, that means you don't get auto-sorting or anything like that. You'd have to sort the keys yourself.

I'm personally not worried about accidental inconsistency. For many folks that seems like a reasonable price to pay to have this special formatting available in some places. The folks in favor of this proposal have all said that they don't do this everywhere, so inconsistency is in line with the current state of the examples given. If they want consistency, they can use the formatter as it normally works - this wouldn't take that option away from them.

josevalim · 2018-02-16T11:31:13Z

I'm personally not worried about accidental inconsistency.

We need to look at it from both sides. Users who want to align from time to time don't mind the inconsistency but the users that would never align now need to handle potential inconsistencies.

I know this can become a "we prefer" vs "they prefer" discussion, but given the goal of the formatter is consistency, it would be hard to justify such implicit behaviour.

So I could only see such feature working if it is explicitly enabled by a flag that says the alignment should be kept when found. So your condition would become:

if (all values of a multi-line keyword list or map begin at the same column) &&
     (formatter was configured to keep alignment) do

AndrewDryga · 2018-02-16T11:46:22Z

We have a project that used aligning in past and I'm -1 for implementing it in the formater. The reason was already told by James, there is a lot of changes when one line becomes longer or shorter. Also, it brings a lot of non-programmable negotiations "how do make it right", especially when there are multiple variables defined close to each other.

josevalim · 2018-02-16T11:51:52Z

@AndrewDryga if we add a solution, it will be a 100% opt-in. So you wouldn't have to worry unless you explicitly allowed it.

AndrewDryga · 2018-02-16T11:53:14Z

@josevalim wouldn't that lead to more people asking for opt-in features and turning formatter into a configurable thing, which, I believe, we tried to avoid?

josevalim · 2018-02-16T13:03:25Z

@AndrewDryga yes, but that does not exclude us from discussing solutions. :) Nothing means those solutions will be accepted. So let's focus on possible alternatives, we can veto them later.

fertapric · 2018-02-17T11:11:27Z

Now, if they by mistake add 2 spaces then we will get undesired code formatting.

The rule should not apply with one single mistake. It requires two (or more) rows with two (or more) spaces to tell the formatter that it should align the values vertically. Here is an example:

If this is the current code:

@system %{
  elixir: "1.4.0",
  os: "Super Duper",
  available_memory: "8 Trillion"
}

And a member of the team includes a new value with two (or more) spaces:

@system %{
  elixir: "1.4.0",
  os: "Super Duper",
  available_memory: "8 Trillion"
  erlang:  "19.1" # 2 spaces
}

The formatter will use the normal rules:

@system %{
  elixir: "1.4.0",
  os: "Super Duper",
  available_memory: "8 Trillion"
  erlang: "19.1"
}

josevalim · 2018-02-17T11:17:56Z

@josevalim just do that because the thread comments are just opinions at the end of the day.

The choice of aligning or not is based on opinion. The mechanism that will support alignment can be discussed objectively. I am not interested on talking about the former, it is already clear that some prefer it, others do not. The solution can be discussed objectively though and I would like to ask you to do so.

Now, if they by mistake add 2 spaces then we will get undesired code formatting.

Yes, that's the problem with any of the implicit behaviour via configuration files. That's why I am personally more inclined to annotations in the code. Yes, I agree it is uglier, but the goal of the formatter is consistency and I think going against the consistency should be as explicit as possible.

@fertapric your solution would also require the formatted result to also include two spaces. Otherwise once the formatted code is formatted again, the formatting would be lost because your formatted result did not include two spaces.

yordis · 2018-02-17T11:23:09Z

@fertapric If the developer just duplicate the line or copy past the lane, then it will fall back to the original issue.

I would prefer to annotate the code and be explicit about it.

P.S

The solution can be discussed objectively

@josevalim how could you talk about a subjective topic by nature, objectively? We are not talking about maths were everything is objective unless you prove it otherwise, we are talking about code style and by nature it is a subjective topic (based on opinions)

I would like to ask you to do so.

So where my subjective or objective comments starts and ends, what is objective for you? Give me a defined rule and I will follow it, but even your comment is subjective to whatever meaning have to you.

fertapric · 2018-02-17T11:23:38Z

@fertapric your solution would also require the formatted result to also include two spaces. Otherwise once the formatted code is formatted again, the formatting would be lost because your formatted result did not include two spaces.

Yeah. I think that's a good compromise. The given use cases usually have more than two spaces in many of their rows, so I guess that should not be a problem.

Here are the examples provided by @PragTob:

@system %{
  elixir:           "1.4.0",       # 11 white-spaces
  erlang:           "19.1",        # 11 white-spaces
  num_cores:        "4",           # 8 white-spaces
  os:               "Super Duper", # 15 white-spaces
  available_memory: "8 Trillion",  # 1 white-space
  cpu_speed:        "light speed"  # 8 white-spaces
}

As you can see, there are 5 columns with more than 1 white-space, which tells the formatter to keep the vertical alignment.

fertapric · 2018-02-17T11:29:35Z

Ah! I think @josevalim that you might be talking about at which column the formatter should align the values. I would say at the right-most column. Examples:

Before running the formatter:

@system {
  erlang:    "1.4.0",
  num_cores: "4",
  os:        "Super Duper",
  available_memory: "8 Trillion"
}

After running the formatter:

@system {
  erlang:           "1.4.0",
  num_cores:        "4",
  os:               "Super Duper",
  available_memory: "8 Trillion"
}

Before running the formatter:

@system {
  erlang:    "1.4.0",
  num_cores: "4",
  os:        "Super Duper",
  available_memory:     "8 Trillion"
}

After running the formatter:

@system {
  erlang:               "1.4.0",
  num_cores:            "4",
  os:                   "Super Duper",
  available_memory:     "8 Trillion"
}

yordis · 2018-02-17T12:02:17Z

@fertapric here is a use case,

Developer 1, used two spaces so the code should align the keys.

@system {
  abc:  "x",
  abcd: "x",
  abce: "x",
}

Notice that there is not more than 1 case where there is more than 2 spaces.

Developer 2 later on,

@system {
  abc:  "x",
  abcd: "x",
  abce: "x",
  abcdefg: "x"
}

What should the formatter do?
Should align everything?

There is not 2 spaces on more than 1 row consecutive and the Developer 2 didn't notice the small extra space on the previous key (he was focus on his fix, no code style)

Another use case that make me support annotations and explicitness.

fertapric · 2018-02-17T12:13:06Z

@yordis I see that as a combination of three mistakes:

The first developer put an extra space by mistake
The formatter was not run by the first developer
The second developer put also an extra space by mistake

josevalim · 2018-02-17T12:31:13Z

@josevalim how could you talk about a subjective topic by nature, objectively? [...] we are talking about code style and by nature it is a subjective topic (based on opinions)

We are not talking about code style. We are talking about solutions that enable certain code styles. The question if it is better to align or not is completely irrelevant.

When you say 99% of the people are fine with the formatter, you are being subjective, unless you have measured so. When you say that "that style where you push the content because you want to align the values is hard to read", you are being subjective.

On the other hand, when you say that adding 2 spaces will completely change how the code is formatted and that is a "cons" for a given solution, then you are being objective.

A simple guideline is: focus on discussing each individual solution and how they compare to each other, which you are already doing most of the time. 👍

yordis · 2018-02-17T21:17:58Z

@fertapric actually the first developer did it on propose, he wanted to align the keys (developer 1 is the boss of developer 2) so the developer 2 should add 2 spaces in more than one row.

PR review, align the keys please, rejected, adding development time because the formatter is relaying on code style now.

If the route of formatter will be adding differences on code style (which I clearly do not support) then I would prefer to be explicit and annotate the code.

Neither for internal work or open source work I would like to deal with the situations that this special case is introducing.

yordis · 2018-02-17T21:29:47Z

We are talking about solutions that enable certain code styles.

@josevalim the whole point of the formatter was to have 1 code style because at the moment you introduce 2 code styles the issue you were trying to fix comes back to a small percentage (probably) and the contra-productive behavior of human kind of imposing their believe isn’t good at the end of the day, just look how much effort, time and energy we are expending (look all the use cases that this code style could present even) because we can’t roll with how it works now (no alignments).

And I took from you de advice of do not pay attention anymore to the code style because the tooling with take those decisions, period.

Despite if I agree with Elixir formatting or not it is not up to my personal preferences but whatever works at big scale, and if I truly have a recommendation, I will have to objective prove my theory that will work for everyone (no just adding an special case that will allow me to do it)

I am meticulous with that and was really unproductive to do PR review, specially working with people on different time zone where everything takes 24 hours to fix.

Adding support to different code style is the root of the problem we are trying to solve and by so, I focus more on the issue that it will carry on, and I would be fine if you will roll with one code style style over the other one, but for that, I am making the argument of how many special cases could present by aligning values.

If you definitely will go with that route, the only I could support is being explicit about “your preferred code style” by annotating your code, but I see where that route will end (too many annotations or a config file). Or add some file where all the maps will be aligned (notice, all of them, let the formatter to decide how it looks like, but do not relay on people to decide, either all maps or none of them based on a config file).

josevalim · 2018-02-27T17:15:41Z

Folks, I have a final proposal here for discussion. As said multiple times, the goal of the formatter is consistency. Given the rules being discussed here are not applied consistently, as multiple examples have shown, the only way we can support them is by being explicit about it. Therefore, my suggestion is to support a formatter annotation and only allow those annotations if they have been explicitly enabled globally.

For example, if you want to support alignment for certain data structures, you will have to:

Change your .formatters.exs and add:

[
  inputs: ...,
  supported_annotations: [:aligned]]
]

And then in your data structures:

@system %{ # aligned
  elixir: "1.4.0",
  erlang: "19.1",
  os:     "Super Duper"
}

The annotation will be done after the %{ opening bracket and be kept in that position to differentiate itself from code comments. We will support maps and keyword lists initially.

For the folks that writing aligned data structures, would you explicitly annotate your aligned data structures?

OvermindDL1 · 2018-02-27T17:22:23Z

I'm not a fan of using a comment for this, but I'd use it if no other option.
Honestly I'd prefer to just set an option in the .formatters.exs to enable alignment globally. I would actually prefer global alignment on single-line cases/keywords/etc compared to not, I just don't currently on all of them because right now it's too much effort, but if the formatter could do it for me then whoo! :-)

pragdave · 2018-02-27T17:30:18Z

I'd suggest this will be confusing, as comments are supposed to appear on the previous line. Maybe a distinguished comment such as `#+`. And elsewhere you use attributes for this kind of thing. Maybe this would be an approach (as it seems unlikely that anyone would want to align just one structure in a module). ~~~ elixir @Format :align_kw ~~~ But, before making this or any other change, you might want to ask the general Elixir user base. I did an informal poll at ElixirConf, and couldn't find anyone who used the formatter. If it really isn't that widely adopted, then I'd suggest just leaving it the way you want it to be, and keep it as a tool for the core team and anyone else who wants to follow that style. If that happens, could we remove formatter.exs from the default generator, assuming that its current contents are the default? Dave

eksperimental · 2018-02-27T17:48:38Z

Is it an option that the formatter leave the spaces in the structure untoched? There seem to be many use cases and no solution to fix them all, and the solution that José proposed seems a bit contrived.

josevalim · 2018-02-27T17:55:29Z

Maybe a distinguished comment such as #+.

A small notation can be helpful. I agree. Thanks @pragdave!

And elsewhere you use attributes for this kind of thing. Maybe this would be an approach

I don't like module attributes because it changes the code semantics for the sake of configuring formatting, as you are literally storing a module attribute in your code. Module attributes also can't be set inside functions or outside modules so it could get awkward.

But, before making this or any other change, you might want to ask the general Elixir user base.

Well, that was my plan since the beginning. :) To quote my first reply in this thread:

We will likely end-up adding more flexibility but it is still too early to tell which ones. Everyone is going to have preferences on which formatting rules they would like to keep and if we adopt all of them then the formatter loses its goal of unifying the styles across the community.

However, there was a lot of demand to discuss this particular topic, so here we are!

Elixir v1.6 is not even a month old so I don't think asking folks right now would be the best way to go. Unless you started your project on the latest Elixir less than a month ago or you actively made installing the formatter in your project a priority, teams are likely not using it yet.

In any case, this discussion has been helpful to draw a path forward and put write a proposal. Thanks everyone for your contributions!

I will go ahead and close this for now. We can bring this specific proposal back to life once we have enough usage and quorum to answer this confidently.

josevalim · 2018-02-27T17:59:13Z

Is it an option that the formatter leave the spaces in the structure untoched?

No and it won't happen. :) Keeping the user formatting opens up space for inconsistencies in the same team and ambiguous interpretation. This has been discussed above. It is a long thread though, I know.

We also want to minimize global configuration as much as we can. The goal of the formatter is overall consistency.

OvermindDL1 · 2018-02-27T18:16:45Z

Unless you started your project on the latest Elixir less than a month ago or you actively made installing the formatter in your project a priority, teams are likely not using it yet.

I've actually been doing so. I've had to exempt a few files because a couple things became unreadable (spoken of elsewhere) and had to change some for\ to for( and some with\ to with( in a lot of places (things that look like multi-arity function calls in elixir still kind of horrify me for some reason...), but overall it's worked pretty well. :-)

RichMorin · 2018-03-09T14:15:24Z

@pragdave and others have provided examples of using inline white space to produce tabular layouts for maps, align comments, etc. However, these are not the only use cases. I frequently use inline white space to clarify parallel structure in my code. I hope that any mechanism for retaining formatting of inline white space will be general enough to handle fairly arbitrary code, eg:

    assert out_node[:lat]  == 30.101
    assert out_node[:ways] == ["201"]

    node_map    = osm_data[:nodes]  |> init_map()
    rel_map     = osm_data[:rels]   |> init_map()

PragTob · 2018-03-09T14:25:23Z

@RichMorin any particular reason why the code below isn't

    node_map = osm_data[:nodes] |> init_map()
    rel_map  = osm_data[:rels]  |> init_map()

i.e. why is there white space that isn't necessary to make it align?

RichMorin · 2018-03-10T00:32:01Z

Mostly esthetics, I suspect, though my geezer's eyesight may play a part. If I'm lining up items in a column, I think the formatting looks better and easier to recognize if there are at least two spaces to the left.

FWIW, I also like to put some space between adjacent braces, brackets, and parentheses. I'll note that @pragdave does some of this in "Programming Elixir":

[ data: [ { 'State', message } ] ]

I'd also like to clarify that I'm not against the idea of code formatters, in general. For example, the Elixir code formatter could make it easier for my blind collaborator to keep her code tidy.

ZombieHarvester · 2018-03-14T19:18:35Z

Three reasons why I still don't use formatter:

Vertical alignment
Trailing commas
Pipe indentation (vertical alignment of multiline structures in the middle of pipe)

As for vertical alignment discussed in this thread: there are two use cases for me. Where I don't care about vertical alignment, and where it makes code a lot easier to understand.

Tables were invented for a reason, try reading transport timetable without vertical alignment. It's a lot easier to scan vertically for a particular value in a particular column than horizontally line by line. Table columns clearly show us repeating structures, which we can ignore as noise (or notice regularity), and then the meaningful data stands out.

I don't really care about consistency for this case (because not all structures benefit from vertical alignment), but if it means all or nothing, I'm ok with that, better to have all aligned than none.

Some examples:

Here is an Ecto schema. Its easy to notice that all fields are strings. It's also easy to ignore repeating field and :string and focus on field names.

field :full_name,     :string
field :short_name,    :string
field :brand,         :string
field :company_name,  :string
field :country,       :string

Ecto validations. Easy to notice fields that need to be greater than 0.

|> validate_number(:depth_mm,      greater_than: 0)
|> validate_number(:height_mm,     greater_than: 0)
|> validate_number(:lifespan_days, greater_than: 0)

Lists: a lot easier to scan through

[key: "(UTC-04:30) Caracas",    value: "America/Caracas"],
[key: "(UTC-04:00) Santiago",   value: "America/Santiago"],
[key: "(UTC-04:00) La Paz",     value: "America/La_Paz"],
[key: "(UTC-03:00) Greenland",  value: "America/Godthab"],

This is from our legacy Rails app. Extra spaces inside brackets also help distinguish meaningful values from noise.

assert_equal 20000004,        generator.call( 2000000       )
assert_equal 200000000004,    generator.call( 20000000000   )
assert_equal 2000000000008,   generator.call( 200000000000  )
assert_equal 20000000000004,  generator.call( 2000000000000 )

tax_rate:      row[ :tax_rate      ],
pack_quantity: row[ :pack_quantity ],
case_quantity: row[ :case_quantity ],

There is a nice episode on RubyTapas (unfortunately paid only) on how vertical alignment helps organize structures. Episode #415: Tabular Struct Refactoring with Sam Livingston-Gray

PS. @pragdave thanks for raising this. I believe this is important.

PPS. I'm also pro alphabetical sorting on keys or assignments, because more than once sorting helped find duplicates lost in a long list (unless its a job for the compiler)

PPPS. Not a fan about annotation after %{ but i can live with that. I thing that using tags as suggested by @pragdave makes more sense. I'd prefer formatter to deduce the intention and act accordingly. And to use annotations only in ambiguous cases, but again, either solution works for me as long as it helps preserve formatting.

P[n]S In case of auto aligning, i'd prefer it to round it to the next even number of spaces (considering the practice of indenting with tabs equal to 2 spaces)

josevalim · 2018-03-14T19:56:03Z

If you want custom formatting rules in that many situations, then I am afraid the formatter just isn’t for you. Even if we had heuristics, it is unlikely they would handle all of those scenarios. The same for annotations. -- *José Valimwww.plataformatec.com.br <http://www.plataformatec.com.br/>Founder and Director of R&D*

ZombieHarvester · 2018-03-14T20:09:19Z

@josevalim I like a lot the concept of automatic formatting, code uniformity, and less arguing about styles. I just hope to find a way to incorporate it into my workflow without compromising code structure expressiveness in some cases.

OvermindDL1 · 2018-03-19T14:34:52Z

Yeah I still have to state that alignment and properly organizing code into logical groupings that are not representable via syntax is so very important for code maintainability (as I dealt with this weekend when some prior-formatted code made some sections really hard to read compared to how they were before, so I ended up wasting time following other sections where the prior formatting made it obvious what I was looking for...).

camstuart · 2020-05-14T04:08:33Z

Hi All, sorry I am late to the party. I was held up writing Go when this discussion was taking place.

I am really enjoying Elixir and functional programming now that I have discovered it. Sadly it took 20 years to find a language and runtime I have a true affinity with. Well done to @josevalim, this is fantastic work.

Regarding this topic of vertical alignment, and community alignment on formatting style (pun intended) I would like to ask if any further progress has been made?

Coming from Go, for those who are not aware, formatting is extremely opinionated and baked into the core tooling. It cannot be overridden and that is all about it.

And for most people this does indeed allow us to focus on things that are more productive.
However, while all the formatting decisions of Go where easy to get used to, vertical alignment always annoyed me, and was often a source of errors.

I would argue that like indentation, white space and alignment make such a significant difference to code readability that it actually impacts on quality (bugs, smells) and I encounter a lot of other dev's who feel this way, across many languages.

The use of a formatter configuration file makes for consistency. A team can agree on specifics there and as far as I am concerned consistency at the project level has been achieved and that is just fine.

Having the ability to specify a vertical alignment strategy is a noble goal, but this thread is a great example of how hard it is to come to an accord on the specifics.

In my view, the simplest approach to implement while simultaneously satisfying the masses is an option such as:

ignore_multiple_spaces_between: true (default value of false)

Allowing the developer to be responsible for the various layouts.

So,

some_function (params)

Would become:

some_function(params)

But:

result         = some_function(params)
another_result = some_function(params)

my_map = %{
  foo: "bar",
  age: 47,
  github_username: "camstuart",
  password:        "nottelling"
}

Would remain untouched, as clearly the author has lovingly spent time and effort to make this as readable as possible.

I find that code authors who have strong views on this kind of syntax style are more than happy to manage formatting by hand, and in fact would prefer to.

Thanks.

josevalim · 2020-05-14T06:36:06Z

Hi @camstuart! 👋

We have made no progress on this feature per se but by Elixir v1.10 we have made public almost all (if not all) private APIs used by the formatter. This means folks that disagree with their Elixir opinions can maintain their own version of the formatter that respects their choices.

Previous attempts at meeting halfway and adding more customization to Elixir's formatter have been exhausting and energy consuming, so it is an area I am not particularly interested in tackling (especially because I am on the camp of an opinionated formatter too 😃).

Rueben65 · 2023-12-03T17:47:12Z

Preserving spaces in the structure during formatting is a viable option, given diverse use cases and the complexity of proposed solutions.

fishcakez mentioned this issue Oct 9, 2017

Code formatter incorrectly formats multiline lists when there is a trailing comma #6646

Closed

josevalim closed this as completed Oct 9, 2017

josevalim reopened this Jan 19, 2018

josevalim added Note:Discussion App:Elixir Note:Formatter labels Jan 19, 2018

devonestes mentioned this issue Jan 31, 2018

Add dry_run functionality bencheeorg/benchee#178

Merged

josevalim closed this as completed Feb 27, 2018

whatyouhide mentioned this issue Mar 22, 2018

Code.format_string! Makes certain cases less readable (IMHO) #7473

Closed

josevalim mentioned this issue May 16, 2018

Add the :trailing_comma option to the formatter #7689

Closed

Could code formatter leave spaces in multiline keyword lists? #6647

Could code formatter leave spaces in multiline keyword lists? #6647

Comments

pragdave commented Oct 9, 2017

fishcakez commented Oct 9, 2017

pragdave commented Oct 9, 2017 via email • edited by josevalim Loading

fishcakez commented Oct 9, 2017 • edited Loading

josevalim commented Oct 9, 2017

RichMorin commented Jan 18, 2018

josevalim commented Jan 19, 2018 • edited Loading

OvermindDL1 commented Jan 19, 2018

pragdave commented Jan 19, 2018 • edited Loading

josevalim commented Jan 19, 2018 via email • edited Loading

pragdave commented Jan 19, 2018

josevalim commented Jan 19, 2018 via email • edited Loading

fertapric commented Jan 26, 2018

PragTob commented Jan 31, 2018

josevalim commented Feb 16, 2018 • edited Loading

fertapric commented Feb 16, 2018

josevalim commented Feb 16, 2018 • edited Loading

devonestes commented Feb 16, 2018

josevalim commented Feb 16, 2018 • edited Loading

AndrewDryga commented Feb 16, 2018

josevalim commented Feb 16, 2018

AndrewDryga commented Feb 16, 2018 • edited Loading

josevalim commented Feb 16, 2018

fertapric commented Feb 17, 2018 • edited Loading

josevalim commented Feb 17, 2018 • edited Loading

yordis commented Feb 17, 2018 • edited Loading

fertapric commented Feb 17, 2018 • edited Loading

fertapric commented Feb 17, 2018 • edited Loading

yordis commented Feb 17, 2018 • edited Loading

fertapric commented Feb 17, 2018

josevalim commented Feb 17, 2018 • edited Loading

yordis commented Feb 17, 2018

yordis commented Feb 17, 2018 • edited Loading

josevalim commented Feb 27, 2018

OvermindDL1 commented Feb 27, 2018 • edited Loading

pragdave commented Feb 27, 2018 via email • edited by josevalim Loading

eksperimental commented Feb 27, 2018

josevalim commented Feb 27, 2018 • edited Loading

josevalim commented Feb 27, 2018

OvermindDL1 commented Feb 27, 2018

RichMorin commented Mar 9, 2018

PragTob commented Mar 9, 2018

RichMorin commented Mar 10, 2018 • edited Loading

ZombieHarvester commented Mar 14, 2018

josevalim commented Mar 14, 2018 via email

ZombieHarvester commented Mar 14, 2018

OvermindDL1 commented Mar 19, 2018

camstuart commented May 14, 2020

josevalim commented May 14, 2020

Rueben65 commented Dec 3, 2023 • edited Loading

pragdave commented Oct 9, 2017 via email •

edited by josevalim

Loading

fishcakez commented Oct 9, 2017 •

edited

Loading

josevalim commented Jan 19, 2018 •

edited

Loading

pragdave commented Jan 19, 2018 •

edited

Loading

josevalim commented Jan 19, 2018 via email •

edited

Loading

josevalim commented Jan 19, 2018 via email •

edited

Loading

josevalim commented Feb 16, 2018 •

edited

Loading

josevalim commented Feb 16, 2018 •

edited

Loading

josevalim commented Feb 16, 2018 •

edited

Loading

AndrewDryga commented Feb 16, 2018 •

edited

Loading

fertapric commented Feb 17, 2018 •

edited

Loading

josevalim commented Feb 17, 2018 •

edited

Loading

yordis commented Feb 17, 2018 •

edited

Loading

fertapric commented Feb 17, 2018 •

edited

Loading

fertapric commented Feb 17, 2018 •

edited

Loading

yordis commented Feb 17, 2018 •

edited

Loading

josevalim commented Feb 17, 2018 •

edited

Loading

yordis commented Feb 17, 2018 •

edited

Loading

OvermindDL1 commented Feb 27, 2018 •

edited

Loading

pragdave commented Feb 27, 2018 via email •

edited by josevalim

Loading

josevalim commented Feb 27, 2018 •

edited

Loading

RichMorin commented Mar 10, 2018 •

edited

Loading

Rueben65 commented Dec 3, 2023 •

edited

Loading