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

EEP-59 slogans #8030

Open
MarkoMin opened this issue Jan 18, 2024 · 9 comments
Open

EEP-59 slogans #8030

MarkoMin opened this issue Jan 18, 2024 · 9 comments
Assignees
Labels
enhancement team:VM Assigned to OTP team VM

Comments

@MarkoMin
Copy link
Contributor

MarkoMin commented Jan 18, 2024

One thing came to my mind while reading the documentation:

It is possible to supply a custom slogan by placing it as the first line of the -doc attribute. The provided slogan must be in the form of a function declaration up until the ->. For example:
-doc """
add(One, Two)
Adds two numbers.
""".
add(A, B) -> A + B.
Will create the slogan add(One, Two). The slogan will be removed from the documentation string, so in the example above only the text "Adds two numbers" will be part of the documentation. This works for functions, types, and callbacks.

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.:

-doc #{slogan => "add(One, Two)"}.
add(X,Y) -> ...

EDIT:

or even better, use the same semantics as equiv:

-doc #{slogan => add(One, Two)}.
add(X,Y) -> ...

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?

@IngelaAndin IngelaAndin added the team:VM Assigned to OTP team VM label Jan 22, 2024
@garazdawi
Copy link
Contributor

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.

@MarkoMin
Copy link
Contributor Author

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.

But doesn't that interfere with hidden property of the documentation? My current reasoning is the following:

  1. Exported and documented type - include in the docs
  2. Exported and not-documented type - warn_missing_docs I guess, include definition in the docs
  3. Not-exported and documented type - use hidden property
  4. Not-exported and not-documented type - don't include in the docs

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().

@garazdawi
Copy link
Contributor

  • 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:

  • 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 docs
  • Exported or referenced type that is hidden - warn_missing_docs will not trigger, include as hidden in the EEP-48 docs
  • Not-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

The 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.

@MarkoMin
Copy link
Contributor Author

  • 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 docs

  • Exported or referenced type that is hidden - warn_missing_docs will not trigger, include as hidden in the EEP-48 docs

  • Not-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).

@garazdawi
Copy link
Contributor

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.

@MarkoMin
Copy link
Contributor Author

MarkoMin commented Jan 26, 2024

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.

@garazdawi
Copy link
Contributor

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.

@MarkoMin
Copy link
Contributor Author

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.

@garazdawi
Copy link
Contributor

I'll make a PR if you agree.

I agree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement team:VM Assigned to OTP team VM
Projects
None yet
Development

No branches or pull requests

3 participants