I rather quite prefer how the formatter is doing that. I always like the starting token to be at the end of it’s line, each element to be on their own, and the closing token to be on it’s own line at the same indentation as the indentation of the first line, just as it is doing. ^.^
I do, however, still wish for trailing comma’s… ^.^;
It’s probably my time with Clojure showing- collapsed delimiters saves a lot of space and reduces noise when all you’re doing is writing nested data. It’s not as good for reorganizing, though.
Yeah I do end-line closing tags in lisp as well, but that is because the ‘code’ is that way, for formatting data it is often good to see the bounds. ^.^
Yeah, we only respect the user’s choice for data structures. We could also do it for function calls but function calls usually give us more liberty to handle those issues, for example by breaking into variables. And there is also the issue with function calls with no parens which have no way of respecting the user’s choice because there are no delimiters…
Not really. The times I often use the multi-line function call syntax like that is when it is a macro and the AST is important. Look at ExSpirit for example (hmm, I need to test how elixir would format some grammars…). ^.^
EDIT1: Not very well, the grammar’s got a lot more right-heavy and the trailing comma’s all vanished, which is really important in grammars considering how much they are shuffled around while testing parsing… >.<
EDIT2: Also what on earth is it doing making this format?! o.O
pipe_result_into: List.to_tuple()
|> case do
{:started, datetime_string} ->
{:started, parse(datetime_string, parse_weirddatetimeformat()).result}
result ->
result
end
EDIT3: It turned this:
defrule parse_entry(seq([
parse_type(),
parse_body(),
])), pipe_result_into: List.to_tuple() |> (case do
{:started, datetime_string} -> {:started, parse(datetime_string, parse_weirddatetimeformat()).result}
result -> result
end)
Into this:
defrule(
parse_entry(
seq([
parse_type(),
parse_body()
])
),
pipe_result_into: List.to_tuple()
|> case do
{:started, datetime_string} ->
{:started, parse(datetime_string, parse_weirddatetimeformat()).result}
result ->
result
end
)
First of all, the function definition is all wrapped up in parenthesis, which is just wrong (that is like formatting def blah(x) to be def(blah(x))), and it formatted the pipe_result_into: option with the pipe into a ‘what the heck is this’, as well as a few other oddities (more minor ones like the seq call not being on the primary line and such).
The thing that bugs me most is how it is doing that piping in the pipe_result_into option, it makes me confused as to if it is piping the output of the List.to_tuple() call or if it is piping the output of the parse_entry call. I’m still having issues mentally parsing that even though I know it is fairly obvious… >.<
Part of why it was so striking to me was a couple lines later another multi-line compare call with literally 2 more characters was not collapsed, and so now I have two visual representations of something that should stand out visually as (no pun intended) comparable.
This is how the formatter leaves this.
def eval(%Expr.Comparative{op: :!=} = expr, node, document, context) do
v1 = Expr.eval(expr.e1, node, document, context)
v2 = Expr.eval(expr.e2, node, document, context)
compare(expr.op, Expr.Helpers.eq_fmt(v1, v2, document), Expr.Helpers.eq_fmt(v2, v1, document))
end
def eval(expr, node, document, context) do
v1 = Expr.eval(expr.e1, node, document, context)
v2 = Expr.eval(expr.e2, node, document, context)
compare(
expr.op,
Expr.Helpers.cmp_fmt(v1, v2, document),
Expr.Helpers.cmp_fmt(v2, v1, document)
)
end
I guess one could argue there is a difference between the two versions of the function that it now better highlighted, but it seems off to me.
That’s true, but in this case I purposefully chose to use a multi-line function call rather than adding two more variables because I felt it produced the clearer code.
There is an option for declaring locals_without_parens, it is in the docs.
For the keyword, we need to do something but I am not sure what. Probably treat it in the same way we would treat =. In this case it would mean breaking into the next line and indenting with two spaces. Please open up an issue.
Good point, please open up an issue too!
I could define that in ex_spirit sure, but how would the users use it without each-and-every-single-one-of-them adding it to their options as well? It should be some in-file definition, like a module attribute or something to state ‘all uses of this is without parenthesis’ or so.
That would work but is more wordy in this case (more so in other cases as they tend to be really short, this example is longer than most).
The above formatted to this without the parenthesis:
defrule parse_entry(
seq([
parse_type(),
parse_body()
])
),
pipe_result_into: List.to_tuple()
|> (case do
{:started, datetime_string} ->
{:started, parse(datetime_string, parse_weirddatetimeformat()).result}
result ->
result
end)
However, why is it indenting it *so*far*to*the*right*? o.O?!
That action is making some of the deeper grammars very unreadable (especially as it is making the lines longer due to some other changes as well). Inside a (/) should be indentation, not alignment (absolutely should not be alignment).
So I can either get parenthesis around the rule function definition, or I get everything pushed waaaaaay over to the right. Neither of these options are right, not right at all… O.o!
I’ve been playing with ExSpirit a lot. It’s a great library but it’s fundamentally weird. It does unconventional things to the AST. It’s a great DSL but it doesn’t mesh exactly well with the wider Elixir code. I dont’ think any effort should be expended in improving the output of the formatter on ExSpirit.
EDIT: ExSpirit is great for reasons other than the DSL, I don’t want to imply that the DSL is the only (or even major) reason you should pick ExSpirit!
Most of the time I actually put the seq in its own line, but as I’ve said above, I don’t think normal formatting rules apply to ExSpirit. Maybe ExSpirit need its own style guide? xD I don’t think you should ever expect good results out of running the formatter on ExSpirit.
I agree, this is weird. But again, see my point above.
In the Javascript ecosystem (where I spend my professional life) we have a formatter called prettier. It’s used in quite a few popular codebases, albeit completely opinionated except for a few tiny options:
{thing: true} vs { thing: true }
We make do with these options (I just converted several repos to prettier at my work) and no one really complains because we auto-format inside our CI pipelines.
That’s what the discussion with @NobbZ above has been about. It can’t be anything set programmatically because the formatter does not evaluate code, it has no semantic understanding. And I would also be wary if we allowed libraries to implicitly tell how my code should be formatted, because they would be able to change how unrelated code to the library would be formatted too.
Parens are indented with two spaces since it has delimeters. Function calls without parens are indented by their name, that’s how most Elixir code is organized:
assert foo ==
bar
If it wrote it as:
assert foo ==
bar
Then some people would complain about the exact opposite that you are complaining. 
The last two bugs reported have been fixed as well. Please give the formatter a try once more and let us know of more suspicious cases!
Actually I’d vote for:
assert(
foo == bar
)
If it managed to get longer that is. ^.^
I did, I saw the changes! I’ll grab master and try it again.
It now formats it to:
defrule parse_entry(
seq([
parse_type(),
parse_body()
])
),
pipe_result_into:
List.to_tuple()
|> (case do
{:started, datetime_string} ->
{:started, parse(datetime_string, parse_weirddatetimeformat()).result}
result ->
result
end)
Looks much more readable like that. 
Still far far too right (I’ve never seen the point to aligning to something above like that, only for repeated calls or so, which this is not the case, thus only indentation should be used, not alignment), and that random newline in the case is jarring, it either needs to be gone, or perhaps made into this (which I do in my especially large case statements, of which this is not one so I would not have empty lines at all):
defrule parse_entry(
seq([
parse_type(),
parse_body()
])
),
pipe_result_into:
List.to_tuple()
|> (case do
{:started, datetime_string} ->
{:started, parse(datetime_string, parse_weirddatetimeformat()).result}
result ->
result
end)
EDIT:
@josevalim https://github.com/elixir-lang/elixir/pull/6979 just went through, it formats:
alias Foo.{
Bar
}
As this instead now:
alias Foo.{
Bar
}
Isn’t that the same thing that I want from this:
defrule parse_entry(
seq([
parse_type(),
parse_body()
])
)
Wanting it to be this:
defrule parse_entry(
seq([
parse_type(),
parse_body()
])
)
Seems the same kind of issue to me? And as such that PR becomes inconsistent considering the above case is not done too?
(Although I’d still put the trailing comma too as without it looks inconsistent and wrong).
Prettier has saved me so much time in Javascript land, this makes me so excited for Elixir. It’s going to save brainpower, avoid hour long Slack discussions on “best practices” and pointless Wiki pages in source repositories.
That rule currently only applies to data-structures and if they are the last argument. Even if we generalized to function calls, it means it wouldn’t apply when you use :pipe_result_into, since the last argument is now the :pipe_result_into. Then you could say: “why not apply it to all arguments?”. The issue is that then you would get this:
defrule parse_entry(
seq([
parse_type(),
parse_body()
])
),
pipe_result_into:
List.to_tuple()
|> (case do
{:started, datetime_string} ->
{:started, parse(datetime_string, parse_weirddatetimeformat()).result}
result ->
result
end)
And that looks very confusing. And no, we can’t keep pipe_result_into in the previous line because once we break you get one argument per line.
Actually what I’d prefer is this:
defrule parse_entry(
seq([
parse_type(),
parse_body()
])
), pipe_result_into:
List.to_tuple()
|> (case do
{:started, datetime_string} ->
{:started, parse(datetime_string, parse_weirddatetimeformat()).result}
result ->
result
end)
Specifically because they are just arguments to defrule. ^.^
Do note, defrule can have a do/end body as well, that body handles direct context manipulation. Which further makes it… hmm, interesting. ^.^;
Ah hah! I just discovered why the indentation in the (/) and such look wrong to me in this formatter, it doesn’t match what the C++ formatting I use does (I’ve been doing a bit of C++ work recently)!
Specifically, in C++ if I have a function call where the arguments span over multiple lines it depends on if the first argument is on the same line as the ( or not, specifically (in elixir’eze):
# With the first argument *not* on the same line as the `(`:
bloop blah(
arg0, # Normal 'indentation level', which should always be a <tab>, but elixir is weird with spaces (which is not indentation!)
arg1,
arg2 # I'd love a trailing comma here, C++ does not support it either though, blah...
)
# With the first argument *on* the same line as the `(`:
bloop blah(arg0,
arg1, # indentation to the prior line (spaces in elixir), then spaces for alignment! ^.^
arg2
) # Sometimes I see this one character left so it is aligned with `(`, I don't really care either way though
And that is indeed what I was expecting before but was not getting it and did not know how to verbalize it at the time.
I just noticed this one.
I have a case where the name of something is represented by an atom, and one of the names is “true”, and so represented by the atom :true (I didn’t choose that name, it’s part of a specification).
You probably see where this is going, but:
def eval(%Expr.Function{f: :true, args: []}, _node, _document, _context) do
is formatted as
def eval(%Expr.Function{f: true, args: []}, _node, _document, _context) do
:true == true, but I’m trying to represent something called “true” as an atom, not use the boolean true.
© Copyright Elixir Forum | Terms | Privacy & Cookies
