Markdown comments

[robertoaloi]

Overview

This is a proposal to enable syntax highlighting (and parentheses matching) for the content of Markdown doc attributes.

VS Code seems to allow embedded languages, and I think we'd gain a lot from having the ability of highlighing syntax for the markdown content

I am not super-familiar with TextMate grammars, so please advise if there's a better way.

This is what it looks like using the ELP VS Code extension:

Compared to the current:

There's a gotcha. By default, the Markdown text looks like plain-text (e.g. white on dark themes), so to be able to see it as a comment, the extension can do something like:

"configurationDefaults": {
      "editor.tokenColorCustomizations": {
        "textMateRules": [
          {
            "scope": "meta.embedded.block.markdown",
            "settings": {
              "foreground": "#6A9955"
            }
          }
        ]
      }
    },

This is far from ideal, since each extension needs to add something similar. It can also play badly with custom themes, even if it's easily possible to override the font. Ideally, we'd like to tell VS Code to map that scope to whatever foreground is used for comments, but I could not find a way.

Other changes included in this PR

These are superseded by the embedding of Markdown, but I think it's useful to share what I found while playing with this feature. Maybe there's something I'm missing.

Incorrect grouping

The orginal match group (\5) seems incorrect to me. I believe it should have been \7 (the closing quotes) as you can see here

Incorrect scopes

The original version did not fully work for me. While all the text was rendered as a comment, inspecting the scopes showed that odds/even lines had different scopes:

In particular, odd lines had the comment.block.documentation.erlang, but even lines didn't.

[robertoaloi]

@garazdawi @KornelH it would be great to have your feedback on this.

[KornelH]

Hi @robertoaloi,

It looks good to me but I'm thinking about two things and you just addressed one of those. If it fits into one theme only then I feel it inconsistent as many people use non-default themes. I have several colleagues using some fancy or light themes that is alright, of course, but a solution shall work well with all the themes.

The other thought is that, how much embedded code will distract the reader. Somehow that part shall not look like real source code to make it possible to focus on the source code or the documentation but not mixing up the two. I hope you can understand this thought.

Anyway, I'll go through the changes soon and play with those a bit.

[robertoaloi]

@KornelH The custom colour seems to work nicely with both light and dark themes and it can be overwritten in a very simple way. Of course it would be great to tell VS Code "use the comment foreground colour" rather than hard-coding a colour, but I'm not sure that's possible (suggestions are welcome).

Regarding the "distracting" part, I share the concern. But if I have to compare the previous flat-green without any support for matching quotes/parentheses with the one proposed here, I think I prefer the latter. After trying the new format internally, I found so many issues with our docs (e.g. missing quotes) that I think the gains justify the compromise.

[robertoaloi]

@KornelH Also, what about e69770b and 8f53a99?

[KornelH]

Incorrect grouping (8f53a99)

Well, It looks I made a mistake when set the comment look for module/function docstrings. Thank you for fixing it! 🙂

Incorrect scopes (e69770b)

The alternating scopes are strange, indeed. You are right, we don't need while loop in the grammar to catch the end of the docstring. At the beginning I didn't know how to properly specify multi-line expressions, so I tried things and while is a remnant of that. Thanks again for spotting it! 🙂

Embedded Markdown (fe898be)

I just checked out your commit and checked the syntax highlight through vscode_erlang and got the below look for lists:filtermap/2:

I still think it's way more source core lookalike than documentation and steals the focus. Therefore I still prefer the comment look. 😕 However, I agree of having support for matching quotes/parentheses would be nice.

[KornelH]

The custom colour seems to work nicely with both light and dark themes and it can be overwritten in a very simple way.

I checked in different themes. In default dark and light modes it's indeed looks nice because the comment is green in both themes. But in more exotic themes it differs. ☹️

Of course it would be great to tell VS Code "use the comment foreground colour" rather than hard-coding a colour, but I'm not sure that's possible (suggestions are welcome).

Extensions can "subscribe" to configuration fragment changes (e.g. theme changes) and if extensions can change their syntax highlight settings on-the-fly too, than we can write a hook in Erlang extensions to react on theme changes. But this thought have a lot of ifs ...

[KornelH]

Hi @robertoaloi, regardless if we agree on the coloring of Markdown comments or not, I think you definitely should submit the other commits to fix those issues.

[robertoaloi]

@KornelH Sorry for the delay (vacation). I will merge this as-is (the PR is already integrated in ELP) and we can revert the highlighting if it becomes too distracting. But that doesn't seem to be the case from the initial feedback from ELP users.