Running the new Elixir formatter

nathanl · November 2, 2017, 2:23pm

I want to try the formatter out on a new module I’m writing. I’m running Elixir 1.5.1 and don’t use a version manager (yet), but thought I’d clone Elixir master and try the formatter from there.

I don’t understand why this doesn’t work.

$: git clone git@github.com:elixir-lang/elixir.git
$: cd elixir
$: make clean compile
$: bin/elixir -v # Elixir 1.6.0-dev (74da11f) (compiled with OTP 20)
$: bin/mix -v # Mix 1.5.1 # ???
$: bin/mix help format # ** (Mix) The task "format" could not be found

Why is mix an older version that can’t run bin/mix format ~/some_file.ex?

josevalim · November 2, 2017, 2:48pm

Mix is a regular Elixir script that will use the Elixir on your PATH. You can do bin/elixir bin/mix help format.

AstonJ · November 29, 2017, 2:51am

This is cool!

cleaver · December 7, 2017, 10:11am

There’s a plugin for Atom which is an easy way to play around with this…

I was able to install 1.6 from master in a separate directory and point the plugin to that version while I work on 1.5

riverrun · December 9, 2017, 5:21am

I ran the new formatter on all of the libraries I maintain, and I think it’s a testament to the Elixir devs that 1) I was so confident that there would be no problems, and 2) it did work perfectly.

It was also funny to see what I thought was a concise two-line function call converted into a 14-liner

josevalim · December 13, 2017, 8:57pm

Please do share.

riverrun · December 14, 2017, 3:26am

This is the example I was talking about. It’s from this example Phoenix app.

This wonderfully concise little function:

def remove_old_sessions(session_age) do
    now = System.system_time(:second)
    Enum.map(list_users(), &change(&1, sessions: :maps.filter(fn _, time ->
      time + session_age > now end, &1.sessions)) |> Repo.update)
  end

became this:

def remove_old_sessions(session_age) do
    now = System.system_time(:second)

    Enum.map(
      list_users(),
      &(change(
          &1,
          sessions:
            :maps.filter(
              fn _, time ->
                time + session_age > now
              end,
              &1.sessions
            )
        )
        |> Repo.update())
    )
  end

Ok, perhaps it’s a little easier to read now

josevalim · December 14, 2017, 12:03pm

To be very honest, I think both before and after are not readable. The code is too dense. When you skip variables, you lose the opportunity to give names to values, so the reader needs to keep a lot in their head.

Here is one alternative:

def remove_old_sessions(session_age) do
  now = System.system_time(:second)

  for user <- list_users() do
    new_sessions =
      user.sessions
      |> Enum.filter(fn {_, time} -> time + session_age > now)
      |> Enum.into(%{})

    user
    |> change(sessions: new_sessions)
    |> Repo.update()
  end
end

Although it probably wouldn’t hurt to break it into more functions.

PS: if you want to further abuse comprehensions:

    new_sessions =
      for {_, time} <- user.sessions,
          into: %{},
          do: time + session_age > now

riverrun · December 14, 2017, 1:37pm

Anyway, the formatter worked perfectly, which was my main point.

net · December 14, 2017, 2:49pm

For the last comprehension I think you mean

    new_sessions =
      for {key, time} <- user.sessions,
          time + session_age > now,
          into: %{},
          do: {key, time}

josevalim · December 14, 2017, 5:37pm

Ah yes, thank you! The filter (non-comprehension) probably looks better then!

atomkirk · January 20, 2018, 3:12pm

But the formatter adds parentheses, so there will be no confusion/inconsistency when using the formatter…

From what I can see, people in this thread overwhelmingly value the trailing comma.

My argument is: We have lists of things that are always being added to in our codebase, like currencies, allowed/required attributes in ecto models etc, and keeping those sorted drastically reduces pointless git conflicts from two people adding things to the end of a list. Most editors have a keyboard shortcut to select and sort lines. I say we allow/require trailing commas for lists. I could understand not having them on function calls, but lists and maybe maps, there seems to be a loud preference and I think there have been some good arguments made for it. I don’t see an edge case inconsistency that won’t be present when using the formatter as outweighing the benefits of the trailing comma

OvermindDL1 · January 23, 2018, 11:46pm

F5 to sort selected lines for me. ^.^

And yes, I use it heavily, especially with multi-line maps!

DevotionGeo · January 26, 2018, 1:06pm

And reading only 4 digits number is easier, but making it >=5 will be better.

DevotionGeo · January 26, 2018, 1:17pm

I like this mix generated migration file

defmodule Gupshup.Repo.Migrations.CreateUsers do
  use Ecto.Migration

  def change do
    create table(:users) do
      add :name, :string
      add :email, :string
      add :password_hash, :string

      timestamps()
    end

    create unique_index(:users, [:email])
  end
end

more than the following formatted file

defmodule Gupshup.Repo.Migrations.CreateUsers do
  use Ecto.Migration

  def change do
    create table(:users) do
      add(:name, :string)
      add(:email, :string)
      add(:password_hash, :string)

      timestamps()
    end

    create(unique_index(:users, [:email]))
  end
