Formatting document or selected text issue

[Ekopalypse]

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 --stdio supported?

UPDATE: I get diagnostics as well as symbols and the goto definition and hovering also works.

Thank you.

[pryrt]

Does anyone have any idea what the problem could be?

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 perl -MPerl::Tidy -e 1 from the command line; it will give an error if it's not installed, or complete silently if it is installed. Checking: yes, the release notes show that the Perl-Tidy distribution is included with Strawberry Perl, so you should have it already.) So I don't know why it would work in VSCode but not for you. (I don't suppose VSCode makes it as easy as you do to snoop on log the client/server conversation?)

btw ... are command line parameters other than --stdio supported

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 perlnavigator -x, it lists an error including Use arguments of createConnection or set command line parameters: '--node-ipc', '--stdio' or '--socket={number}' -- so --node-ipc or --stdio or --socket={number} are definitely accepted (but since you currently only use STDIN/STDOUT, I don't think you need to worry about them.

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:

[lspservers.perl.settings.perlnavigator]
perlPath = "c:\\strawberry\\perl\\perl.exe"
includePaths = "$workspaceFolder"
perlcriticProfile = "$workspaceFolder\\.perlcriticrc"

or whether it be as part of the main section for perl, like

[lspservers.perl]
mode = "io"
executable = 'c:\\usr\\local\\bin\\perlnavigator.exe'
args = '--stdio'
settings.perlnavigator.perlPath = "c:\\strawberry\\perl\\perl.exe"
...

... that would be good, so that users can pass in the options to be able to customize such things.

---- update: sorry, I replied as if this were an issue in @Ekopalypse's repo for the client he's developing; I didn't notice it was in the PerlNavigator repo until I saw the notification for the reply; I probably wouldn't've brought up the tangential stuff if I'd noticed earlier...

[Ekopalypse]

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. Thx

[bscan]

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 Syntax: Global symbol "$foo" requires explicit package name ). This proves that the language server is successfully communicating with your version of Perl. Sounds like this piece is working fine.

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 Now starting perltidy with: C:\Users\bscan\AppData\Local\Temp\perl-navigatorILXRrE\src\perl\tidyWrapper.pl

btw ... are command line parameters other than --stdio supported?

--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: https://github.com/bscan/PerlNavigator/pull/37 Vscode was more tolerant of code edit boundaries and worked before and after this change. Perhaps the Notepad++ client is even stricter around ranges?

Also, what version of Perl are you using?

[bscan]

@pryrt, thanks for chiming in. Always happy to get more eyes on the Perl Navigator.

I don't suppose VSCode makes it as easy as you do to snoop on log the client/server conversation?

In vscode, you can set "perlnavigator.trace.server": "verbose", and vscode will log much of the communication. The navigator logs plus the trace will show something like this:

[Trace - 10:17:18 AM] Sending request 'textDocument/formatting - (547)'.
Params: {
    "textDocument": {
        "uri": "file:///mnt/d/sandbox.pl"
    },
    "options": {
        "tabSize": 4,
        "insertSpaces": true
    }
}
...
Now starting perltidy with: /home/brian/.vscode-server/extensions/bscan.perlnavigator-0.8.3/server/src/perl/tidyWrapper.pl --profile /home/brian/perltidy_config
[Trace - 10:17:18 AM] Received response 'textDocument/formatting - (547)' in 121ms.
Result: [
    {
        "range": {
            "start": {
                "line": 0,
                "character": 0
            },
            "end": {
                "line": 16,
                "character": 0
            }
        },
        "newText": "# Formatted code here"
    }
]

I don't think there are many command-line options.

Correct. --stdio is assumed by default, so most LSP clients shouldn't need to set any command line options.

If I've understood the README correctly, everything else seems to be done with "settings"

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 onDidChangeConfiguration ), the Perl Navigator will re-evaluate all open files for diagnostics.

[Ekopalypse]

Hi @bscan, thanks for the quick reply.

I just double checked on Windows using the latest perlnavigator.exe, and Perl::Tidy worked for me using Strawberry Perl and Sublime Text.

I'll give that a try too.

... Sounds like this piece is working fine.

Yes, diagnostics, symbols, goto definiton just works.

One thing you could try is adding perlnavigator.logging = true, you may get some

I tried that, but I can't find the log file and I don't think one is created. At least procmon was not able to recognize it. I have found the file that is created when I use VSCode, but no other.

