Skip to content
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.

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

Now offers a summary of the beam file and not the full thing #2

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

Conversation

gordonguthrie
Copy link

See changes to the README for details

Gordon Guthrie added 2 commits December 29, 2017 11:57
@aerosol
Copy link
Owner

aerosol commented Jan 24, 2018

Hi @gordonguthrie this is cool, please bare with me, I'll get to it shortly

Copy link
Owner

@aerosol aerosol left a comment

Choose a reason for hiding this comment

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

A few suggestions inline if you don't mind, mainly just style

[]
end
defp format(behaviours, prefix) do
[h | t] = behaviours
Copy link
Owner

Choose a reason for hiding this comment

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

Match in the function clause instead?

This can be called as so:

```elixir
Decompilerl.summarise('_build/dev/lib/myapp/ebin/Elixir.MyApp.AuthController.beam')
Copy link
Owner

Choose a reason for hiding this comment

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

It's uncommon for elixir APIs to accept lists as "strings", since strings are binaries by default. What do you think?

@@ -73,14 +108,16 @@ $ ./decompilerl

Decompilerl

usage: decompierl <beam_file> [-o <erl_file> | --output=<erl_file>]
usage: decompierl <beam_file> [-o <erl_file> | --output=<erl_file> | -s | --summary]
Copy link
Owner

Choose a reason for hiding this comment

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

my bad, there's a typo there: decompierl

def decompile(module, device \\ :stdio) do
obtain_beam(module)
|> do_decompile
|> write_to(device)
end

defp format_summary(map) do
%{:file => file,
Copy link
Owner

Choose a reason for hiding this comment

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

IMO it'd be more idiomatic to match (preferably in the function clause) like this:

%{file: file, module: module, ...}

format(behaviours, "Behaviours : "),
format(exports, "Exported Fns : "),
get_private_functions(functions, exports)
|>format("Private Fns : ")
Copy link
Owner

Choose a reason for hiding this comment

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

💅 nitpick: |> format

lines = [
"File : " <> file,
"Module : " <> module,
format(behaviours, "Behaviours : "),
Copy link
Owner

Choose a reason for hiding this comment

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

Would you consider using String.pad_trailing/3 for all the formatting touch ups?

get_private_functions(functions, exports)
|>format("Private Fns : ")
]
Enum.join(List.flatten(lines), "\n")
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 no need to flatten, iolists are OK to write

append_value(map, :behaviours, [Atom.to_string(behaviour)])
end

defp process_ast({:function, _, func, arity, _body}, map) do
Copy link
Owner

Choose a reason for hiding this comment

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

Perhaps glue the function clauses together, i.e. no white space in between?

end

defp make_fn_declaration(func, arity) do
Atom.to_string(func) <> "/" <> Integer.to_string(arity)
Copy link
Owner

Choose a reason for hiding this comment

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

How about:

"#{func}/#{arity}"


defp append_value(map, key, valuelist) when is_list(valuelist) do
%{^key => values} = map
Map.put(map, key, values ++ valuelist)
Copy link
Owner

Choose a reason for hiding this comment

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

You could avoid matching and pinning the key with:

Map.update(map, key, xs, &(xs ++ &1))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants