PerlNavigator
PerlNavigator copied to clipboard
First pass at enabling perlimports
This still needs a lot of work, but I figured I'd create a draft PR to document my progress. perlimports
as a linter is pretty much working, though.
Closes #22.

Tasks
- [x] Enable as linter
- [x] Enable as tidier
- [x] Figure out off-by-one error in line numbering for diagnostics
- [x] Start using config file if configured
Looks great so far and is working on my machine. Let me know how I can help.
I know it's still a draft, but I figured I'd chime in anyway 😁: I believe the off-by-one error is fine. Although vscode presents the line numbers as using one-based indexing, they still have a zero-based index on the backend objects. I needed the same adjustment for perlcritic https://github.com/bscan/PerlNavigator/blob/28867f406a4e33e1e5919a9127f037ef3c3e419a/server/src/diagnostics.ts#L222
It would be nice to allow customizable severity levels for the linter. Currently, the Navigator uses DiagnosticSeverity.Error (red squiggles) only for code that can't compile with perl -c. If a file has any red errors, the entire tab is marked red. It uses DiagnosticSeverity.Warning (yellow squiggles / yellow tab) for perl -c compilation warnings and perl critic severity 5 issues. There are also info and hint levels, which do not change the color of the tab. We can also add this with a slight refactoring of getCriticDiagnosticSeverity
We'll also need to add the settings to the package.json. The double specification of settings is a bit odd, but it's the original Microsoft design from the lsp-sample. Essentially there is one set of defaults for people who use the language server directly (e.g. vim, emacs). Then vscode has another specification and set of defaults to be read by vscode itself and provide a nice GUI settings interface.
This is awesome and I appreciate the work you've done here.
There are also info and hint levels, which do not change the color of the tab. We can also add this with a slight refactoring of getCriticDiagnosticSeverity
Cool. I could see how it would be handy to make the severity level user-configurable. It shouldn't be an error. 😄 I guess I can make them warnings for now.
We'll also need to add the settings to the package.json 👍🏻
Would you know, offhand, if it's possible to offer a quick fix the way things are currently set up? It's saying "no quick fixes available" for the perlimports
linter, but running it as a formatter on the line(s) with the error would fix the warning.
Quick fixes are a type of Code Action but the Navigator does not have any Code Actions setup. I really like the idea though for both perlimports and perlcritic. For example, if someone (especially new perl programmers) didn't use strict
or use warnings
, those should be pretty easy to insert. Perl::Critic::Policy::ValuesAndExpressions::ProhibitLeadingZeros could be paired with an action to turn 0777
into oct(777)
. Even some compiler warnings like defined(@array) is deprecated
could be paired with fixes. I believe refactorings would also count as code actions.
The nice thing about the server being written in typescript is that there are usually open source examples to leverage. An example I found is below:
Some discussion here: https://stackoverflow.com/questions/43328582/how-to-implement-quickfix-via-a-language-server https://github.com/YuanboXue-Amber/endevor-scl-support/blob/master/server/src/CodeActionProvider.ts
On a related note, @gugod's https://metacpan.org/pod/App::PerlNitpick does some things that could also eventually be implemented as fixers.
@mm-jpoole could you give this a review?
Thanks very much for this @mm-jpoole!
@bscan I think this is ready for a review now. Thanks to you and to my colleague @mm-jpoole for the helpful reviews so far!
Linting unused import:
Linting import which needs to be tidied:
After formatting:
@bscan thanks very much for this. I'm not familiar with the release process. I guess people building from source can get the changes now. Will the VSCode users need to wait on a new tag?
Yes, people building from source can run it now. Emacs/vim users often build the server from source. For vscode, the entire extension is packaged in the perlnavigator-0.3.1.vsix
file (I have this file checked into the repo itself, but I should move it to a Github Release). I did the initial testing on my home computer and I just installed it on my $work computer using code --install-extension /path/to/perlnavigator-0.3.1.vsix
. I'm doing some final testing and then I'll just need to push the vsix file to the vscode Marketplace. By default, vscode automatically updates any installed extensions to the latest version, so all current Navigator users would get the updates fairly quickly.
Hi @oalders, I just updated the docs and pushed the latest version of the Navigator to the VSCode marketplace. Thanks again for the contribution!
Thank you so much @bscan for the quick turnaround. It looks great! I will likely need to send another PR when I get the STDIN
stuff figured out, but it already works quite nicely as far as I can tell.