Language Server Protocol Experiments

[agg23]

A very rough draft of implementing the Language Server Protocol as mentioned in #80.

You can use this by pulling https://github.com/agg23/millfork-lsp-vscode and switching the jar to point to this version of Millfork.

Functionality

• Hover and go to definition support of definitions, including import statements and cross-file definitions
  • Length information is currently not available in the AST, so looking up the Node under the cursor relies on the last column index before the cursor position

TODO

• [x] Proper LSP initialization (use CLI arg -lsp)
• [x] Implement length measurement in Node
  • I cannot figure out what is needed to accomplish this. @KarolS for help
• [x] Support client configuration of include directories
• [x] Cache parse results (rather than re-parsing on every interaction) and update on file updates
• [ ] More gracefully handle parse errors
• [x] Add AST positions for import statements (used for go to definition)

[agg23]

@KarolS, what is your opinion on adding a config file for common arguments (a .millforkrc.json or similar file)? The other option is to manually add configuration for the language server plugin in every repo (lib directories, arch, etc...) and not have it committed/part of the file structure.

[KarolS]

Common tooling configs are fine and welcome, there's a Notepad++ syntax file in the repo already. So if I understand correctly, the file specifies a configuration for the server that is good enough for most potential users? If so, then sure, add it.

[agg23]

There's two separate conversations about config files here. The first is about editor specific (.vscode directory, .scalafmt.conf) config, which should work for all users mostly out of the box. The second is introducing project specific config for the language server (and likely the CLI in general). This would allow the user to define what directories to include, what architecture to build for, and configure other Millfork options, in a format that is easily consumable for the LSP server. Additionally, I imagine we expand this to the CLI, so running java -jar millfork.jar by default pulls the config present in .millforkrc.json in your working directory, unless CLI args override them.

On a different topic, have you had a chance to look into how we can add the length of an expression to the AST? I imagine it's relatively easy to do, but in reading through FastParse docs I couldn't find anything

[KarolS]

As for the length of the expression, I guess it would be enough to record the position at both the beginning and end of a node. I think it should be doable. I might implement it partially though, for example just for expressions. I'll see how it goes.

[agg23]

I naively tried collecting the position() at the end of the parsing for that object (at least, I think that's when it runs), but that just breaks parsing entirely. You can see what I tried here: https://github.com/agg23/millfork/commit/d9d62f3b34adb5543874bf5caaa2ccb09a04cc45

[agg23]

Opening this up for review. Are there requirements you'd like to meet before this is considered ready to merge? There's still some scenarios the LSP implementation doesn't cover, but I think its useful enough to open it up to a broader audience.

[agg23]

@KarolS If you get a chance, I would really appreciate you taking a look at what I have so far.

[KarolS]

I'll take the look over the weekend. Thanks!

[KarolS]

I did some quick tests and apart from the sbt version thing and the docComment extension method, nothing else broke. However I would like to avoid using stderr for the diagnostic messages (I guess it can be done conditionally, depending on the LSP mode) and implementing multiline comments (C# uses ///, I think we can steal it). I left some comments in the relevant spots in the code.

I haven't tested the LSP itself yet, I'll do it tomorrow.

[agg23]

However I would like to avoid using stderr for the diagnostic messages (I guess it can be done conditionally, depending on the LSP mode)

I wasn't particularly a fan of doing it that way, but I wasn't sure how else to handle it. We could bind user interaction to stdout and the LSP to stderr, but that doesn't fit as well into the *nix style.

implementing multiline comments (C# uses ///, I think we can steal it)

Do you include the docstrings with the multiline comments? I don't care as much about the standard multiline style. I only added it because I added the docstrings.

I left some comments in the relevant spots in the code.

Do you mean Github review comments? I don't see them at all.

[agg23]

I cleaned up the writing to stdout/err and a few other things. I'm still not sure what you want in terms of the doc comments and multiline comments.

I also cleaned up the VSCode extension; it should be easier to set up and run (no code changes required).

[agg23]

It's been a long time, but I finally got back to looking at this. I have a working identifier position recording branch available as a PR against this one https://github.com/agg23/millfork/pull/1. It's messier than I would like, given the way Node is spread throughout the code. Let me know if you're ok with this as it is @KarolS, or if you would prefer if I clean it up before we look at getting LSP as a whole merged.

[agg23]

Wanted to ping again @KarolS and see where you stand on this PR. Any input would be appreciated.

[KarolS]

Sorry for keeping you waiting, I was a bit busy in last few months, and when I got back to Millfork recently, I focused more on bugfixes and small usability improvements. I'll take a good look at it this weekend. I'm planning on building the fork and testing it with some of my projects.

[agg23]

I greatly appreciate you sharing Millfork with the world, and I don't mean to be rude @KarolS, but do you realistically think there will ever be progress on adding LSP support to Millfork? I've spent a not insignificant amount of time on this set of changes, and after nearly two years I haven't really received any feedback.

I fully understand being busy and/or not wanting to dedicate time to the project at the moment (I can see you haven't been very publicly active on Github recently). However, it would be nice to have some indication of your desires.

[KarolS]

I'm really sorry for lack of communication. The last few years have been really busy and stressful, and I don't think I had any free weekend for many months. With all that, I barely had any time to even think about Millfork and therefore forgot about many things I planned to do, including this pull request. Even when I did have some time for Millfork, it was never enough to handle major issues like this one, so I again pushed it back for later, and forgot about it again. I don't think I'll have any free time before late August, maybe September. I'm planning to return to everything I put on hold then, including Millfork, and I'll take an honest look on this PR. Again, sorry!

[agg23]

I appreciate the response, and I really sympathize with your stress. Don't stress extra about the PR or Millfork; they'll be here when you're ready.