add table of contents to the specification #114 #117
Conversation
|
Hi @vladdu, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! TTYL, MSBOT; |
|
@vladdu, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR. |
|
|
||
| Notifications client → server | ||
|
|
||
| * [workspace/didChangeConfiguration](#workspace_didChangeConfiguration) :arrow_right: |
There was a problem hiding this comment.
This link does not work. I think you miss the anchor.
|
|
||
| * [workspace/didChangeConfiguration](#workspace_didChangeConfiguration) :arrow_right: | ||
| * [workspace/didChangeWatchedFiles](#workspace_didChangeWatchedFiles) :arrow_right: | ||
| * [workspace/symbol](#workspace_symbol) :arrow_right: |
There was a problem hiding this comment.
This is a request. It should be in the "Requests" section and the icon should be changed.
| Notifications server → client | ||
|
|
||
| * [window/showMessage](#window_showMessage) notification :arrow_left: | ||
| * [window/showMessageRequest](#window_showMessageRequest) :arrow_right_hook: |
There was a problem hiding this comment.
This is a request, not a notification.
|
Amended with the suggested changes. Changed the icons at the beginning, which gives a better feeling and grouped by area (workspace, window, textDocument). I feel it would be even better if there were different colors for the arrows, to be able to differentiate between c->s vs s->c. Maybe using real images instead of emojis is better, because the emojis are a github feature? |
|
|
||
| >**New:** The telemetry notification is sent from the server to the client to ask the client to log a telemetry event. | ||
|
|
||
| _Notification_: | ||
| * method: 'telemetry/event' | ||
| * params: 'any' | ||
|
|
||
| #### DidChangeConfiguration Notification | ||
| #### <a name="workspace_didChangeNotification"></a>DidChangeConfiguration Notification |
There was a problem hiding this comment.
Did you mean "workspace_didChangeConfiguration" here?
|
The new layout looks better. I am personally fine with emojis. I read the protocol document directly on GitHub. |
|
Hopefully the specification document will get generated from the schema, once #25 is implemented. The emoji names have no relation to their meaning. |
|
I love this, but the ToC should be at the top, not the bottom |
|
I put it at the end because it's a rather long list. The best solution would have been to have it as an aside, and with smaller text, but I think that would only be possible with raw HTML and that would make it a bit more difficult to maintain manually. Maybe at the top we could add a link to the TOC located at the bottom? |
|
Well GitHub supports |
|
If moving the TOC at the top, do you think it's best at the very top (before "Base Protocol") or before " Actual Protocol"? |
|
The very top, after the introduction. |
|
Ok, moved it. |
|
Cool. |
resolves #114
The TOC also has icons added to specify if the messages are requests or notifications and in which directions they go. These could also be added to the descriptions.