Eep 48 markdown #1100

garazdawi · 2021-10-01T16:02:23Z

Description

This PR adds a markdown rendering backend for EEP-48 style documentation.

TODO:

Create markdown renderer
Fetch docs for types
Use edoc doc chunks renderer when available
Use edoc doc chunks renderer when available
Add docs to type completion
Extract clauses when docs do not exist
Check all versions of file for eep48 docs before resorting to edoc (i.e. if developing OTP and autocompleting the lists module)
Fix multiline code examples in emacs. See atom_to_list/1 for example.
Add testing...

In VS code the change looks like this.

Before:

After:

I'm opening this as a draft to get some feedback if this is something you want. If so, there is more work to be done in order to add tests and also check that is actually renders the markdown nicely in most of the editors.

robertoaloi · 2021-10-03T19:19:14Z

Hi @garazdawi !

I guess improving the way documentation is displayed would be a nice thing to do. I just wonder why we'd need to fork the shell_docs module which is currently part of stdlib. Is the idea to iterate faster within the erlang_ls repo and eventually merge the changes back to OTP?

garazdawi · 2021-10-04T06:51:55Z

I guess improving the way documentation is displayed would be a nice thing to do.

You don't sound overly convinced?

I just wonder why we'd need to fork the shell_docs module which is currently part of stdlib. Is the idea to iterate faster within the erlang_ls repo and eventually merge the changes back to OTP?

I choose to make a copy of the Erlang/OTP implementation for rendering in the shell in order to get some of the basic framework in place. I've redone much of the actual rendering logic as markdown and ansi shells are different in what they need.

I started out with the idea of creating a markdown renderer for EEP-48 style documentation in Erlang/OTP. But I think that in order to make the docs look nice, it will have to render different markdown depending on the editor, which leads me to think that this code belongs here instead.

If it turns out that I was wrong and no difference was needed for different editors, I'm not opposed to moving it into Erlang/OTP as I would rather maintain them both in one place than two separate versions.

robertoaloi · 2021-10-04T07:02:06Z

You don't sound overly convinced?

I am very convinced! I was just wondering if there was any reason for not wanting this, given you asked :)
So, go ahead! The plan makes sense to me.

garazdawi · 2021-10-04T07:54:54Z

I am very convinced! I was just wondering if there was any reason for not wanting this, given you asked :)
So, go ahead! The plan makes sense to me.

My main concern was that it is a lot of code and might increase maintenance while not providing enough benefit.

Happy to hear that you would like to have this. I'll spend more time on it and get something that can be merged soon(tm).

garazdawi · 2021-10-04T16:34:42Z

Did some more work.

Added documentation lookup for types:

