Add --stdio switch

[jjlee]

This was discussed on the forum here. I understand your focus is on VS code so I'm not taking for granted that this will be accepted -- obviously I think the patch should be applied though :-)

Not sure if it helps, but I don't expect to be coming back with any requests to change behaviour beyond this in order to support emacs. Everything already seems workable as-is, the emacs lsp-mode.el people target feature parity with vscode, and also provide general-purpose message processing hooks to allow ad-hoc server-dependent treatment.

Should I add an explicit comment next to the command line switch code along the lines below?

// The fact that rescript-vscode uses LSP under the hood is an implementation
// detail we don’t really guarantee.  This --stdio switch is therefore not
// guaranteed to remain useable.

[chenglou]

Thanks! Whether we merge this or not, thanks for the PR =)

Yeah I'm a little worried about supporting important codepaths when we don't actively use or recommend them. In particular, providing an upfront disclaimer doesn't work at community scale because folks still get frustrated even with the warning and/or there ends up being folks who adopt this setup from someone's ill-guided recommendation without knowing about the disclaimer.

However we can make exceptions when the ROI is big enough... so that's a maybe. Let's see how clean this PR gets first.

[chenglou]

Why are we looping over the args? Is it because you're catering to having the server opened with extra arguments from the debugging mode (does it do that...? I forgot)

[jjlee]

Thanks for looking at the PR!

It's not because of debug mode: I wasn't aware of that until now and I'm not yet familiar with whatever debugging facilities are provided by either VS code or LSP.

I think I haven't understood your question correctly. Are you just saying it would be nicer to use something like process.argv.includes("--stdio") instead of a loop?

If not: I think you understand all this already, but I'll state the context explicitly. Some editors, emacs included, want to talk LSP over standard in / standard out (stdio). Currently rescript-vscode's server.ts, if I've understood right, is only able to talk over "node IPC", which is only intended for use by node programs (i.e. programs like emacs shouldn't try to use that).

I don't know another way for the server process to know it should use stdio than looking at process.argv? I believe this is standard in LSP servers. I think I lifted this loop from vscode-languageserver-node:
https://github.com/microsoft/vscode-languageserver-node/blob/53d122831050088171296dc02b57257f5178f0b6/server/src/node/main.ts#L203

Or are you maybe thinking that vscode-languageclient should be used to tell the server to use stdio, even for editors other than VSCode? If so: I think 1. vscode-languageclient is indended only for use by VSCode clients, not servers 2. All it does other than arranging to use stdin/stdout on the client end is to pass "--stdio" to the server it's starting -- so it's still up to server.ts to test whether "--stdio" is present in the args and implement using that channel.

But as I say I I've probably misunderstood what you're asking, sorry!

[chenglou]

Nah I mean why not just check the third argument =)

[jjlee]

lol

OK, only because: 1. I copied the code from vscode-languageserver-node, 2. expecting commandline arguments only at a particular place in argv always seems a bit confusing / error-prone.

Happy to check argv[2] if you prefer, though .includes("--stdio") seems nicer?

[chenglou]

Sure! Could you also simplify messageSender and onMessage to 2 function calls initiated by whether we see the flag? A little bit of contortion in the code right now

[jjlee]

Hope that's what you meant

[chenglou]

No just 2 calls, but I might have missed something. That's fine; I'll merge and do it later.

[chenglou]

Thanks. I'll cut a release before merging this. Then I'll ping willing folks to test master

[chenglou]

Alright, I've cleaned up the code and pushed to your branch. I've briefly tested on Sublime Text and it seems this is good to go now. Feel free to test master.

Thanks!