end

And I think it’s also more readable, anyway it’s a matter of taste and with time we’ll get used to it.

In all other places Elixir formatter does an amazing job.

mischov · January 26, 2018, 4:35pm

This solution is not compatible with dates after 9999- Elixir needs conventions that’ll work in the long-term!

OvermindDL1 · January 26, 2018, 5:48pm

1_555_123_4567 would break then.

Deithrian · January 29, 2018, 6:21pm

Not sure if bugs?
Using vscode with ElixirLS and 1.6 formatter.

1.Removed parentheses from “if” statements:

if String.length(io_data_valid_string) !== 0 do

vs

if (String.length(io_data_valid_string) !== 0) do

2.Placed “case” statements on new line and an additional new line between them

case db_query_result do
  {:ok, []} ->
    {:error, socket}

  {:error, _reason} ->
    {:error, socket}

vs

case db_query_result do
  {:ok, []} -> {:error, socket}
  {:error, _reason} -> {:error, socket}

3.Closing parenthesis placed on new line:

   with
     {:ok, socket} <-
       SomeApp.SomeModule.some_function(
         some_argument_1,
         some_argument_2,
         some_argument_3,
         some_argument_4,
         socket
       ) do

vs

   with 
     {:ok, socket} <-
       SomeApp.SomeModule.some_function(
         some_argument_1,
         some_argument_2,
         some_argument_3,
         some_argument_4,
         socket)
    do

I expect to see a closing parenthesis after the last argument, like I expect to see a period at the end of a sentence
.
4. “with” statement’s “do” placed at the end of a case, leaving the next line looking like it’s misplaced, or not formatted.

  def sanitize(io_data, socket) do
    with {:ok, socket, io_data_valid_string} <- check_if_string_valid(io_data, socket),
         {:ok, socket} <- check_if_empty(io_data_valid_string, socket),
         {:ok, socket, io_data_valid_string_safe} <- escape_html(io_data_valid_string, socket) do
      {:ok, socket, io_data_valid_string_safe}
    else
      {:error, socket} -> {:error, socket}
    end
  end

vs

  def sanitize(io_data, socket) do
    with {:ok, socket, io_data_valid_string} <- check_if_string_valid(io_data, socket),
         {:ok, socket} <- check_if_empty(io_data_valid_string, socket),
         {:ok, socket, io_data_valid_string_safe} <- escape_html(io_data_valid_string, socket)
    do
      {:ok, socket, io_data_valid_string_safe}
    else
      {:error, socket} -> {:error, socket}
    end
  end

Are these the way it’s supposed to be or are any of them bugs?
If they are bugs any idea if it’s vscode ElixirLS related or the formatter itself?

If they’re not bugs, can someone explain why:

Remove parentheses from “if” statements?
Why double the lines for “case” statements from 3 to 6?
Why place closing parenthesis on a new line instead at the end of last argument?
Why is this

{:ok,
[
%{
“something_something_something” => something_something_something,
“something_something_something” => something_something_something,
“something_something_something” => something_something_something,
“something_something_something” => something_something_something,
“something_something_something” => something_something_something,
“something_something_something” => something_something_something,
“something_something_something” => something_something_something,
“something_something_something” => something_something_something
}
]} ->
{:ok, something_something}

better than this?

  {:ok,[%{
       "something_something_something" => something_something_something,
       "something_something_something" => something_something_something,
       "something_something_something" => something_something_something,
       "something_something_something" => something_something_something,
       "something_something_something" => something_something_something,
       "something_something_something" => something_something_something,
       "something_something_something" => something_something_something,
       "something_something_something" => something_something_something
       }]} ->
    {:ok, something_something}

Or even

  {:ok,
   [
     %{
       "something_something_something" => something_something_something,
       "something_something_something" => something_something_something,
       "something_something_something" => something_something_something,
       "something_something_something" => something_something_something,
       "something_something_something" => something_something_something,
       "something_something_something" => something_something_something,
       "something_something_something" => something_something_something,
       "something_something_something" => something_something_something
     }
   ]
  } ->
    {:ok, something_something}

But why have a new line for every opening bracket and then only have two of the three closing brackets on the same line “]} ->”?

NobbZ · January 29, 2018, 6:33pm

Why place them there in the first place? They are not necessary.

Because it’s easier to distinguish clauses now.[quote=“Deithrian, post:139, topic:9344”]
3. Why place closing parenthesis on a new line instead at the end of last argument?
[/quote]

Because it’s considered looking more nice by the core team. Also it’s very common through a lot of formatting styles across languages. Also closing parents on next line goes hand in hand with a trailing comma (which we can’t place :().

Nothing is better or worse, but it’s uniform among projects, which makes it easier for developers to switch code bases.

I do not like the format produced by gofmt and elixir format as they are, but I love that I will see the same formatting everywhere.

Deithrian · January 29, 2018, 6:45pm

Hopefully not