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.

23 Likes

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.

6 Likes

:sweat_smile:

I occurred to me I didn’t include that this shouldn’t apply to single clause withs but didn’t edit as I figured it was obvious, as you say!

It never occurred to me that it could be a logistical problem for the formatter to accomplish this. I keep forgetting that we now have the ability to write formatter extensions though honestly, I would prefer to see this in the formatter proper. I’m not generally super picky about syntax at this level (as in where things physically go) and unbegrudgingly adopted all of the formatter’s rules into my own style (though I do feel for those who need hardcoded matrices). This is the only rule that I can’t get behind and it would great if code I read in the wild has this style and not have to fight co-workers on it. Of course it’s subjective but that’s where I’m coming from!

I wasn’t even thinking about these! This one does indeed bug me a little but least the when is super obvious, so it’s never caused me anywhere close to the negative reaction I get when I big withs. Similarily I just avoid multiline function heads like the plague though I also never do that binary matches to that extent. But I would be all for it applying to these scenarios as well.

That would be great! As per the ideals of proposals I should really be doing this now that I remembered it’s possible. I have a few things to do first but may also take a stab.

Thanks for all the feedback!

2 Likes

Are you aware of the freedom_formatter? I would look at their code and probably raise a PR there to add this as an option.

1 Like

The Elixir version of the “open brace at the end of the line or on a new line” debate.

My 2¢ is that I put the do on a newline if it would land past column 80. But also, I don’t see style so do what you want in your code :slight_smile:

2 Likes

Thanks for pointing this out! But my goal here is to suss out appetite for getting into the formatter-proper as I personally value having code that is inline with a community standard. I realize it’s a long shot and if it’s a no-go that’s a-ok and I’ll probably just keep using it. I’ve sat on this for a very long time now and just wanted to see what the response was.

I don’t quite think that is a fair comparison. Brace-on-its-own-line always felt like purely personal preference to me. I don’t have a good argument as for why I find my brace preference more readable, nor have I ever heard a convincing argument from anyone else. As for with/do, I find there to be a night-and-day difference between the two styles from a readability standpoint, but if I’m the only one who feels that way then that’s ok! That’s what I’m trying to figure out here :slight_smile:

1 Like

Sorry, I was just being facetious. I actually think there’s merit in both sides of the brace debate too. I’m really not the right person to debate style, and from the look of the thread you’re definitely not the only one who feels strongly about this.

1 Like

I figured! I was just kind of trying to convey that motivation of this proposal is that I find this to massive improvement and status quo to be very hard to read. It’s also motivated by the number of complaints I’ve heard about with. I have several other things about the formatter I would change if I had the choice, but I would never make a proposal about them because they are 100% purely preference. I do actually think the trailing coma thing is unfortunate, especially for diffs, but I don’t really care all that much. Although it is the #1 reason I only ever use assign/2 and reassign the socket:

socket =
  socket
  |> assign(:foo, "foo")
  |> assign(:bar, "bar")

{:noreply, socket}

I reorder assigns often enough during development and it can be quite frustrating otherwise. I actually made a credo rule than bans assign/2 :sweat_smile: This doubles to make LiveComponent assigns more explicit until we get attrs for them since without assign/2 you can’t just do the ol’ |> assign(assigns) in the update callback.

1 Like

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.

3 Likes

Thanks, this is the kind of feedback I’m looking for! Although it does make me feel I’m not quite expressing how much of a problem this is for me as I generally really don’t care about this stuff and like to think I can get used to almost any style. It’s this one thing has been bugging me for since the formatter has been out and has it’s never dissipated. I’m just gauging how others feel and I’m a-ok with being told I’m alone :slight_smile: I may end up taking @cmo’s advice and making a PR to freedom_formatter. I am well aware this is Bikesheddy but I enjoy bikeshedding from time to time (or more often than that :sweat_smile:). I feel this is an appropriate platform for it where people can ignore it if they want. I don’t tolerate bikeshedding on teams and even wrote a short blog post about how my old team used to deal with it.

As for your comments, I’m… mostly in line with all them, I think? I never buy the “wide screen” argument. I’m not going to dive into it but newspapers are laid out in columns for a reason.

