bscan / PerlNavigator

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

Highlighting of bitshift operator with constants #109

Closed maros closed 8 months ago

maros commented 8 months ago

The following code is interpreted as a heredoc, instead of a bitshift left using a constant see Screenshot 2024-01-22 at 19 04 41

use 5.38
use constant SHIFT_LEFT => 4;

my $test = 1234;
my $shifted_test = $test << SHIFT_LEFT;
say $shifted_test;

Valid heredocs in Perl are - a whitespace directly after the << isn't allowed in any case

say <<END; 
say <<'END';
say <<"END";
say <<`END`;
say <<'  END';
say <<~END;

perl-navigator 0.5.1 2023-01-16 vscodium 1.85.1 perl v5.38.0

bscan commented 8 months ago

Hi @maros, thanks for filing an issue, I'm able to reproduce. However, the Navigator doesn't currently override the default Perl syntax highlighting. This is an issue in the base vscode even without using this extension. These types of issues are tracked in the upstream repo here: https://github.com/textmate/perl.tmbundle/issues . The Navigator currently only augments the syntax highlighting by adding additional keywords for Moo, Moose, Object::Pad, with additional TextMate grammars.

However, it is possible to override the built-in highlighting with a replacement grammar, which I've been considering. There is a long list of syntax highlighting bugs that I would love to fix. It appears that the TextMate repo is no longer under active development, so fixing them here would make sense and allow for more rapid iteration. Because of this, I'll leave this ticket open for now.

Thanks!

rabbiveesh commented 8 months ago

Just gonna shamelessly plug that my tree-sitter grammar parses this correctly. I don't know enough about LSPs, but if there's a way to use tree-sitter to help i'm happy to get involved

WhoIsSethDaniel commented 8 months ago

I know nothing about vscode, but the post above made me curious about what vscode does re: highlighting. It seems there is a plugin that allows for highlighting via treesitter. Although it may not be able to use the perl highlighter -- based on my reading of how it works. Too bad. The new perl tree-sitter highlighting works very well.

bscan commented 8 months ago

Thanks @rabbiveesh and @WhoIsSethDaniel! Unfortunately, vscode doesn't natively allow for using tree-sitter. They explored using it in this PR https://github.com/microsoft/vscode/pull/161479 , but decided it wasn't worth it, especially given how many people have been pulled into working on Copilot.

Language Servers do support semantic highlighting where additional information can be sent per token. This is necessary because the TextMate based regexes are fundamentally limited by lack of knowledge about imports and types, plus they're painful to deal with (as I'm sure you know). Quick example from Python where the PyLance LSP is adding highlighting to imports based on if the identifiers are variables, classes, constants, or functions:
image

I'm not opposed to adding Semantic highlighting to the Navigator, but it could be a lot of work and my concern is that vscode would still need updated Textmate grammars anyway. I'm also not sure how the two highlighters will interact, especially when fixing broken scopes instead of simply adding additional semantic information.

Have you guys thought about Github syntax highlighting? This is the other major consumer of the perl TextMate grammars and is similarly broken, except Github/Linguist supports tree-sitter already https://github.com/github-linguist/linguist/blob/master/vendor/README.md . Example:

use 5.38
use constant SHIFT_LEFT => 4;

my $test = 1234; # Correctly highlighted as a comment
my $shifted_test = $test << SHIFT_LEFT;
say $shifted_test; # Incorrectly highlighted as a string
bscan commented 8 months ago

This is now resolved. I copied the textmate grammar into this repository, and set the extension to override the default Perl grammar. The grammar includes a number of bug fixes, including this one. It also fixes most of the outstanding reported bugs in the textmate repo, so moving it upstream at some point may be helpful as well. Available in the Marketplace as 0.8.0.

The grammars are outside of the language server and only applied to vscode.

Before: image

After: image