Show docs for callback with multiple clauses #8553
Conversation
| @@ -211,6 +215,8 @@ defmodule Kernel.DocsTest do | |||
| callback_bar, | |||
| callback_baz, | |||
| callback_foo, | |||
| callback_qux, | |||
| callback_qux, | |||
There was a problem hiding this comment.
this is still not right, we should get the callback only once in the chunk. I couldn't figure out how to deduplicate it so assistance would be appreciated!
There was a problem hiding this comment.
According to my understand of the code this should be correct. So maybe we have something in elixir_erl.erl that is duplicating it again?
There was a problem hiding this comment.
@josevalim Good call. I noticed the issue in elixir_erl and fixed it, but I couldn't come up with a nicer solution yet. See: a0869d3. Will keep at it.
wojtekmach
force-pushed
the
wm-multiple-clauses
branch
from
e2650dc to
a0869d3
Compare
| {line, doc} = get_doc_info(set, :doc, line) | ||
| store_doc(set, kind, name, arity, line, :doc, doc, %{}) | ||
| # store doc only once in case callback has multiple clauses | ||
| unless :ets.member(set, {kind, name, arity}) do |
There was a problem hiding this comment.
Just checking: is this still necessary with the new fix?
lib/elixir/src/elixir_erl.erl
Outdated
| @@ -318,7 +318,23 @@ typespecs_form(Map, TranslatedTypespecs, MacroNames) -> | |||
| Forms1 = types_form(Types, Forms0), | |||
| Forms2 = callspecs_form(spec, Specs, [], MacroNames, Forms1, Map), | |||
| Forms3 = callspecs_form(callback, AllCallbacks, OptionalCallbacks, MacroCallbackNames, Forms2, Map), | |||
| {Types, AllCallbacks, Forms3}. | |||
| AllCallbacks2 = typespecs_back_to_callbacks(Forms3), | |||
There was a problem hiding this comment.
Are we doing this because of duplicates in AllCallbacks? If so, maybe it would be better to call lists:uniq or lists:usort in callbacks?
There was a problem hiding this comment.
Good call, I'm now stripping spec information from callbacks and usorting them, check out latest commit.
lib/elixir/src/elixir_erl.erl
Outdated
|
|
||
| {Kind, FA, Line, Rest}. | ||
| AllCallbacksWithoutSpecs = [ | ||
| {Kind, NameArity, Line} || {Kind, NameArity, Line, _Spec} <- AllCallbacks |
There was a problem hiding this comment.
We don't seem to use the Line later on? So maybe we should return {Kind, Name, Arity} and usort on that.
|
Thank you! ❤️ |
No description provided.