Do you know if the server is responding with any suggested edits, and then perhaps getting rejected?

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.

Also, what version of Perl are you using?

Strawberry 5.38.0

This is what ProcMon sees if I trace perl.exe

The differences are the AppData\local... and the .vscode\extensions... directories and the call to tidyWrapper.pl. But I have also double-checked this and it actually exists in the AppData... path.

By the way, I shortened the path D:\...\perl.exe to fit on my screen, but the full path which is the same for both is D:\ProgramData\Compiler\perl\perl\bin\perl.exe

[Ekopalypse]

@bscan

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();

[bscan]

@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 criticWrapper.pl on both, but tidyWrapper.pl is missing from the Notepad++ one.

I tried that, but I can't find the log file and I don't think one is created. At least procmon was not able to recognize it. I have found the file that is created when I use VSCode, but no other.

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.

{
    "clients": {
        "perlnavigator": {
            "enabled": true,
            "command": ["D:\\Applications\\perlnavigator.exe"],
            "selector": "source.perl",
            "settings": {
                 "perlnavigator.logging": true,
                 "perlnavigator.perlPath": "c:\\perl5_38\\perl\\bin\\perl.exe"
            }
        },
    }
}

If needed, I can also add more verbose logging to the code to see where this is failing.

[bscan]

Hi @Ekopalypse, I see a bug in the code that could be causing this. When the Navigator gets the settings for document formatting, it pulls them from a Map at the individual document level, that should normally contain the settings: https://github.com/bscan/PerlNavigator/blob/37c9bc4749fee0b7305dc2032e16d7e0519a27e0/server/src/server.ts#L400

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. https://github.com/bscan/PerlNavigator/blob/37c9bc4749fee0b7305dc2032e16d7e0519a27e0/server/src/server.ts#L242

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.

[Ekopalypse]

@bscan, thx for the ongoing investigation.

This doesn't exactly explain why we have a mismatch

Maybe it explains that, because after using the configuration you suggested I can also format the code in sublimetext.

If I remove

            "settings": {
                 "perlnavigator.logging": true,
                 "perlnavigator.perlPath": "D:\\ProgramData\\Compiler\\perl\\perl\\bin\\perl.exe"
            }

there is no formatting, but if I use them, I can format. And in fact, it is only necessary to specify one setting for it to work in sublimetext. For example, if you use

            "settings": {
                 "perlnavigator.logging": false
            }

It still works. Now the question remains why it doesn't work when used within Npp. I need to dig deeper.

Just for my understanding, when "perlnavigator.logging": true is set, no additional log file is created, but it is printed to stderr. That explains why I'm not seeing anything as I currently have reading stderr disabled as I'm having trouble using rust-analyzer with it.

[Ekopalypse]

@bscan

when reading from stderr I get this

I also see the perlcritic error with sublimetext, but the formatting still doesn't work. No attempt to call perltidy is logged. I will clone and debug the repo, hopefully I can get to the root of the cause.

[bscan]

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.

[Ekopalypse]

@bscan

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. So I changed the following

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? It's obviously in documents. Sorry if this is a stupid question, but these are my first steps with js and ts.

