Open MarkoMin opened 7 months ago
I've been thinking about this for the last week and I think your suggestion is better than what we have now. I'll look into it and see if we can change it.
Regarding the exported
key, it is only useful for types as those the only thing that can be included in the doc chunk that is not exported. So for functions and callbacks it would always be true
.
Regarding the
exported
key, it is only useful for types as those the only thing that can be included in the doc chunk that is not exported. So for functions and callbacks it would always betrue
.
But doesn't that interfere with hidden
property of the documentation? My current reasoning is the following:
That said, I'd also promote hidden
to be part of the meta map, containing boolean value. For everything exported, default would be true
and for everything else, default would be false
. It's very simple to reason about and very simple to override for custom behavior (given that hidden is a key in a meta map).
That said, instead of:
-doc hidden.
we'd have:
-doc #{hidden => true}.
Eventually, if you want to include undocumented and not-exported type you could do it easily:
-doc #{hidden => false}.
-type not_exported() :: any().
- Exported and documented type - include in the docs
- Exported and not-documented type - warn_missing_docs I guess, include definition in the docs
- Not-exported and documented type - use hidden property
- Not-exported and not-documented type - don't include in the docs
It should be:
none
in the EEP-48 docshidden
- warn_missing_docs will not trigger, include as hidden
in the EEP-48 docsThe rules are the same for functions and callbacks, except that the "or referenced" logic is not applied.
In EEP-48, which is what EEP-59 targets, the "Doc" field in the tuple for each entry can be #{ Lang => Doc } | hidden | none
. This is why hidden is not part of the map, as the map is translated into metadata. I also just realized that this is the reason why I did not put the slogan in the metadata map, as the slogan is not part of the metadata map either.
Exported or referenced type that is documented - include in the EEP-48 docs
Exported or referenced type that is not documented - warn_missing_docs will trigger, include as
none
in the EEP-48 docsExported or referenced type that is
hidden
- warn_missing_docs will not trigger, include ashidden
in the EEP-48 docsNot-exported nor referenced type that is documented - do not include in the EEP-48 docs
Not-exported nor referenced type that is not-documented - don't include in the EEP-48 docs
Makes sense and I'm not sure where exported
is required? If it's something internal to the doc compiler, shouldn't that stay internal in the sense that it's not documented and does not interfere with user specified keys?
In EEP-48, which is what EEP-59 targets, the "Doc" field in the tuple for each entry can be
#{ Lang => Doc } | hidden | none
. This is why hidden is not part of the map, as the map is translated into metadata. I also just realized that this is the reason why I did not put the slogan in the metadata map, as the slogan is not part of the metadata map either.
If i got it correctly "Signature" from EEP-48 is what "Slogan" is in EEP-59, right? Should we choose one of those names (given that semantics are equal ofc)? It's not part of the metadata either, but if it can be extracted from doc string, I don't see a reason why it couldn't be extracted from the meta map. Moving "Signature" to the map would now be backward incompatible because of the tuple, but we could move it to the map in EEP-59 and leave internal representation distinct from that. There already are keys that exist in EEP-48 metamap and not in EEP-59 anyways (and vice versa).
Makes sense and I'm not sure where exported is required? If it's something internal to the doc compiler, shouldn't that stay internal in the sense that it's not documented and does not interfere with user specified keys?
exported
is added as a way for tools that work with the EEP-48 chunks (i.e. shell completion, ErlangLS, ExDoc etc) to easily see whether a type is exported or not. Without it each tool would have to parse all specs, callbacks and types (just like we do in the compiler) to figure out this information.
If i got it correctly "Signature" from EEP-48 is what "Slogan" is in EEP-59, right?
It should indeed be "Signature". Thanks for noticing, not sure how I missed that.
There already are keys that exist in EEP-48 metamap and not in EEP-59 anyways (and vice versa).
Both EEP-48 and EEP-59 allow for any keys you want to be added to the metadata. EEP-48 defines the keys that are used across BEAM languages while EEP-59 defines keys used only in Erlang (or rather it should, the current EEP-59 does not define any keys). I've chosen to not list all the keys of EEP-48 in the Erlang Reference Manual, though maybe we should do that for clarity.
One of the problems that I try to keep in mind when looking at the signature things is that in the future (maybe soon, or later, or maybe never) we want to be able to document function clauses and then using the signature to figure out which clause documentation belongs to could be one way. So I don't want to paint the implementation into a corner that I later would like to get out of.
exported
is added as a way for tools that work with the EEP-48 chunks (i.e. shell completion, ErlangLS, ExDoc etc) to easily see whether a type is exported or not. Without it each tool would have to parse all specs, callbacks and types (just like we do in the compiler) to figure out this information.
So it should never be manually set by the user, but is documented just so one does not set it by mistake (to use it in for a different task)?
One of the problems that I try to keep in mind when looking at the signature things is that in the future (maybe soon, or later, or maybe never) we want to be able to document function clauses and then using the signature to figure out which clause documentation belongs to could be one way. So I don't want to paint the implementation into a corner that I later would like to get out of.
Aha, you mean something like:
-doc """
add(X,X) -> Y
First clause doc.
add(X,Y) -> Z
2nd clause doc.
""".
? But again, how could MD parser conclude that "add(X,Y) -> Z" is a signature, rather than custom documentation line? Also, that would mean if you want to provide a signature for one clause only, you'd need to write custom signatures for all clauses.
If going that way, I'd consider adding -clausedoc
attribute or consider enabling multiple -doc string()
, but clause documentation is indeed a whole new discussion.
Would it make sense to introduce EEP-59 implementation as an experimental feature? Because we may introduce something in OTP27 and conclude that it's a deal-breaker for something better in OTP28. The reason is that (from my point of view) something "experimental" have a higher chance of getting backward incompatible changes than something that is just introduced.
So it should never be manually set by the user, but is documented just so one does not set it by mistake (to use it in for a different task)?
yes. The documentation states "For any -type attribute this value is automatically set by the compiler and should not be set by the user."
If going that way, I'd consider adding -clausedoc attribute or consider enabling multiple -doc string(), but clause documentation is indeed a whole new discussion.
It is indeed, but it ties into the signature discussion.
Would it make sense to introduce EEP-59 implementation as an experimental feature?
Maybe. We have until May to figure out if we want that or not.
yes. The documentation states "For any -type attribute this value is automatically set by the compiler and should not be set by the user."
Okay, but can we change the docs to "This value is automatically set by the compiler and should not be set by the user"? Because, from the current docs, it's not clear that to do with this attribute for functions and callbacks, it only says that it's automatic for types and that is what totally confused me. I'll make a PR if you agree.
I'll make a PR if you agree.
I agree.
One thing came to my mind while reading the documentation:
Seems like a "magical" behavior to me. Couldn't
slogan
be a reserved key in a function metadata? Seems like a natural fit. E.g.:EDIT:
or even better, use the same semantics as
equiv
:EDIT 2: Maybe off topic, but I noticed that
exported
is a reserved key. Here it says that "For any -type attribute this value is automatically set by the compiler and should not be set by the user." - why isn't the same for functions? Is there a case where compiler can't figure out whether function/type is or isn't exported?