Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 23 additions & 4 deletions lib/ex_format.ex
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ defmodule ExFormat do
state = %{
parenless_calls: MapSet.new(@parenless_calls),
parenless_zero_arity?: false,
in_list?: false,
in_spec: nil,
}
for {line, i} <- Enum.with_index(lines) do
Expand Down Expand Up @@ -428,7 +429,14 @@ defmodule ExFormat do

# Tuple containers
def to_string({:{}, _, args} = ast, fun, state) do
tuple = "{" <> tuple_to_string(args, fun, state) <> "}"
tuple =
# Enforce keyword syntax for tuples of size 2 inside list
if state.in_list? and length(args) == 2 do
[k, v] = args
kw_list_to_string([{k, v}], fun, state)
else
"{" <> tuple_to_string(args, fun, state) <> "}"
end
fun.(ast, tuple)
end

Expand Down Expand Up @@ -972,6 +980,7 @@ defmodule ExFormat do
end

defp list_to_string(list, fun, state) do
state = %{state | in_list?: true}
list_string = Enum.map_join(list, ", ", &to_string(&1, fun, state))
if not fits?(" " <> list_string <> " ") or line_breaks?(list) do
list_to_multiline_string(list, fun, state)
Expand All @@ -988,12 +997,22 @@ defmodule ExFormat do
"\n " <> list_string <> ",\n"
end

defp kw_list_to_string(list, fun, state) do
list_string = Enum.map_join(list, ", ", fn {key, value} ->
atom_name = case Inspect.Atom.inspect(key) do
defp kw_list_to_string([{key, value}], fun, state) when not is_atom(key) do
atom_name =
case to_string(key, fun, state) do
":" <> rest -> rest
other -> other
end
atom_name <> ": " <> to_string(value, fn(_ast, string) -> string end, state)
end

defp kw_list_to_string(list, fun, state) do
list_string = Enum.map_join(list, ", ", fn {key, value} ->
atom_name =
case Inspect.Atom.inspect(key) do
":" <> rest -> rest
other -> other
end
atom_name <> ": " <> to_string(value, fn(_ast, string) -> string end, state)
end)
if not fits?(" " <> list_string <> " ") or line_breaks?(list) do
Expand Down
7 changes: 3 additions & 4 deletions test/unit/indentation_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,9 @@ defmodule ExFormat.Unit.IndentationTest do
[{:prefix_newline, get_prefix_newline(curr_lineno-1, prev_lineno)}] ++ curr_ctx
"""
good = """
[{:prev, prev_lineno}] ++
[{:prefix_comments, get_prefix_comments(curr_lineno - 1, prev_lineno)}] ++
[{:prefix_newline, get_prefix_newline(curr_lineno - 1, prev_lineno)}] ++
curr_ctx
[prev: prev_lineno] ++
[prefix_comments: get_prefix_comments(curr_lineno - 1, prev_lineno)] ++
[prefix_newline: get_prefix_newline(curr_lineno - 1, prev_lineno)] ++ curr_ctx
"""
assert_format_string(bad, good)
end
Expand Down
16 changes: 16 additions & 0 deletions test/unit/kw_list_syntax_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,20 @@ defmodule ExFormat.Unit.KwListSyntaxTest do
"""
end
end

describe "keyword syntax for tuples inside list" do
test "maintain keyword sugar for tuples of size 2" do
assert_format_string("[:foo, bar: :baz]\n")
assert_format_string("[bar: :baz]\n")
end

test "enforce keyword sugar for tuples of size 2" do
assert_format_string("[{:bar, :baz}]", "[bar: :baz]\n")
assert_format_string("[:foo, {:bar, :baz}]", "[:foo, bar: :baz]\n")
end

test "do not enforce keyword sugar for tuples of size more than 2" do
assert_format_string("[{:foo, :bar, :baz}]\n")
end
end
end
2 changes: 1 addition & 1 deletion test/unit/kw_list_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ defmodule ExFormat.Unit.KeywordListTest do
package: package(),
aliases: aliases(),
test_coverage: [tool: ExCoveralls],
preferred_cli_env: [{:"test.local", :test}, {:coveralls, :test}, {:"coveralls.travis", :test}],
preferred_cli_env: ["test.local": :test, coveralls: :test, "coveralls.travis": :test],
name: "Mssqlex",
source_url: "https://github.com/findmypast-oss/mssqlex",
docs: [main: "readme", extras: ["README.md"]],
Expand Down
8 changes: 4 additions & 4 deletions test/unit/list_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,13 @@ defmodule ExFormat.Unit.ListTest do
defp deps() do
[
# Web server
{:cowboy, "~> 1.0"},
cowboy: "~> 1.0",
# Web framework
{:phoenix, "~> 1.3.0-rc"},
phoenix: "~> 1.3.0-rc",
# XML parser helper
{:sweet_xml, "~> 0.6"},
sweet_xml: "~> 0.6",
# Statsd metrics sink client
{:statix, "~> 1.0"},
statix: "~> 1.0",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh boy this is an interesting one. We always write this as tuples for some reason :| Should we start writing them like this, which sucks because only works if you don't have options? I'm not sure how to proceed. \cc @josevalim

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, what could break if we start formatting mix.exs like this? 🤔

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think anything would break, it'd just be annoying to have to rewrite everything using tuples when you wanted to add options.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, thanks @lpil. I feel that this is a minor compromise since you are getting a cleaner syntax in return. Safe to merge? @whatyouhide @josevalim

]
end
"""
Expand Down