2024-04-05 17:40:52.843999 [ERROR][ThreadId(1)] - onDocumentRangeFormatting
2024-04-05 17:40:52.844109 [ERROR][ThreadId(1)] - params.textDocument.uri: file:///D:/scripts/perl/tests/main.pl
2024-04-05 17:40:52.848311 [ERROR][ThreadId(1)] - documents: TextDocuments {
2024-04-05 17:40:52.848489 [ERROR][ThreadId(1)] -   _documents: [Object: null prototype] {
2024-04-05 17:40:52.848727 [ERROR][ThreadId(1)] -     'file:///D:/scripts/perl/tests/main.pl': FullTextDocument {
2024-04-05 17:40:52.848896 [ERROR][ThreadId(1)] -       _uri: 'file:///D:/scripts/perl/tests/main.pl',
2024-04-05 17:40:52.849052 [ERROR][ThreadId(1)] -       _languageId: 'perl',
2024-04-05 17:40:52.849238 [ERROR][ThreadId(1)] -       _version: 0,
2024-04-05 17:40:52.849404 [ERROR][ThreadId(1)] -       _content: '#!/usr/bin/perl\r\n' +
2024-04-05 17:40:52.849569 [ERROR][ThreadId(1)] -         '\r\n' +
2024-04-05 17:40:52.849721 [ERROR][ThreadId(1)] -         'use warnings;\r\n' +
2024-04-05 17:40:52.849864 [ERROR][ThreadId(1)] -         'use strict;\r\n' +
2024-04-05 17:40:52.849989 [ERROR][ThreadId(1)] -         "    use lib '.';\r\n" +
2024-04-05 17:40:52.850139 [ERROR][ThreadId(1)] -         '\r\n' +
2024-04-05 17:40:52.850289 [ERROR][ThreadId(1)] -         'use myModule;\r\n' +
2024-04-05 17:40:52.850501 [ERROR][ThreadId(1)] -         '\r\n' +
2024-04-05 17:40:52.850658 [ERROR][ThreadId(1)] -         '# print "Hello World\\n"\r\n' +
2024-04-05 17:40:52.850804 [ERROR][ThreadId(1)] -         '\r\n' +
2024-04-05 17:40:52.850958 [ERROR][ThreadId(1)] -         'sub ThisHereFunction {\r\n' +
2024-04-05 17:40:52.851111 [ERROR][ThreadId(1)] -         '    sub this_is_another_function {\r\n' +
2024-04-05 17:40:52.851243 [ERROR][ThreadId(1)] -         '        sub this_is_another_function2 {\r\n' +
2024-04-05 17:40:52.851362 [ERROR][ThreadId(1)] -         '        }\r\n' +
2024-04-05 17:40:52.851483 [ERROR][ThreadId(1)] -         '        sub this_is_another_function1 {\r\n' +
2024-04-05 17:40:52.851586 [ERROR][ThreadId(1)] -         '        }\r\n' +
2024-04-05 17:40:52.851713 [ERROR][ThreadId(1)] -         '    }\r\n' +
2024-04-05 17:40:52.851814 [ERROR][ThreadId(1)] -         '    my ( $x, $y, $z ) = @_;\r\n' +
2024-04-05 17:40:52.851915 [ERROR][ThreadId(1)] -         '    return 0;\r\n' +
2024-04-05 17:40:52.851997 [ERROR][ThreadId(1)] -         '}\r\n' +
2024-04-05 17:40:52.852073 [ERROR][ThreadId(1)] -         '\r\n' +
2024-04-05 17:40:52.852150 [ERROR][ThreadId(1)] -         'my $t  = ThisHereFunction();\r\n' +
2024-04-05 17:40:52.852369 [ERROR][ThreadId(1)] -         'my $t2 = this_is_another_function();\r\n' +
2024-04-05 17:40:52.852482 [ERROR][ThreadId(1)] -         'my $t3 = this_is_another_function1();\r\n' +
2024-04-05 17:40:52.852606 [ERROR][ThreadId(1)] -         'my $t4 = this_is_another_function2();\r\n',
2024-04-05 17:40:52.852748 [ERROR][ThreadId(1)] -       _lineOffsets: undefined
2024-04-05 17:40:52.852951 [ERROR][ThreadId(1)] -     }
2024-04-05 17:40:52.853100 [ERROR][ThreadId(1)] -   },
2024-04-05 17:40:52.853235 [ERROR][ThreadId(1)] -   _configuration: {
2024-04-05 17:40:52.853312 [ERROR][ThreadId(1)] -     create: [Function: create],
2024-04-05 17:40:52.853389 [ERROR][ThreadId(1)] -     update: [Function: update],
2024-04-05 17:40:52.853465 [ERROR][ThreadId(1)] -     applyEdits: [Function: applyEdits]
2024-04-05 17:40:52.853546 [ERROR][ThreadId(1)] -   },
2024-04-05 17:40:52.853623 [ERROR][ThreadId(1)] -   _onDidChangeContent: Emitter {
2024-04-05 17:40:52.853700 [ERROR][ThreadId(1)] -     _options: undefined,
2024-04-05 17:40:52.853776 [ERROR][ThreadId(1)] -     _event: [Function (anonymous)],
2024-04-05 17:40:52.853853 [ERROR][ThreadId(1)] -     _callbacks: CallbackList { _callbacks: [Array], _contexts: [Array] }
2024-04-05 17:40:52.853936 [ERROR][ThreadId(1)] -   },
2024-04-05 17:40:52.854012 [ERROR][ThreadId(1)] -   _onDidOpen: Emitter {
2024-04-05 17:40:52.854088 [ERROR][ThreadId(1)] -     _options: undefined,
2024-04-05 17:40:52.854168 [ERROR][ThreadId(1)] -     _event: [Function (anonymous)],
2024-04-05 17:40:52.854244 [ERROR][ThreadId(1)] -     _callbacks: CallbackList { _callbacks: [Array], _contexts: [Array] }
2024-04-05 17:40:52.854321 [ERROR][ThreadId(1)] -   },
2024-04-05 17:40:52.854397 [ERROR][ThreadId(1)] -   _onDidClose: Emitter {
2024-04-05 17:40:52.854473 [ERROR][ThreadId(1)] -     _options: undefined,
2024-04-05 17:40:52.854550 [ERROR][ThreadId(1)] -     _event: [Function (anonymous)],
2024-04-05 17:40:52.854626 [ERROR][ThreadId(1)] -     _callbacks: CallbackList { _callbacks: [Array], _contexts: [Array] }
2024-04-05 17:40:52.854703 [ERROR][ThreadId(1)] -   },
2024-04-05 17:40:52.854779 [ERROR][ThreadId(1)] -   _onDidSave: Emitter {
2024-04-05 17:40:52.854855 [ERROR][ThreadId(1)] -     _options: undefined,
2024-04-05 17:40:52.854931 [ERROR][ThreadId(1)] -     _event: [Function (anonymous)],
2024-04-05 17:40:52.855007 [ERROR][ThreadId(1)] -     _callbacks: CallbackList { _callbacks: [Array], _contexts: [Array] }
2024-04-05 17:40:52.855084 [ERROR][ThreadId(1)] -   },
2024-04-05 17:40:52.855163 [ERROR][ThreadId(1)] -   _onWillSave: Emitter { _options: undefined }
2024-04-05 17:40:52.855240 [ERROR][ThreadId(1)] - }
2024-04-05 17:40:52.855329 [ERROR][ThreadId(1)] - document false
2024-04-05 17:40:52.855406 [ERROR][ThreadId(1)] - settings true