I did start coding in regular ol’ NotePad although I didn’t get really serious about programming until years later (I’m in my 40s and only started my career at 28). Technically I did also start on an 80-column display learning QBasic when I was very young, but that never took :slight_smile: But in response to the quoted text there, several years ago I did indeed switch off all of my colour thanks to this guy who also called syntax highlighting utterly useless. I tried really hard for 2-3 months to convince myself it was going to work out but holy heck, I still vividly remember the wash of relief I felt when I brought back the colour :sweat_smile: It just did not work for me and call it what you will but colour schemes are an absolute must for me. I actually use my own that highlights my primary language differently from JS and CSS as I find these visual queues incredibly helpful. But ya, there might be a deeper problem where most code just looks like noise to me :upside_down_face: I dunno! It’s largely why I’m very attached to Elixir as I find it the easiest for understanding complex code.

Ha, that’s a blast from the past, being from both a Perl background AND Australia, I’m very familiar with Damian Conway. In fact I ported his Acme::Bleach to Ruby :slight_smile:

Bravo for trying it for that long, maybe it’s an inherent trait of some people then :person_shrugging: Maybe some missing strategy.

FWIW, I reject the “newspaper columns” argument, code is fundamentally different to prose, you have to read prose sequentially, whereas you should be reading code in hierarchical chunks.

This all might be totally subjective/unrelatable, but…

To parse/comprehend a line in the with I skim to <- or =, and then find the ), or do. So because you’re looking for symbols (or symbol combinations) the “width works against you” argument of newspaper columns just doesn’t apply.

I know your position isn’t unique, I’ve seen the code where people line up operators on successive lines of code, as if columnizing their code makes it more readable. I don’t deny that it makes it aesthetically more pleasing, but I do reject that it makes it more readable - it actually makes it harder to keep track of which line you’re on because you’ve lost the (not massively significant, but every bit helps) information about which column the (say) = symbol was in for this line.

So to read the whole with block I’m actually just scanning for the do, honestly if it’s at the start of the line or the end makes very little difference. When I introspect as I do this in actual highlighted code, I find that I just straight down to the outdent line and just see the trailing do above in my peripheral. I even experimented with code where the with statements are at the same indent level as the function body, in those cases I hit the end prematurely because I was initially dismissing the function body as part of the with, but as soon as I hit it I backtracked to look for the do and it all made sense again. So that was definitely a small additional cognitive load, but not significant. About the same as hitting the end of your line with the missing ) where I dropped to the next line expecting an indent and a , or ) but found a variable so knew it was a typo.

Oh, that’s cool!!

So long as you are indulging me in some bikeshedding… :slight_smile: I don’t see it as a 1-1 but shorter lines are more digestible. Not that I even want to touch on this but I’d say it’s the opposite side of the tabs vs spaces war in that it’s also an accessibility issue. I am overly sensitive to having to move my eyes side-to-side too much and I certainly don’t want to ever be moving my neck. My rule of thumb is pretty much “A comma signifies a new line.” There are always exceptions, of course, like functions with 2 or 3 parameters, but all my maps and lists are always on multiple lines if they contain any more than one member, even if they fit on one line (though again, there are always exceptions).

I’m in 100% agreement with this. 110% actually :sweat_smile: On top of everything you said, talk about horrendous diffs!!!

Continuing on building this shed…

For me my with proposal is about vertical space. I don’t want to scan for a do, I want an in-my-face delineation of sections. Not to be a dick about it but I would actually through your “not massively significant, but every bit helps” back at you in this regard :wink: Although for me it is massively significant :sweat_smile: Well, significant enough.

Ha, we’re onto the door hardware at this point :wink:

It’s funny because for me the inverse is the case, having to visually wrap from the end of one line back to the start of the next is a (small) mental load I’d rather not have, again not significant enough to do anything about it :slight_smile:

The similarity to the tab vs space debate for me is that you can use tooling to wrap a long line at whatever column or character you want (in the same way that with a tab you can use your tooling to have whatever indent you want). So for me the long line is friendly to all, the chopped line is only friendly to those who (a) like chopped lines and (b) agree with where you chopped it.

Funny, because I’ve been wanting to add (but couldn’t make it relevant) the fact that in my own code it seems that I usually add an empty line after the trailing do :slight_smile: I didn’t even remember that I did that.

