Replace Specs heading #1455
Replace Specs heading #1455
Conversation
|
Types should be rendered with |
|
I prefer the one with the padding. It makes it stand out from the rest of the docs that follow. |
|
No padding unless we are adding border around it, to make consistent elsewhere. |
Note that the current specs are padded and have no border around them. |
|
I'm fine with either way. |
|
Sorry, I was not clear enough. The previous had the header, to give some context about it. If we pad it now, it will look like code, but not quite like code, and not enough context around it. Although I think making it look like code is worse? 🤔 |
|
@wojtekmach good point, thanks! Should opaque types be rendered with I only need to consider |
|
There are also |
Odaeus
force-pushed
the
main
branch
2 times, most recently
from
b66953c to
419fa87
Compare
|
I've added support for all the mentioned attribute strings (and Callbacks: |
|
Oh, we need to check how those look for Erlang. I think |
|
The contrast ratio on the attribute in night mode was too low (using Chromium's built-in analyser) so I've set it to a night mode gray and it's happy now. (Thank you to whoever set up these vars!) Before: After: I need a pointer on how to generate Erlang examples please. @josevalim 's suggestion of |
|
Sorry, you are right, you need to set |
|
besides what José said, if you're generating the docs with the escript, |
| if "opaque" in node.annotations do | ||
| "@opaque" | ||
| else | ||
| "@type" |
There was a problem hiding this comment.
@type and friends is Elixir specific which means it won't be appropriate for e.g. Erlang. We try to keep language-specific functionality in the corresponding ExDoc.Language.* module so let's see if we can make the change there.
|
Thanks for the config setting, I didn't realise it was working because the output looks identical... except with red highlights instead of blue! I will work on adjusting the output for Erlang support. |
|
I've added a new commit on top so you can see the extraction I made to support both languages as suggested. But I'm lost on how to demonstrate or test the Erlang side. I'm not sure what effect |
Yup. I just added a basic test here: https://github.com/elixir-lang/ex_doc/pull/1514/files. Once this lands, could you rebase your PR on top and adjust the test accordingly? Thanks! cc @josevalim |
|
btw, |
|
Thanks so much @wojtekmach! I approached trying to do the same thing a few times but kept getting stuck. |
|
the PR landed, please rebase. Thank you for working on this! <3 |
|
@wojtekmach I've rebased and amended your new test slightly to test the new output. I hit an issue with running the tests but it seems isolated to my system. |
|
@Odaeus thanks, fixed on main! |
|
@wojtekmach updates made with a commit on top (22ab1c9). Let me know if I should squash away the in-between commits. |
|
No need to squash yourself, we'll do it when merging. |
Issue: elixir-lang#602 Remove the "Specs" heading in favour of specifying `@spec` in-line with the spec definition. This makes it clearer visually because the it reduces the prominence of the typespec over the function signature, and semantically because the heading is not correct for the descriptive text.
Instead of prefixing `@spec` for all typespec output, use the correct matching module attribute used to define it.
Show the typespec definition in Erlang format (`-spec`) for Erlang code by adding `ExDoc.Language.format_attribute/1` and calling it from the template via the `module.language` property.
The formatting of spec attributes is language specific and not all languages support the same attributes, so rename and move `get_spec_attribute/1` from the formatter to the language modules.
|
Just checking in to make sure you're not waiting on me for anything that I've forgotten. 😄 |
|
Thank you! |
See issue #602.
As well as replacing the heading I removed the padding as it seemed excessive, but I'm happy to restore that if you like.
Original:
New (without padding):
With padding:
