sodapopcan

sodapopcan

Proposal: Have the formatter put `with`'s `do` on its own line

So after complaining about this for the third or fourth time on this forum, I figured I should make a proposal.

TL;DR

with can be hard to read sometimes as it can be hard to pick out where the clauses end and the body begins. While it’s syntactically valid to put the do on its own line, the formatter forces it back up on the same line as the last clause.

The Problem

While this is a very simple proposal, here are a truckload of words describing it in detail:

A non-insignificant number of people seem to have a problem with with. I think these people fall into two camps. The first has the people who are pipline obsessed who wrap their code or create macros that will make their everything they do pipeable. This proposal is not about this camp as that is a completely separate issue. The second has the people who very much like with as a construct but find the syntax a little clunky at times. This is evident by libraries like happy_with. While I don’t use any of these libraries myself, I am a member of this group.

For me the problem with with isn’t that I have to put commas or that I have to use <- (never understood this complaint) but simply that when there are more than a few clauses and the lines get a little longer—and especially when we aren’t necessarily matching on tuples—it is visually very difficult to find the do, ie, where the clauses end and the body begins. Having played around with it, I’ve found that the very simple act of putting the do on its own line completely solves this (for me).

I’m going to start by sharing this particularly hairy example from LiveBeats. I’m sure this will result in some saying, “Well I would find a different way to write this,” and I think I would try to too (but I haven’t) but this isn’t the point.

Here it is:

