Formatting document or selected text issue #125
Comments
The PerlNavigator README says that Perl::Tidy is used for doing the formatting. So I would have assumed that you'd have to install that module into your Strawberry Perl for it to work... but if you also say that VSCode formatting works, I am confused. (You can test if Perl::Tidy is installed by running
The PerlNavigator docs aren't very clear, but I don't think there are many command-line options. If I run with an invalid option, like If I've understood the README correctly, everything else seems to be done with "settings" (they show some examples of the nested perlnavigator settings in the Neovim and Kate and LiteXL examples). I assume those "settings" are passed as part of the initial handshake or something (though I'm sure you understand that portion better than I do). So if there were an interface in the TOML file to set the internal settings, that would be good -- whether it be through a nested section, like: or whether it be as part of the main section for perl, like ... that would be good, so that users can pass in the options to be able to customize such things. ---- |
|
perl tidy seems to be installed
Yes, settings are used by VSCode via [Trace - 3:24:52 PM] Sending notification 'workspace/didChangeConfiguration'. Params: {
"settings": {
"perlnavigator": {
"perlPath": "perl",
"perlParams": [],
"enableWarnings": true,
"perlimportsLintEnabled": false,
"perlimportsTidyEnabled": false,
"perlimportsProfile": "",
"perltidyProfile": "",
"perltidyEnabled": true,
"perlcriticProfile": "",
"perlcriticEnabled": true,
"perlcriticSeverity": 0,
"perlcriticTheme": "",
"perlcriticExclude": "",
"perlcriticInclude": "",
"perlCompileEnabled": true,
"severity5": "warning",
"severity4": "info",
"severity3": "hint",
"severity2": "hint",
"severity1": "hint",
"includePaths": [],
"includeLib": true,
"logging": true,
"trace": {
"server": "verbose"
}
}
}
}imho, it should work without having to send this but I'll implement it and give it a try. |
|
Hi @Ekopalypse, thanks for trying to get this working with Notepad++. I just double checked on Windows using the latest perlnavigator.exe, and Perl::Tidy worked for me using Strawberry Perl and Sublime Text. The most important thing is getting diagnostics working (e.g. things like One thing you could try is adding perlnavigator.logging = true, you may get some additional logs from the server (logging will be enabled by default in future releases). It should say something like
--stdio is now assumed by default as that's what most LSP clients seem to use (except vscode, which use --node-ipc). It also supports --socket, but I don't know of anyone that uses that option. Do you know if the server is responding with any suggested edits, and then perhaps getting rejected? An issue happened once where Helix was rejecting edits as out of bounds: #37 Also, what version of Perl are you using? |
|
@pryrt, thanks for chiming in. Always happy to get more eyes on the Perl Navigator.
In vscode, you can set
Correct.
Yes, exactly. The node.js portion of the language server can start without any config, and only needs settings to be able to communicate with the Perl interpreter (e.g. for diagnostics and Perl::Tidy). Also, whenever the LSP Client sends updated settings (e.g. via |
|
Hi @bscan, thanks for the quick reply.
I'll give that a try too.
Yes, diagnostics, symbols, goto definiton just works.
I tried that, but I can't find the log file and I don't think one is created.
No, all other things seem to be working. I'm logging at the communication level, and since perlnavigator returns a null result on the request, I assume I have something not set correctly and the server might think the formatting is not supported by my client, but to be honest, I have no idea what that might be, since other servers like pylsp, clangd, rust-analyzer work in this case, but none of them use notifications about workspace configuration changes.
Strawberry 5.38.0 This is what ProcMon sees if I trace perl.exe
The differences are the
By the way, I shortened the path |
|
it doesn't work for me with sublimetext either.
Is there something wrong with my test code? #!/usr/bin/perl
use warnings;
use strict;
use lib '.';
use myModule;
# print "Hello World\n"
sub ThisHereFunction {
sub this_is_another_function {
sub this_is_another_function2 {
}
sub this_is_another_function1 {
}
}
my ( $x, $y, $z ) = @_;
return 0;
}
my $t = ThisHereFunction();
my $t2 = this_is_another_function();
my $t3 = this_is_another_function1();
my $t4 = this_is_another_function2();
|
|
@Ekopalypse, thanks for continuing to work on this. I tested using Strawberry Perl 5.38 in Sublime with that exact code, and both Document Formatting and Range Formatting worked for me, so I'm not sure what the difference could be. The logs you shared are also surprising since it showed
When logging from the navigator, it currently adds additional logging over stderr. For example, when using Sublime Text, you can use the following settings, and there will be additional logging in that LSP Log Panel you showed. If needed, I can also add more verbose logging to the code to see where this is failing. |
|
Hi @Ekopalypse, I see a bug in the code that could be causing this. PerlNavigator/server/src/server.ts Line 400 in 37c9bc4 When the other features (e.g. hover, diagnostics) grab the settings, they call a function for settings that resolves settings including workspace level, fallbacks, etc, and are much more likely to get real settings. This is much more robust. PerlNavigator/server/src/server.ts Line 242 in 37c9bc4 This doesn't exactly explain why we have a mismatch in behaviors between our machines, and between editors, but it could explain why document formatting is less robust. Thanks again for filing the issue. I'll make this change and we can see if it solves the issue. |
|
@bscan, thx for the ongoing investigation.
Maybe it explains that, because after using the configuration you suggested I can also format the code in sublimetext. If I remove there is no formatting, but if I use them, I can format. It still works. Just for my understanding, when |
|
when reading from stderr I get this
I also see the perlcritic error with sublimetext, but the formatting still doesn't work. |
|
Ah, this makes sense now. When the client doesn't send any settings, the server should fall back to the default settings. However, this piece is not implemented for perltidy. This never happens in vscode, because vscode always sends settings (and has defaults specified independently from the defaults in the server). This should be a straightforward fix. Thanks! In the meantime, I may work if you specify perlnavigator settings, similar to how it started working when settings were specified for Sublime. |
I did that but it didn't work. connection.onDocumentRangeFormatting(async (params) => {
console.error("onDocumentRangeFormatting");
console.error("params.textDocument.uri:", params.textDocument.uri);
console.error("documents:", documents);
let document = documents.get(params.textDocument.uri);
let settings = documentSettings.get(params.textDocument.uri);
const workspaceFolders = await getWorkspaceFoldersSafe();
console.error("document", !document);
console.error("settings", !settings);
if (!document || !settings) return;
const editOut: TextEdit[] | undefined = await formatRange(params, document, settings, workspaceFolders, connection);
return editOut;
});and I see that the document is undefined. Do you have any idea why? |
|
This is great, thanks for digging in. I think there's a bug in your debug code: The
Nice. Even better for diving in. Not only do I appreciate the help with the Navigator, I think it's also a useful exercise as the Navigator is very similar to other typescript-based language servers. It's based on the Microsoft sample, which is a common starting point for many language servers. https://github.com/microsoft/vscode-extension-samples/blob/main/lsp-sample/server/src/server.ts |
|
Yes, that change did it. Can I create a perlnavigator binary to test with, or is it enough to test with Update: Update2: |
|
Thanks for fixing! I think testing with node and server.js is sufficient. If you wanted to test sublime, you could use node there as well instead of the binary. using the getDocumentSettings function is also how the official lsp-sample does it, which is a good sign. I think the prior way I was grabbing settings was simply a bug. You could also test your client against the lsp-sample if you wanted. In my view, that's the most authoritative implementation of a language server (although it doesn't do very much). |
|
Just tested with node and it still works
Thx for the link, now that I have node setup I think I will do that :-) |
|
Thanks @Ekopalypse! This should now be working, and is available in version 0.8.9 later. Let me know if you have any other issues or questions about anything. I'm always up for new issues, or even general discussions on the discussions tab. |
|
@bscan, thank you for the quick fix and release build. For the sake of completeness, it should be said that logging is now enabled by default, which I think is intentional. |
I am testing perlnavigator.exe (version 16.16.0.0) on Windows 10 and Notepad++ v8.6.5 (64-bit)
Neither the formatting of the document nor the formatting of the selected text works for me.
Here is an example of the communication between my client and the server
{ "id": 13, "jsonrpc": "2.0", "method": "textDocument/formatting", "params": { "options": { "insertFinalNewline": true, "insertSpaces": false, "tabSize": 4, "trimFinalNewlines": true, "trimTrailingWhitespace": true }, "textDocument": { "uri": "file:///D:/scripts/perl/tests/main.pl" } } } { "id": 9, "jsonrpc": "2.0", "method": "workspace/workspaceFolders" } { "id": 9, "jsonrpc": "2.0", "result": [ { "name": "tests", "uri": "file:///D:/scripts/perl/tests" } ] } { "id": 13, "jsonrpc": "2.0", "result": null }Using the same file with VSCode and formatting works.
When starting the perlnavigator server I see it starts the perl.exe
So I ASSUME that the setup is somehow ok.
Does anyone have any idea what the problem could be?
btw ... are command line parameters other than
--stdiosupported?UPDATE:
I get diagnostics as well as symbols and the goto definition and hovering also works.
Thank you.
The text was updated successfully, but these errors were encountered: