-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Code.get_docs/2: check for valid kinds in public function + set :all as default value for kind #3746
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
Conversation
defp do_lookup_docs(docs, :all), do: docs | ||
defp do_lookup_docs(docs, kind) when kind in @doc_sections, | ||
defp do_lookup_docs(docs, kind) when kind in @doc_kinds, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If kind in @doc_kinds
moves to get_docs
there is no need to have it here, but you need to add it to the second get_docs
definition as well. I think it probably doesn't worth it to move it then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooops....
fixed! thanks for your comments (as usual),
I have created a function head, and removed the check in the private functions.
Well, we do need to update it, because this is the full error I was getting: https://gist.github.com/eksperimental/399036a2ccb27fe198e5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, now it'll look better.
29ea1b6
to
cb1bc00
Compare
@doc_kinds [:docs, :moduledoc, :callback_docs, :type_docs] | ||
def get_docs(module, kind \\ :all) | ||
|
||
def get_docs(module, kind) when is_atom(module) and kind in @doc_kinds or kind == :all do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think parenthesis around kind
checks will improve readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add :all
to @doc_kinds
and skip the extra check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point. now that we removed the checked in the private function we can do it.
I'm afraid that default value for |
cb1bc00
to
72d86e1
Compare
Exactly, let's please remove the default value. |
yes. that was a really good point @lexmag |
…private This was giving me a long cryptic message because we were checking for a valid kind way to far. I have renamed the attribute to @doc_kinds instead of @doc_sections Corrected kinds for :all option Mention that no valid module returns nil, and created a doctest example.
72d86e1
to
c3b9344
Compare
done. I don't see the travis build status for this |
Code.get_docs/2: check for valid kinds in public function
I was getting a really long cyptic message showing the whole list of docs, but for a private function,
It also add :all as the default value for kind