Skip to content

Added keys_to_atoms option for display maps #28

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Added keys_to_atoms option for display maps #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
38c3435
Added keys_to_atoms option for display maps
ajardolino3 Mar 30, 2017
453c1a3
Fixed to work with maps that had keys as both atoms and strings
ajardolino3 Mar 30, 2017
018c281
Made suggested changes to pull request. Now supports keys_to_atoms u…
ajardolino3 Apr 11, 2017
8a70f91
Small revision to use ap_options
ajardolino3 Apr 11, 2017
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
31 changes: 24 additions & 7 deletions lib/apex/format.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,30 @@ defimpl Apex.Format, for: Map do
end

def format(data, options) do
Apex.Format.Seq.format(
Map.to_list(data),
options,
start_token: "\%{",
end_token: "}",
numbers: false) |> colorize(data, options)
end
ap_options = :apex
Copy link
Owner

Choose a reason for hiding this comment

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

I would probably not do the config reading here. Doing it here would only apply it for the Map protocol implementation.

A better position would be in ap before the protocol invocation here.

Then I would also probably extract it into defp configuration(options).

|> Application.get_all_env
|> Keyword.merge(options)

data
|> keys_to_atoms(ap_options[:keys_to_atoms])
|> Map.to_list
|> Apex.Format.Seq.format(
ap_options,
start_token: "\%{",
end_token: "}",
numbers: false)
|> colorize(data, ap_options)
end

defp keys_to_atoms(data, shouldConvert)
Copy link
Owner

Choose a reason for hiding this comment

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

also only a small nitpick. Please use "snake_case" for parameter names, so should_convert here.

defp keys_to_atoms(data, true) do
Map.new(data, fn {k,v} -> { convert_to_atom(k), v} end)
Copy link
Owner

Choose a reason for hiding this comment

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

There's inconsistent whitespace usage here before convert_to_atom:

fn {k,v} -> { convert_to_atom(k), v} end

Please remove the whitespace for the sake of consistency

end
defp keys_to_atoms(data, _), do: data

defp convert_to_atom(key)
Copy link
Owner

Choose a reason for hiding this comment

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

👍

defp convert_to_atom(key) when is_binary(key), do: String.to_atom(key)
defp convert_to_atom(key), do: key
end

defimpl Apex.Format, for: Any do
Expand Down
99 changes: 99 additions & 0 deletions test/format_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,4 +172,103 @@ defmodule Apex.Format.Test do
]
"""
end

test "Can format maps and change keys to atoms" do
data = %{"foo" => "bar", "bar" => "baz",
"mappings" => [
%{"foo1" => "bar1", "bar1" => "baz1" },
%{"foo2" => "bar2", "bar2" => "baz2" }
]
}
assert format(data, keys_to_atoms: true, color: false) == """
%{
bar: "baz"
foo: "bar"
mappings: [
[0] %{
bar1: "baz1"
foo1: "bar1"
}
[1] %{
bar2: "baz2"
foo2: "bar2"
}
]
}
"""
end

test "Can format maps and change keys to atoms (mixed)" do
data = %{
foo: "bar",
bar: "baz",
mappings: [
%{ "foo1" => "bar1", "bar1" => "baz1" },
%{ "foo2" => "bar2", "bar2" => "baz2" }
]
}
assert format(data, keys_to_atoms: true, color: false) == """
%{
bar: "baz"
foo: "bar"
mappings: [
[0] %{
bar1: "baz1"
foo1: "bar1"
}
[1] %{
bar2: "baz2"
foo2: "bar2"
}
]
}
"""
end

test "Can format maps and leave keys as is" do
data = %{"foo" => "bar", "bar" => "baz",
"mappings" => [
%{"foo1" => "bar1", "bar1" => "baz1" },
%{"foo2" => "bar2", "bar2" => "baz2" }
]
}
assert format(data, keys_to_atoms: false, color: false) == """
%{
{
[0] "bar"
[1] "baz"
}
{
[0] "foo"
[1] "bar"
}
{
[0] "mappings"
[1] [
[0] %{
{
[0] "bar1"
[1] "baz1"
}
{
[0] "foo1"
[1] "bar1"
}
}
[1] %{
{
[0] "bar2"
[1] "baz2"
}
{
[0] "foo2"
[1] "bar2"
}
}
]
}
}
"""
end

end