-
-
Notifications
You must be signed in to change notification settings - Fork 139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MEGA-ISSUE: Syntax highlighting in language-php
#876
Comments
@croaker000 reports occasional failures with injection of HTML into PHP files. We're working to establish reproduction steps. If you can get this issue to reproduce consistently, please add a comment and tell us how. |
A user on Discord reported that constructors and some builtin functions were no longer highlighted like functions. For instance, $foo = new Bar();
unset($baz); This has been fixed in #859, specifically in this commit. At the latest it'll be released as part of 1.114, but we might be able to get it out sooner as part of a rolling release. If so, it'll be announced in this ticket. |
I can confirm this on the latest #859. @croaker000 is there any chance that line 4 in your example using a ternary? I am seeing: # indent is wrong
$id = (foo(1) ?: 1);
$id = (foo(1) ? 1 : 2);
$id = [(foo(1) ?: 1)];
$id = 1 ? 1 : 1;
# indent seems fine
$id = (foo(1));
$id = (foo(1) ?? 2);
$id = []; Re the highlighting issue you saw w/ HTML+PHP templates, I have not seen that happen yet, personally. I spent all day yesterday in such templates w/ the new parser and they always behaved fine for me. I'll keep an eye out for it, too, though. Does it just happen "on it's own", or is it after you've altered the buffer or added any new code? Does removing any code seem to fix it? If I'm reading between the lines of @savetheclocktower comment, it could be that something in your file is triggering an uncommon code path in the bundled PHP parser, and that might be using something that wasn't accounted for when the parser was bundled in Pulsar. So if it only happens when you use certain syntax or something like that, that may be the case. Also, @savetheclocktower could the issue be in the HTML parser and not in PHP? In the posted screenshot, the 2 chunks of PHP appear to be highlighted correctly. It's just the HTML in between that isn't. Perhaps the injection is working fine, but the HTML parser is falling over when trying to deal with that part of the document? @croaker000 is all of the HTML in the whole file losing hightlights, or just certain blocks of it between |
Yes, I can duplicate the indenting problem with ternary lines and seems OK for other lines. HTML/PHP mixed files have been working fine for me today. Yesterday (on a different set of similar files) was a nightmare until I switched back to the legacy tree-sitter. It seemed to happen randomly and become visible at the point of opening a file. After which, all other open files displayed the same problem. I wasn't in a debug mindset at the time so it's tricky to be definitive. I'm waiting for the problem to crop up again so I can try to duplicate it. Yes, all the HTML in an affected document loses highlighting but snippets and code folding stop working at the same time for all code types. |
That is ridiculous and I had no idea it was even present. I imagine that we could probably eventually detect a span of consecutive line comments and convert them into a single fold, but folding a commented-out function is not a feature I can reasonably provide you in the near future. Meanwhile, those ternary test cases are useful. I can have that indentation problem fixed shortly. |
Indentation fix now present in #859.
It's possible. The root language layer is So the PHP parser could be screwing up at marking the HTML nodes as HTML nodes, or the HTML grammar could be screwing up at parsing the HTML parts as HTML. Or it could be a bug in the injection handling. For instance, if we fail to determine the correct range(s) for the injection and fall victim to an off-by-one error, it could throw off the HTML parser entirely. |
That's a huge shame. I've found that massively useful for my professional work every since I began using Atom. |
Hello. I'd like to report the followings. Sample code<?php
/**/
/***/
/** */
/****/
/** @var string $str */
/** @var array $arr */
/** @var array<mixed> $arr */
/** @var array<string> $arrOfStr */
/** @var array<object> $arrOfObj */
/** @var array<array> $arrOfArr */
/** @var array<array<mixed>> $arrOfArr */
/** @var array<array<string>> $arrOfArrOfStr */
/** @var array<array<object>> $arrOfArrOfObj */
/** @var object $obj */
/** @var stdClass $obj */
intdiv(num1: $dvdnd, num2: $dvsr);
str_pad(string: $str, length: 10, pad_string: "0");
ksort(array: $arr, flags: SORT_REGULAR); |
The good news is that this feature isn't going away; it's just not something we can do in a Tree-sitter grammar. The existing PHP TextMate-style grammar that you've used for years will still be able to do this. Keep the setting override in your The older Tree-sitter grammars are the ones that will be going away, but PHP never even had one of those. |
@claytonrcarter, these lines cause errors in the PHPDoc parsing: /** @var array<array<mixed>> $arrOfArr */
/** @var array<array<string>> $arrOfArrOfStr */
/** @var array<array<object>> $arrOfArrOfObj */ Are these legal PHPDoc constructs? Meanwhile, @Digitalone1, we'll make sure that the PHPDoc syntax is changed to look much more like what it used to be, and that comment fragments like |
@Digitalone1, this commit should fix most of what you're seeing in that code example. However, I wasn't able to reproduce the lack of highlighting of variables in the PHPDoc snippet: Are you using One Dark? If so, can you put the cursor in the middle of one of those gray variables, then open the command palette (Ctrl-Shift-P or Cmd-Shift-P and run the Editor: Log Cursor Scope command? Please copy the text of the notification that appears, or else take a screenshot, and show it to us. |
@garrettboone reported in #887 that the PHP grammar is using HTML comment delimiters inside of PHP sections. This happened because we weren't annotating any parts of the buffer with the root The fix is now present in #859. |
This only adds support for "list" array types, though. We can add support for more variations as requests come in. Ref: pulsar-edit/pulsar#876 (comment)
Short story: no. Long story: yes. As I recall, they are not valid phpdoc in the literal sense of phpdocumentor, but – since these are all just comments anyway – static analysis tools have "extended" phpdoc to allow for things like this. I just checked, and |
This only adds support for "list" array types, though. We can add support for more variations as requests come in. Ref: pulsar-edit/pulsar#876 (comment)
I pushed this commit about an hour ago. Now each All the tests pass, but of course the language bundles need more test coverage, so let me know if you run into any situations where your PHP editing experience goes sideways. This would match some of the symptoms already reported: some part of the document, typically the HTML, would suddenly lose syntax highlighting. If it happens to you, please try to notice what you were doing when it happened so I can try to replicate it myself. I'm pretty confident about what I've done here; I think it's no more likely to fail than the previous approach, and I couldn't have delivered the |
This only adds support for "list" array types, though. We can add support for more variations as requests come in. Ref: pulsar-edit/pulsar#876 (comment)
I had to push a couple of fixes to the new system I described above. I also noticed that There's now a “php-only” version of the grammar which we could theoretically be using on the injected PHP layer instead of re-injecting the top-level PHP parser (the one that also identifies HTML), but I can't think of a reason to do this. At least it's there should we ever find a use for it. |
@garrettboone, give the latest build a shot — the one on this page. Apologies; PHP wasn't working on CI builds for most of Sunday because of an error I made. |
Using 1.113.2024012204 the behavior was the same (with packages running). I decided to investigate further with safe mode using a local file.
|
OK, I'll take a look in a few hours with the latest CI build, using my own WordPress project as sample code. |
I'm having no luck reproducing those symptoms on a CI build on my Mac, so I might try tomorrow on my seldom-used Windows machine. |
@savetheclocktower I installed on a fresh user profile with no packages installed except what comes with it, and I am concluding it must either be something about my user profile and environment, or packages installed. But would an installed package affect safe mode? From a user updating standpoint, I would like to help pinpoint what the issue is. Any suggested steps?
BTW I'm using the folder as a standalone/portable vs installation using the executable. |
It works fine on a pure PHP file. It seems tree-sitter isn't loading on mixed syntax files? |
I think I know what's going on. We define a I'll fix this. In the meantime, you can click on the “PHP” in your status bar and explicitly choose the “PHP” option when the menu appears. On default settings, there will be only one in the list, and it will be the modern PHP grammar. For now, you may find it useful to go into the settings for the |
Pushed a commit to remove those patterns. |
Addresssed! #906 is the new rolling PR now that #859 has landed. (By the way, grab a rolling release if you want all the fixes from #859.) Thanks for the report! |
@Digitalone1 I updated @claytonrcarter’s version of /** @var array<array<mixed>> $arrOfArr */ should now be parsed better. That didn't make it into the most recent rolling release, but it's now in #906. |
I just tried to reproduce with something similar and found:
With no |
My file is a *.php like yours but starts with Not sure if this error log helps? (there's x3 of the first one)
|
I got a highlighting bug on a blade file. It's happened twice (2nd time after restarting Pulsar). To try to reproduce:
No highlighting on 2nd file. The first instance had an error:
File content: <!DOCTYPE html>
<html lang="{{ str_replace('_', '-', app()->getLocale()) }}">
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<meta name="csrf-token" content="{{ csrf_token() }}">
</head>
<body>
<table>
<tr>
<th>Series Instance (ordered)</th><th>SBS</th><th>1aat</th>
</tr>
@foreach($participant_array as $set => $array)
<tr>
<td>{{ $set }}</td><td>{{ $array['sbs'] }}</td><td>{{ $array['1aat'] }}</td>
</tr>
@endforeach
</table>
</body>
</html> The grammar selector is properly selecting as |
Actually, it seems any blade file, when opened, is not getting highlighted. v1.114.0 (windows) Not sure if this is related, but I was looking in the LocalStorage and filtered |
@savetheclocktower Pulsar does not highlight types for class constants. PHP 8.3 supports typehint for class constants: https://stitcher.io/blog/new-in-php-83#typed-class-constants-rfc Update: It happens also for traits. And the trait name in |
@savetheclocktower As reported on discord, the code that trigger the old grammar: <?php
trait TraitName
{
use Trait1, Trait2, Trait3, Trait4;
private bool $flag = false;
public string $v1 = "A string...";
public string $v2 = "A string...";
public string $v3 = "A string...";
public const string CONST1 = "A string...";
public const string CONST2 = "A string...";
public const string CONST3 = "A string...";
public function function(): string
{
return <<<EOT
A string...
EOT;
}
} |
OK, @Digitalone1 and others, I've got good news. First, I should've noticed long ago that the TextMate grammar for PHP had a This should result in the new grammar being picked consistently in nearly all cases. This'll land in 1.118 in just under a month; once it does, please let me know if there are other files that don't pick the new grammar when they're opened. As for the type annotations on class constants: the underlying Tree-sitter parser was erroring on those constructs, so I checked the latest Both fixes are present in #1010 and will go out in the 1.118 release. Thanks for the reports! |
I suppose trait names in |
They'd be yellow on definition, but not on mere mention. If you want them to be yellow in all usages, you can add this to your user stylesheet: @import "syntax-variables";
.syntax--support.syntax--trait {
color: @syntax-color-function;
} |
Unfortunately I'm still having issues on PHP files with the Tree-sitter grammar. I can reproduce the issue writing a simple comment, but only in a file of my project which I can't share. Specifically, the syntax gets disabled and the text is all white. Unfortunately I can't reproduce it elsewhere. That does not happen with TextMate grammar. Anyway when the issue occurs, I get the following error in the console.
The filename is |
v1.119.0 has turned this somewhat intermittent bug into something that happens every time: After opening a project and opening a PHP file that has not previously been open in v1.119, there is no syntax highlighting. View->Developer->Reload Windows will fix this. Once highlighted, the code folding fails to work; the correct twisties appear, but don't do anything. A "CTRL-ALT-SHIFT-[" will fold everything as expected, after which code-folding works fine. After these procedures, all works fine. Closing and opening the file works fine. Restarting Pulsar and opening the file again works fine. Clearing the cache and storage folders in ~/.pulsar makes no difference. |
Is this happening despite an override in your If it's the latter, I sincerely appreciate it, since I can't deliver you the ability to fold commented-out functions. If it's the former — if you're trying to opt into the old grammar, but having to do this workaround every time — then that's a more serious bug. Just so I understand correctly:
Do I have all this right? This is the mother of all weirdnesses. There are some weak points in the new PHP grammar — I had to do some strange things to divide files up into PHP parts and non-PHP parts for highlighting purposes — but your symptoms are pretty crazy. You're already going above and beyond just by reporting this to me, but if you felt like making a screencast to illustrate this process, I'd be incredibly grateful. I could try to reproduce it step-by-step on all three platforms. If not, no worries. |
Thanks for looking.
Try this screencast: |
When it first comes up as unhighlighted code, does the status bar show “PHP”? If you use the grammar selector to switch to another grammar and then back to PHP, does that have the same effect as reloading the window? Does anything show up in the dev tools console? |
Yes, it shows PHP, see before/after screenshot Changing the grammar changes the syntax highlighting in some cases ("C" does something useful for example) but swapping back to PHP restores the unhighlighted state. Opening the unhighlighted PHP file (or any other file come to that) doesn't show any new Console debugging information. NB: My x3 deprecations in the screenshot are "todo-show" and "tool-bar" |
@croaker000, I keep typing theories into this box and then deleting them because I think of reasons they can't possibly be true. If this were happening to a file that you already had open at startup from a previous session, then I could chalk it up to a race condition. If it had to do with grammar heuristics putting you into the wrong PHP grammar, you'd still see some highlighting even if it's not the right PHP grammar between the two that are present. I'm glad that reloading the window helps, but I'm wondering if you can achieve the same effect just by closing and re-opening the offending buffer. I regret that I can't reproduce this issue, but I'm open to any other information you can give me about it in case other clues reveal themselves. |
My projects all sit on an NFS share. That's about the only oddity I can think of. |
IMPORTANT: Some issues have already been fixed!
If you’re still on the regular 1.113 release, you might be suffering from a problem that has already been fixed. Many fixes landed on
master
in #859. You are encouraged to download the latest rolling release — both (a) to see whether what you’re reporting has been fixed, and (b) so that you can enjoy the fixes that have been reported since the 1.113 release.This will serve as a catch-all issue for any problems you may encounter with syntax highlighting in PHP files (
language-php
). If you have problems with the new syntax highlighting (or folds, or indents) that are specific tophp
files, keep reading.Something isn't highlighting correctly!
If there are any highlighting regressions since 1.113:
First, please scroll down and see if someone else has reported the issue. If not, please comment with
I want to go back to the old highlighting!
You can easily opt out of the new Tree-sitter highlighting for any language, but first please:
To revert to the TextMate-style grammar for this language only, add the following to your
config.cson
:The text was updated successfully, but these errors were encountered: