vscode-ocaml-platform icon indicating copy to clipboard operation
vscode-ocaml-platform copied to clipboard

Add new syntax patterns for dune(-project) files

Open ComanderP opened this issue 1 year ago • 3 comments

Resolves #1180

ComanderP avatar Feb 18 '24 22:02 ComanderP

Hi, this is my first time contributing here, sorry if I did something wrong. I went through most of the dune documentation and added syntax highlighting to most of the stanzas in the list of the issue. I think most of the stanzas are covered now, with the exception of the stanzas from the config section, which I do not know if they're in a separate file or not.

On a side note, I took note of some things while doing this, concerning the syntax highlighting and the documentation:

  • I tried to not touch the executable(s) and library/libraries stanzas, as I didn't want to mess with the existing syntax highlighting that they have in common.
  • The menhir stanza added to env in version 3.0 of the menhir extension was not added as I was not sure if I was allowed to make a separate rule in the file's repository for both the normal menhir stanza and the menhir stanza in env. I can add it if it's okay. I also didn't want to just add the menhir stanza rules inside the env stanza rules, as it would lead to code duplication.
  • The documentation for the subst stanza seems to be incorrect, I think. It says that the subst stanza takes a <bool> as an argument, but it actually takes <enabled|disabled>. For example, the warnings stanza takes <enabled|disabled> and is documented as such. If you want, I can make a PR to fix this.
  • Another thing that I noticed is that the different TextMate grammar files have different formatting styles. This isn't that big of a deal, but it's something that I noticed.
  • Should external_variant be added to the syntax highlighting? I didn't add it as it was an experimental feature removed in version 2.6 of dune, and there's no documentation for it in the dune docs anymore.
  • I noticed that there is a variable.declaration.other.dune name and a variable.other.declaration.dune name in the syntax highlighting files. I don't know if this is intentional or not, and I do not know if there is a difference between the two. I tried both of them and they both seem to work the same way.
  • For the install stanza, the documentation only mentions 3 fields (files, section, package), but the syntax highlighting file is highlighting way more fields than that. I think there may have been a mistake with the executable fields, as there's a mention of the install stanza in the documentation for the executable stanza followed by the fields for the executable stanza. I'm not sure if this is a mistake or not, but I mentioned it just in case.

I hope this is helpful. Let me know if I should change something or if I missed something.

ComanderP avatar Feb 18 '24 22:02 ComanderP

Hi, thank you for the feedback! I've added the config stanzas to the dune-workspace file as you said, but I'll try changing some things based on what you said as well before committing.

ComanderP avatar Mar 05 '24 01:03 ComanderP

Sorry for the delay, got caught up with my studies. I fixed/changed/added stuff based on your previous insights. Hopefully, all's fine now.

ComanderP avatar May 04 '24 01:05 ComanderP