Right, which is probably why I add an empty line :slight_smile: I’ve said a few times though, I don’t mind the do on a new line either. I’m not arguing FOR trailing do my position is ~s[I don’t care enough to warrant a change from _“whatever it currently is”_] and generally I’m enjoying discovering a well thought through perspective from someone who does.

The difference with lining operators up is the defaults. Right now the default for do is trailing, you’re considering spending time changing that, the default for writing a bunch of assignments is not lining up the operator, so the onus is on those that took the time to have a net improvement and I’d argue they failed.

I will add, because the impact is so low, I don’t “fix” code like that when I come across it. Ie. once that code is written, if I wanted to change it then the onus would be on ME now to have a compelling argument to.

Btw, mostly mine do too, but my argument there is that logically a vertical list makes more sense than a horizontal list for mapping keys to values, the binding of the key to its value is the far more significant than each pair’s relationship to its neighbors; there’s rarely any positional relevance. So it’s better aligned to the underlying data to present a map vertically.

This is not true for function parameters though, where the fact that this is a function header is of far greater significance than any individual parameter, also their order is often an important indicator of significance (the earlier params are usually more important than the later ones, and/or the later ones are treated in some special way - like params with defaults, or functions).

So basically you are in total agreement with the spirit of this proposal :smiley: I would accept a space but the formatter says THAT IS NOT ALLOWED! I didn’t mention this because it really goes against all other formatting rules—there is no precedent where there is an empty line after the beginning of a block. do on its own line seemed like the more likely choice to have even a sliver of a chance of getting through.

That’s fair and I’m certainly enjoying this convo :smiley: I’m not banking on getting this through and as I’ve stated it’s not the end of the world if doesn’t happen.

I’m not going to touch the tabs vs spaces debate, though. I do see the argument about long lines and formatting and the only retort is that it makes for some chaotic formatting if you leave it up to the editor but ya, I realize I’m on shaky ground there. If you want to really panic, read through this thread (so don’t actually read it).

In any event, I have a love-hate relationship with reading code. I like to read it quickly when I can, especially when faced with a production bug so that any bike-shedding I do around style is aimed at making that easier (for me… lol). Vertical maps and lists are mainly so I don’t miss and entry with quick scanning.

Heh, I do a lot of things that formatters hate.

Yeah, I get this, and certainly for your aim here it’s s non-starter. My argument against “no precedent” is that there’s no precedent for the amount of code we stuff into with before the do either :slight_smile: It’s ok to have unique solutions to unique problems.

There was another thread recently that I’m sure you’ll remember about trying to make with blocks (what do we even call that chunk of code, header-block? Personally I like “stanzas” but :person_shrugging:) simpler, or easier to read, or whatever. People act as if it’s mandatory to use with :slight_smile: If you don’t like them, don’t use them. If you use them, just accept that increasing the density of functionality unavoidably impacts readability, can anyone say regex? :wink:

Me too, and I’d argue that dense code like with stanzas and regex need a bit more time.

Ah, so you really don’t have a horse in this race :slight_smile: I really like the formatter and I love the idea of an un-configurable formatter but that means things like this pop up once in a while.

Don’t get me started on that! I’ve been on this soapbox before but I firmly believe Elixir’s few constructs are all useful and you should just use the one that makes most sense for your situation, not look to jam every problem into your favourite one.

1 Like

Yeah, in general I think spending time on style is a net loss to a team. But I’m reconsidering in this thread that perhaps that’s a misinformed bias regarding people’s capability to improve their code reading skills.

This is my first time meeting a decent programmer who hasn’t switched teams after trying the no-syntax-highlighting for (usually much less than) 2 months. Now, mostly the people I’ve tried this with have been my own hires, so there’s an obvious bias at play there.

1 Like

Do you require that your team member not use syntax highlighting?? Do you guys do a lot of pair programming? (Sorry this is getting off topic). People actually leave your team?? I actually pretty much left a team once because they wanted the whole team to standardize on mouse, keyboard, and IDE! It made sense because we paired 100% of the time but then I moved to another team who also paired 100% but didn’t have such rules and it worked really well.