Started using edoc chunks (made possible by erlang/otp#2803 in Erlang/OTP 24) for creating docs from edoc:

Before:

After:

I haven't added so that you get documentation on completion of types (yet). Any hints you have of what has to be changed in els_completion_provider to make it work would be most welcome.

I also noticed that Elixir already has done this work, though I think my renderer is a bit more correct/complete. So maybe there is an opportunity to share some code in between the projects.

It would have been really neat if you could refer to other files or allow hovering on things within the returned markdown docs so that we could expand the type documentation within the function documentation if the user want that. But it seems like it is not possible.

robertoaloi · 2021-10-06T17:41:02Z

@garazdawi Let us know when the PR is ready for review!

garazdawi · 2021-10-07T06:39:53Z

Will do. I've added a todo list in the first post so that you can track progress.

garazdawi · 2021-10-21T11:17:29Z

I've given up on fixing it so that emacs works properly. There seems to be some strange bug in lsp-ui that sometimes deletes newlines from code fenced blocks and my emacs debugging skills are not good enough to figure out what is going on. I may submit an issue to them and see if anyone can assist.

The only thing remaining is creating testcases for all the new code (and fixing any bugs I find a long the way).

So, I would say the code is ready for review!

When there are multiple versions of the same file we sometimes want to check all of them.

When developing Erlang/OTP all the modules (i.e. lists.erl) show up at least twice so when looking for the EEP-48 style docs we should check all of them in prioritization order for docs and display the first we find.

This makes the -type poi act more like -spec, which is needed for fetching the -type for rendering docs. The builtin types have documentation in 24.2, so we should create pois for them so that we can use that in future versions.

The docs will be create by els_docs instead.

apps/els_lsp/src/els_rename_provider.erl

rebar.config

robertoaloi

LGTM (we may found issues later on, but I don't see a problem solving them incrementally).
Only things are support for "-opaque" as H. mentioned and the "missing_spec_all" warning that has been commented out. Otherwise we can merge, unless someone else would like to have an extra review.

garazdawi · 2021-10-24T19:40:37Z

I've added tests for most of the things now and addressed the other issues brought up.

Tests are missing for:

Documentation rendering
Multiple modules of same name (i.e. when editing the lists module)
Rendering documentation for elixir modules

When I get the CI to pass (just need to fix some linting issues) I think we can merge this and I'll try to find some more time to add more tests.

garazdawi · 2021-10-25T07:50:28Z

There we go, CI passed. Feel free to merge when you want to. Or if you have any more comments for things you want me to address.

robertoaloi · 2021-10-27T06:41:23Z

apps/els_lsp/src/els_rename_provider.erl

@@ -156,6 +156,8 @@ editable_range(#{kind := type_application, id := {M, T, _A}, range := Range}) ->
  EditToC = EditFromC + length(atom_to_string(T)),
  els_protocol:range(#{ from => {FromL, EditFromC}
                      , to => {FromL, EditToC} });
+editable_range(#{kind := type_definition, data := #{ name := Range }}) ->


It can be addressed as a follow-up, but isn't it a bit counterintuitive to have a range under name?

naming is hard :)

robertoaloi · 2021-10-27T06:41:53Z

Thanks so much for this @garazdawi ! 🙏

garazdawi force-pushed the eep-48-markdown branch 5 times, most recently from b352bec to fe65d62 Compare October 5, 2021 07:46

garazdawi force-pushed the eep-48-markdown branch from 12ec252 to ad993d4 Compare October 8, 2021 15:21

garazdawi force-pushed the eep-48-markdown branch from ad993d4 to 3bd8d40 Compare October 17, 2021 13:16

garazdawi added 8 commits October 20, 2021 10:42

Vendor Erlang/OTP shell_docs rendering

d33f099

Implement markdown rendering

ddc37bd

Add lookup of docs for types and use edoc chunks

0d7ab9d

Add eep48 to elvis ignore list

56b6ba9

Fix compilation before Erlang/OTP 23

046f76e

Fix dialyzer error

c2012ea

Fix a bunch of doc rendering issues

ac75964

Fix Erlang/OTP < 23 dialyzer issue

40a2280

garazdawi force-pushed the eep-48-markdown branch from 90d26ce to 120ee00 Compare October 20, 2021 08:55

gomoripeti mentioned this pull request Oct 21, 2021

Add support for hover docs for types #1118

Closed

garazdawi marked this pull request as ready for review October 21, 2021 11:17

garazdawi added 4 commits October 21, 2021 15:01

Introduce local group_leader for doc to capture output

c583613

Add els_utils:find_modules for fetching all variants of a module

f1cfe5b

When there are multiple versions of the same file we sometimes want to check all of them.

Build edoc doc-chunks in user cache directory

f98d438

Check all variants of a module for docs before generating

317d385

When developing Erlang/OTP all the modules (i.e. lists.erl) show up at least twice so when looking for the EEP-48 style docs we should check all of them in prioritization order for docs and display the first we find.

garazdawi added 12 commits October 21, 2021 15:01

Polish fetching of type documentation

95aec6c

Do not include private functions when generating edoc

773ae3e

Do not generate hr if no other docs are to be shown

43c0750

Fix testcases

41c079e

Include type docs in completion

a75d9e4

Fix crash when macro definition cannot be found

3607c8b

Indentation fix

0fb84e4

Remove dead code from eep48 module

92fa84f

Do not render docs for functions/types with none docs

9156248

The docs will be create by els_docs instead.

Fix completion testcases

ae61200

Fix testcases

a0fa304

plux reviewed Oct 21, 2021

View reviewed changes

apps/els_lsp/src/els_rename_provider.erl Outdated Show resolved Hide resolved

garazdawi force-pushed the eep-48-markdown branch from 3bb3f88 to a0fa304 Compare October 21, 2021 14:09

robertoaloi reviewed Oct 22, 2021

View reviewed changes

rebar.config Outdated Show resolved Hide resolved

robertoaloi reviewed Oct 22, 2021

View reviewed changes

garazdawi added 7 commits October 23, 2021 15:50

Add specs to all els_eep48 functions

7b705a4

Fix rename of opaque types

77bd8c7

Add rename_opaque testcase

6b9c1ba

Add testing of edoc completion docs

736006a

Fix sending of unicode text in els_client

a84d93f

Expand testing of documentation for completion items

e854f2e

Added test for hover documentation

bb57290

Fix line length issues

923b5dc

robertoaloi reviewed Oct 27, 2021

View reviewed changes

robertoaloi merged commit d975ac7 into erlang-ls:main Oct 27, 2021

garazdawi deleted the eep-48-markdown branch October 27, 2021 06:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Eep 48 markdown #1100

Eep 48 markdown #1100

garazdawi commented Oct 1, 2021 •

edited

Loading

robertoaloi commented Oct 3, 2021

garazdawi commented Oct 4, 2021

robertoaloi commented Oct 4, 2021

garazdawi commented Oct 4, 2021

garazdawi commented Oct 4, 2021

robertoaloi commented Oct 6, 2021

garazdawi commented Oct 7, 2021

garazdawi commented Oct 21, 2021

robertoaloi left a comment

garazdawi commented Oct 24, 2021

garazdawi commented Oct 25, 2021

robertoaloi Oct 27, 2021

garazdawi Oct 27, 2021

robertoaloi commented Oct 27, 2021

Eep 48 markdown #1100

Eep 48 markdown #1100

Conversation

garazdawi commented Oct 1, 2021 • edited Loading

Description

robertoaloi commented Oct 3, 2021

garazdawi commented Oct 4, 2021

robertoaloi commented Oct 4, 2021

garazdawi commented Oct 4, 2021

garazdawi commented Oct 4, 2021

robertoaloi commented Oct 6, 2021

garazdawi commented Oct 7, 2021

garazdawi commented Oct 21, 2021

robertoaloi left a comment

Choose a reason for hiding this comment

garazdawi commented Oct 24, 2021

garazdawi commented Oct 25, 2021

robertoaloi Oct 27, 2021

Choose a reason for hiding this comment

garazdawi Oct 27, 2021

Choose a reason for hiding this comment

robertoaloi commented Oct 27, 2021

garazdawi commented Oct 1, 2021 •

edited

Loading