with version when version != :invalid <- lookup_version(version_bits),
     layer when layer != :invalid <- lookup_layer(layer_bits),
     sampling_rate when sampling_rate != :invalid <-
       lookup_sampling_rate(version, sampling_rate_index),
     bitrate when bitrate != :invalid <- lookup_bitrate(version, layer, bitrate_index) do
  samples = lookup_samples_per_frame(version, layer
  frame_size = get_frame_size(samples, layer, bitrate, sampling_rate, padding)
  frame_duration = samples / sampling_rate
  <<_skipped::binary-size(frame_size), rest::binary>> = data
  parse_frame(rest, acc + frame_duration, frame_count + 1, offset + frame_size)
else
  # ...
end

So yes, this is a little hairy, but I find that the meat of my negative reaction I get looking at this is that my brain can’t immediately identify where the clauses end and the body begins. The indentation makes it feel like someone just forgot to format which causes quite the visceral reaction in me along with a bit of mild panic! Sounds dramatic but I’m being sincere.

Here is it with do on its own line.

with version when version != :invalid <- lookup_version(version_bits),
      layer when layer != :invalid <- lookup_layer(layer_bits),
      sampling_rate when sampling_rate != :invalid <-
        lookup_sampling_rate(version, sampling_rate_index),
     bitrate when bitrate != :invalid <- lookup_bitrate(version, layer, bitrate_index)
do
  samples = lookup_samples_per_frame(version, layer)
  frame_size = get_frame_size(samples, layer, bitrate, sampling_rate, padding)
  frame_duration = samples / sampling_rate
  <<_skipped::binary-size(frame_size), rest::binary>> = data
  parse_frame(rest, acc + frame_duration, frame_count + 1, offset + frame_size)
else
  # ...
end

I still have that “Whoa, ok what’s going on here,” but I feel way calmer and more prepared/willing to get right to reading through it and figuring it out.

So here’s a much simpler example:

with {:ok, contents} <- File.read("/Users/andrew/passwords.txt"),
     lines = contents |> String.split("\n") |> Enum.map(&String.split/1),
     ["My Bank Account", password] <- Enum.find(lines, fn [label, _] -> label == "My Bank Account" end) do
  do_a_hack(bank_account, password)
end

But even here, while the indentation certainly helps, I still get that “looks like a botched formatting attempt” feeling, which is distracting.

I think is better:

with {:ok, contents} <- File.read("/Users/andrew/passwords.txt"),
     lines = contents |> String.split("\n") |> Enum.map(&String.split/1),
     ["My Bank Account", password] <- Enum.find(lines, fn [label, _] -> label == "My Bank Account" end)
do
  do_a_hack(bank_account, password)
end

Current Solutions

Using parens, the formatter leaves the following alone:

with (
  {:ok, contents} <- File.read!(filename),
  true <- String.contains?("foo")
) do
  do_something_with(contents)
end

This is better but I think the non-paren version reads better. And of course having to add parens to such constructs is not very Elixiry.

Solution

Have the formatter put do on its own line. I’m specifically proposing that the formatter forces this style. I would be somewhat happy with (no pun intended but I’m pretty happy about it) the formatter accepting both styles, but of course that starts to degrade the usefulness of a formatter.

Thanks for reading!

EDIT: Just fixed some code sample errors and a small bit of wording.

Most Liked

christhekeele

christhekeele

I like this proposal.

aragorn-ii-elessar-sword

I wonder if the current implementation of the formatter makes it hard to detect when a single-cause with would be able to fit do on the same single line as with, which is why it defaults to simply clinging to the last clause. Since, obviously, the below is undesirable:

with {:ok, contents} <- File.read!(filename)
do
  do_something_with(contents)
end

A similar formatting outcome happens a lot with long function clauses so I imagine they use a similar heuristic, see:

Many guards
def multi_transaction_lock(multi = %Ecto.Multi{}, scope, id)
    when is_atom(scope) and is_integer(id) do
  multi
end

VS

def multi_transaction_lock(multi = %Ecto.Multi{}, scope, id)
    when is_atom(scope) and is_integer(id)
do
  multi
end

Or even, from the very same LiveBeats function

Much pattern matching
defp parse_frame(
         <<
           0xFF::size(8),
           0b111::size(3),
           version_bits::size(2),
           layer_bits::size(2),
           _protected::size(1),
           bitrate_index::size(4),
           sampling_rate_index::size(2),
           padding::size(1),
           _private::size(1),
           _channel_mode_index::size(2),
           _mode_extension::size(2),
           _copyright::size(1),
           _original::size(1),
           _emphasis::size(2),
           _rest::binary
         >> = data,
         acc,
         frame_count,
         offset
       ) do
  with...
end

VS something more like


defp parse_frame(
  <<
    0xFF::size(8),
    0b111::size(3),
    version_bits::size(2),
    layer_bits::size(2),
    _protected::size(1),
    bitrate_index::size(4),
    sampling_rate_index::size(2),
    padding::size(1),
    _private::size(1),
    _channel_mode_index::size(2),
    _mode_extension::size(2),
    _copyright::size(1),
    _original::size(1),
    _emphasis::size(2),
    _rest::binary
  >> = data,
  acc,
  frame_count,
  offset
) do
  with...
end

More generally, I’d personally prefer the do on its own line in all the above multi-line cases, under the philosophy that indentation is closely aligned with the lexical scoping of blocks.

do often delineates pattern-matching constructs (which have their own scoping rules) on the left from bound code on the right; and in multi-line cases having the do on its own line “resets” my mental expectations of the lexical scope when scanning code, which I think is an accurate mental model of these constructs.


I bet it is possible to develop a formatter extension that applies this rule to just multi-clause with expressions, or even to all multi-line do blocks as per my preference—I’d be interested to give such an extension a go, if anyone develops one to trivially preview how this proposal looks on real-world codebases.

smathy

smathy

My real point here is just that it’s likely that there’ll be people who feel strongly, and arguments and edge cases on both sides that will be difficult to accommodate.

I don’t have a horse in this race, so the following are just general thoughts:

  1. The comma thing is the same issue with the trailing do anyway, so that’s not a comparative negative for your new line do. I rely on my diff tools to make any reordering of attributes easier to see, it’s a shame github doesn’t have a more reliable “color-words” implementation, but I generally review code locally anyway.

  2. My approach to reading code is no doubt heavily influenced by the fact that I started coding in an 80 column monochrome world. In fact more often than not the first step to reading any sizable source code was to send a print job off to the dot matrix.

  3. I have been told I read code like a compiler.

  4. Wide screens and editor tooling means that I really can’t relate to the feelings you’re expressing about readability difficulty. Ie. the two big messy original examples you showed look the same to me. I’m not unfamiliar with your position, I’ve heard it from many others, it’s just not something that impacts me. The only thing that threw me reading that first example was the missing ) in the lookup_samples_per_frame call.

  5. I’ve often wondered whether it might be a good learning exercise for developers to switch off all the color and helpers we’re so used to (reliant upon) these days, and just get some experience reading black and white source code.

  6. I secretly wish I could convert the world to my way of thinking about this, because all discussions of style feel like bikeshedding to me. That “panic” you talked about hits me when I get invited to a meeting to discuss style :slight_smile:

  7. There are definitely some things which I believe are inherently hard for anyone to read, such as your assign/2 reordered attributes thing. Obviously big diffs where only indenting has changed fall into the same bucket.