[bscan]

This is great, thanks for digging in. I think there's a bug in your debug code:

    console.error("document", !document);
    console.error("settings", !settings);

The ! is negating the object and printing the opposite of what you wanted. So, the document is true and !document is printing false. Similarly, the settings are false and !settings is printing true. Since you're in there, try changing let settings = documentSettings.get(params.textDocument.uri); to let settings = await getDocumentSettings(params.textDocument.uri);

these are my first steps with js and ts.

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

[Ekopalypse]

Yes, that change did it.

 [INFO][ThreadId(4)] - <<< {"id":2,"jsonrpc":"2.0","method":"textDocument/rangeFormatting","params":{"options":{"insertSpaces":true,"tabSize":4},"range":{"end":{"character":16,"line":4},"start":{"character":0,"line":3}},"textDocument":{"uri":"file:///D:/scripts/perl/tests/main.pl"}}}
 [INFO][ThreadId(3)] - >>>  {"jsonrpc":"2.0","id":2,"result":[{"range":{"start":{"line":3,"character":0},"end":{"line":5,"character":0}},"newText":"use strict;\r\nuse lib '.';\r\n"}]}

Can I create a perlnavigator binary to test with, or is it enough to test with node and server\out\server.js ? I just wanted to see if VSCode and sublimetext still work.

Update: Just for the sake of completeness: The change was also made to onDocumentFormatting and now works there too, of course.

Update2: What is still a mystery to me is why it worked for sublimetext. I still have the feeling that my client is doing something unexpected.

[bscan]

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. https://github.com/microsoft/vscode-extension-samples/blob/f4e09a3b7bce2604b93be118f9e006ef808e1ae9/lsp-sample/server/src/server.ts#L164

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).

[Ekopalypse]

Just tested with node and it still works

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).

Thx for the link, now that I have node setup I think I will do that :-)

[bscan]

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.

[Ekopalypse]

@bscan, thank you for the quick fix and release build. I can confirm that the latest perlnavigator.exe works as expected with Notepad++ and sublimetext under Windows x64 with the minimum configuration like

"perlnavigator": {
            "enabled": true,
            "command": ["D:/Repositories/eko/npplspclient/tests/servers/perl/perlnavigator.exe"],
            "selector": "source.perl"
        }

For the sake of completeness, it should be said that logging is now enabled by default, which I think is intentional.