vim-ocaml
vim-ocaml copied to clipboard
Add OCamldoc/Odoc highlighting
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:
syntax/ocaml.vim→syntax/odoc.vim→syntax/ocaml.vim→ STOPsyntax/odoc.vim→syntax/ocaml.vim→ STOP
This is joint work with @shym.
Thank you :-)
We can't reproduce the first example.
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).
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.
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.
This also interferes with regular comments:
(* type below is highlighted as a comment *)
type t
This also interferes with regular comments:
(* type below is highlighted as a comment *) type t
I have this behaviour only with neovim.
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?
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] *).
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.
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?
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.
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.
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.
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.
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.
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
.mlifile, 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/.mlifiles.