tomekowal

tomekowal

I have another problem with the formatting of with. It is somewhat artificial and solved by tooling, but it is annoying from time to time. Functions between with and do are the only thing that is indent odd amount of spaces :stuck_out_tongue:

This is annoying when I work with editors that have commands like indent that atumoatically put two spaces. Unlike if and case that work with one expression before do part, with is designed to work with many. So I’d actually see it like this:

with
  expr1 <- func1()
  expr2 <- func2()
do
  expr3
  expr4
else
  :a -> a
  :b -> b
end

Where Next?

Popular in Proposals: Ideas Top

sbennett33
When building a component library, it is often useful to give users the ability to customize the underlying element or component to use. ...
New
woylie
We are seeing a lot of warning logs like this: navigate event to "https://someurl" failed because you are redirecting across live_sessio...
New
azyzz228
The slow network is known to be an Achilles heel of LiveView’s architecture. Recently, I was working on creating a fast rendering map wi...
New
andreamancuso
Hey folks, This might sound niche, but I think it’s worth bringing up - especially given Phoenix’s reputation for being lightweight, por...
New
marcandre
I notice that most events have bindings (e.g. phx-keyup) but not the input event. The input event is the preferred way to interact with ...
New
engineeringdept
In 2026 double submit/session tokens are no longer necessary to prevent against CSRF attacks. Instead, we can use the Sec-Fetch-Site head...
New
sevensidedmarble
Hello all, Apologies if this has been proposed before I guess, but I have a very simple one: With the increasing importance of LV, I th...
New
BartOtten
I’d like to propose that we refrain from using the term "DeadView" as the opposite of “LiveView” and instead choose an alternative. A new...
New
spicychickensauce
I’ve previously explored what is possible today with hacks to implement view transitions in our apps: I have since created a fork to im...
New
Flo0807
Hello everyone! Phoenix LiveView v0.18 introduced the special attributes :let, :for and :if. In addition to the :if special attribute, I...
New

Other popular topics Top

Emily
I have VueJS GUIs with the project generated using Webpack. I have Elixir modules that will need to be used by the VueJS GUIs. I forese...
New
lessless
I believe there are people here who are dealing with CSV files import on the daily basis, and since Excel is a really popular tool there ...
New
johnnyicon
Hi all, I’ve just started learning Elixir and Phoenix Framework, so please pardon my n00bness at this stage. I’m trying to use Postgres...
New
fireproofsocks
Forgive me if this is obvious, but how does one delete a database record WITHOUT selecting it first? Ecto.Repo — Ecto v3.14.0 has exampl...
New
RisingFromAshes
I’ve read in another post that it may be possible with a router helper - but I couldn’t find an appropriate one, and tbh, I’m still just ...
New
dblack
I’ve got an issue with an app and I’ve no idea of how to troubleshoot it. I’m hoping someone here might have seen something similar. I p...
New
hariharasudhan94
I would like to know what is the best IDE for elixir development?
New
dogweather
I wrote this comment on r/haskell, and it’s not popular there. :wink: But I think I’m on to something… Haskell reminds me of Java, and e...
New
jononomo
For some reason my phoenix channels are working for me in my local dev environment, but as soon as I deploy via Docker, I get a 403 error...
New
JorisKok
I have a server on AWS, and was running a load test using artillery. When looking at the Phoenix dashboard I see the Ports going to 100% ...
New

We're in Beta

About us Mission Statement