bscan / PerlNavigator

Perl Language Server that includes syntax checking, perl critic, and code navigation
MIT License
190 stars 35 forks source link

add npm install bin entry #83

Closed mcgaw closed 11 months ago

mcgaw commented 11 months ago

According to https://docs.npmjs.com/cli/v6/configuring-npm/package-json#bin this should result in a symlink being made to your system bin folder when installing globally, which means perlnavigator will be immediately usable after npm install -g perlnavigator-server.

bscan commented 11 months ago

This is excellent, thanks! I think this will help make the installation much easier for people.

Also, it seems like all non-vscode editors use stdio as the default communication method with the lsp. It also supports node-ipc (which vscode uses), tcp sockets and named pipes. However, every editor I've seen so far uses --stdio. in server/bin/perlnavigator, it would be nice to add --stdio if no other arguments have been passed. If any arguments are passed, just pass them through to server.js

mcgaw commented 11 months ago

No problem! Is there a strong reason that these editors should not explicitly pass this flag? I know Helix does by default, for example. Or if this was to be a general "default" behaviour then would it not be better in the node app itself, so that it was common? As it stands the main releases using pkg wouldn't have this default, since in the top level package.json the bin entry is still pointing to server.js.

bscan commented 11 months ago

Thanks! I've been thinking about it, and I agree that it would be better to set the default as stdio directly in the server.js node app. This would also allow the bundled binaries to work without specifying --stdio. Reading through the options, I believe the vscode-languageserver-node designers didn't specify it as default because it's the slowest option. VScode uses node-ipc which is highly optimized for communicating between node apps. TCP is also pretty fast. The overhead of all the text encoding and string manipulation of stdio slows things down. However, most of the editors seem to use stdio anyway for ease of use, so I'm happy to follow suit since it will make installation easier for most users. I'll update the main app. Thanks again for the contribution!

mcgaw commented 11 months ago

Awesome, thanks!