PerlNavigator icon indicating copy to clipboard operation
PerlNavigator copied to clipboard

First pass at enabling perlimports

Open oalders opened this issue 2 years ago • 5 comments

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.

Screen Shot 2022-10-21 at 17 57 40

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

oalders avatar Oct 21 '22 21:10 oalders

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.

bscan avatar Oct 23 '22 19:10 bscan

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.

oalders avatar Oct 24 '22 19:10 oalders

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

bscan avatar Oct 24 '22 21:10 bscan

On a related note, @gugod's https://metacpan.org/pod/App::PerlNitpick does some things that could also eventually be implemented as fixers.

oalders avatar Oct 24 '22 21:10 oalders

@mm-jpoole could you give this a review?

oalders avatar Oct 27 '22 21:10 oalders

Thanks very much for this @mm-jpoole!

oalders avatar Oct 28 '22 17:10 oalders

@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!

oalders avatar Oct 28 '22 22:10 oalders

Linting unused import: Screenshot 2022-10-28 at 18 57 28

Linting import which needs to be tidied: Screenshot 2022-10-28 at 18 57 44

After formatting: Screenshot 2022-10-28 at 18 58 12

oalders avatar Oct 28 '22 23:10 oalders

@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?

oalders avatar Nov 02 '22 13:11 oalders

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.

bscan avatar Nov 02 '22 14:11 bscan

Hi @oalders, I just updated the docs and pushed the latest version of the Navigator to the VSCode marketplace. Thanks again for the contribution!

bscan avatar Nov 04 '22 00:11 bscan

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.

oalders avatar Nov 04 '22 16:11 oalders