I think style is important but again I agree should not be discussed at any length at work. A decision should be made a quickly as possible. My old colleague used to say that the low-stakes decisions the decisions to make as soon as they arise and as quickly as possible (or, you know, something along those lines).

Heh, no, but I’ve suggested it as a learning exercise, and people have chosen to try it. I suggest a lot of things for growth, learning assembler, building electronics kits,
learning new (high level) languages, contributing to OSS projects (ideally in a new language)

I’m not personally a fan of it, so I don’t tend to participate, but I’ve had people who did like it - I let people use whatever process best suits them. Including test coverage and peer review btw, if a developer wants to they can push straight to production without tests… but they are held responsible for the outcome. Unsurprisingly, almost universally people write tests and seek out peer reviews. I’m an “we’re all professionals here” / “remove the bumpers” kind of guy.

Not much, no.

Heh, yeah, I once took an on-site 6 month contract with a company that refused to allow me to install Linux on my desktop (it was Windows), so I just brought in my own hard drive and ran Linux. I didn’t renew after 6 months although it was a great team there was too much friction.

Yeah, I don’t. In fact I literally have “Style is unimportant.” as one of my team principles. I’m not even going to try to argue that a consistent style doesn’t help readability, I don’t personally find that it does, but I’m willing to accept that some people find it easier if all the code looks the same. I just don’t think it’s a significant impact vs say naming, or PIE, or many other important architectural/design principles, like not even in the same ballpark.

“Unimportant” sums it up for me, it’s not of zero value, it’s just not important, it’s trivial. To me it’s as unimportant as what people name their comprehension function params. Even if there’s a universally accepted correct style for some language feature, I think bothering about it is like people in a debate commenting on some grammatical mistake, or people online commenting about a spelling error. The meaning is what’s important, and if the meaning was conveyed accurately, who cares if you had to mentally hurdle their mistaken “their” vs “they’re”, or wrong “it’s” :slight_smile: That’s how I see coding style comments/debates/tooling/changes.
</rant> :slight_smile:

I would personally rather remove as many hurdles as possible, no matter how small. There are so many little ones in programming that the further from Death by a Thousand Cuts I can get, I’ll take. I agree there are far more important things to worry about and that’s why I like the formatter. The highest performing team I was on just picked a style and ran with it. We never talked about it again after the initial decisions were made when we were forming. I always try and imagine myself looking at code I’ve never seen before or having to fix a bug in unfamiliar code. The least weirdness I have to deal with the better! I guess I’ve worked with people very “creative” styles in the past, though.

Anyway, I certainly hear you and think we understand each other and agree to disagree :sweat_smile: Both can work, of course, and software still gets made! I love pair programming and I love TDD and I wish that both were far more of the norm than they are. But of course, good software gets made where neither of those tools are used, but that change my feelings about them and will use them any chance I get.

Having said that, I do like this:

At an old company they had a very hard policy that there had to be a code review before going to production (even if it just said “LGTM!”) even if it was a hotfix because production was down. I still really don’t get that (other than having to adhere to the documented bureaucracy).

While we’re still going away here, I had one more thing about style. On that one team I was talking about, one of our rules was full variable names, no exceptions. This was Ruby and it even applied to single statement maps that couldn’t use proc2block style (these next examples would certainly use it but I’m simplifying them for the example):

work_orders.map do |work_order|
  work_order.ends_at
end

The next codebase I worked on was an Elixir one that had no such rules. So one day I’m reading code I’d never seen before and I’m flying through it at a pretty good pace. I pass by some code that looks like this:

Enum.map(users, fn user ->
  user.name
end

and then suddenly get tripped up at something that looks like this:

Enum.map(some_longer_name, fn ifwtw ->
  sln.something
end)

I talk about “code scanning” a lot and this was when it really hit home how consistency makes for really good scannability. A super consistent style allows me to verrrrrry quickly pick out shapes and patterns with hardly even a glance at the code and know exactly what’s going on. So when I see some_longer_name then sln I immediately stopped and was like, “Wait, something more is going on here!” Of course there wasn’t. Soooooo, TL;DR, that’s why I care about style and again, I don’t care what it is just so long as there is one and it only gets decided quickly then never talked about again.

Annnyway, that’s my novel on that! I’ve found myself with a lot of free time this week so I’ve been spending way too much time on this site the past couple of days.