vim-ocaml icon indicating copy to clipboard operation
vim-ocaml copied to clipboard

Add OCamldoc/Odoc highlighting

Open n-osborne opened this issue 1 year ago • 15 comments

This PR proposes to add syntax highlighting for OCamldoc and Odoc documentation.

This adds syntax highlighting for both *.mld files and documentation comments in OCaml files.

OCamldoc/Odoc syntax needs to load OCaml syntax in order to highlight code extracts. This PR uses the following strategy to avoid an infinite loop: documentation inside code extract inside documentation is highlighted as simple comments.

This means that we can have the following load chains:

  1. syntax/ocaml.vimsyntax/odoc.vimsyntax/ocaml.vim → STOP
  2. syntax/odoc.vimsyntax/ocaml.vim → STOP

This is joint work with @shym.

n-osborne avatar Jan 26 '24 15:01 n-osborne

Thank you :-)

We can't reproduce the first example.

n-osborne avatar Jan 26 '24 16:01 n-osborne

Besides that, the OCaml syntax is heavily using “regions”, for instance for module. I’m not sure I really understand why it is written in this way and I didn’t find any way to escape, so that ]} would break those regions without modifying the OCaml syntax for that purpose (which is maybe not worth it, it is complex enough as it is).

shym avatar Jan 29 '24 10:01 shym

I checked that I use the last version of the patch, merged on top of https://github.com/ocaml/vim-ocaml/pull/81. I reproduce the bug both in a .mld file and in doc comment.

Julow avatar Jan 30 '24 10:01 Julow

I can reproduce it, I don’t really what happened last time :thinking: Adding \]} to possible end tokens for ocamlTypeDef and ocamlTypeDefAnd fixes a few type examples for me. Now I wonder if we are missing other cases. I’m not sure what behaviour we should aim for in the module case.

shym avatar Jan 31 '24 18:01 shym

This also interferes with regular comments:

(* type below is highlighted as a comment *)

type t

Julow avatar Feb 01 '24 19:02 Julow

This also interferes with regular comments:

(* type below is highlighted as a comment *)

type t

I have this behaviour only with neovim.

n-osborne avatar Feb 02 '24 09:02 n-osborne

We’ve pushed an update that fixes the bug in which a single (* opens two ocamlComment scopes. We’ve also updated the syntax so that documentation is accepted where comments are. With this updated version, the mld

{[ type t = int ]}
This is as text, [code]

is correctly parsed with vim 9.1.16 and neovim 0.9.4 for us. Is it broken for you? On which version of vim?

shym avatar Feb 29 '24 17:02 shym

I confirm this now works fine :)

Though, the doc comments in ml and mli files are no longer rendered in the same color as regular comments, which is confusing. Also, code spans are not highlighted differently: (** foo [bar] *).

Julow avatar Mar 01 '24 16:03 Julow

I confirm this now works fine :)

We’re making progress, great! Unfortunately, we already found a bug :-( In the OCaml code:

module Make (X: sig end) =
  struct
    type t = int
  end

the body of the module is not highlighted correctly. This is similar to the bug we solved for comments, where an opening parenthesis was opening two syntax scopes. The parenthesis for the functor parameter is opening two ocamlModParam. Maybe trying to inline the documentation highlighting would end up more reliable?

Though, the doc comments in ml and mli files are no longer rendered in the same color as regular comments, which is confusing.

You think there is more than getting accustomed to this new highlighting? We could indeed highlight the text body as comments. Our rationale is that comments are usually dimmed while we want documentation to stand out.

Also, code spans are not highlighted differently: (** foo [bar] *).

Indeed. I think odoc makes very little highlighting for inline code blocks, so we kept it very sober. Any suggestion?

shym avatar Mar 01 '24 18:03 shym

We've removed ocamlModParam from ocamlModParam's contains. This fixes the last bug and doesn't seem to change things regarding #3 (even on today's master branch).

I believe all the known bugs are fixed.

n-osborne avatar Mar 22 '24 10:03 n-osborne

Hi, sorry for the delay! At last I found time to have a closer look at the PR. I found a couple of bugs, I will send a review soon.

Maelan avatar Apr 02 '24 16:04 Maelan

Now that I think of it, I think many of the regions in odoc.vim should be made transparent (:help syn-transparent), so that what’s inside them is styled just like what’s containing them (while still highlighting the delimiters). This makes it easier to change the styling of documentation text (because otherwise, regions that are not transparent don’t inherit style from their containers, they reset it). The relevant regions to be made transparent are:

odocLinkText ?
odocMiscInline
odocTable (both rules)
odocTableRow
odocTableEntry
odocList
odocListItem
odocDyckWord (already mentioned)
odocBalancedBracket (from another reviewing comment)

We don’t want to give the transparent attribute to regions that we might want to style specially (such as odocCode), because, apparently, transparent regions can’t be given proper style later.

Maelan avatar Apr 16 '24 12:04 Maelan

Sorry for all these comments! I have finally a higher-level remark: when I tried the PR with a real-life .mli file, I found the bilingual ocaml/odoc highlighting to be too noisy for my eyes, and I was a bit lost. For instance, odoc tags are highlighted like Keyword, but the OCaml syntax also uses Keyword a lot, and (with my colorscheme at least) it stands out a lot. On the other hand, PreProc would almost make sense here, OCaml dosn’t use this group, and with my colorscheme it looks dimmer than Keyword, closer to the default styling.

So I did these customizations on my part:

hi link ocamlDocEncl PreProc
" ^ defined by PR review
hi link odocMarker PreProc
hi link odocCrossrefMarker odocMarker
" ^ defined by PR review
hi link odocTag PreProc
hi link odocCrossrefKw Structure
" ^ or PreProc
hi link odocLinkText odocItalic
hi link odocCode Constant
hi link odocCodeBlock odocCode
hi link odocVerbatim odocCodeBlock

… and I also disabled OCaml syntax linting within code blocks within odoc text. And now it looks better to me. These are just suggestions, in case you like it too.

Maelan avatar Apr 16 '24 13:04 Maelan

In case that helps, in the branch odoc-syntax-patched of my own fork I have commited most of the changes I suggested in this review, now rebased on top of your latest update. It is split in several commits for easier reviewing, but feel free to squash everything, or copy-paste the parts you want.

Maelan avatar Jul 01 '24 22:07 Maelan

First of all, thanks a lot for your very thorough review! I hope we didn’t miss anything, as we took so much time to address it.

I have finally a higher-level remark: when I tried the PR with a real-life .mli file, I found the bilingual ocaml/odoc highlighting to be too noisy for my eyes, and I was a bit lost. [...]

I’m not really good at styling, especially in a case that is so dependent on the independently-chosen color scheme. I wonder now whether we should:

  • make a poll to pick the default color,
  • create a variable to disable highlighting inside .ml/.mli files.

shym avatar Sep 27 '24 18:09 shym