Open savetheclocktower opened 10 months ago
@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, Bar
and unset
below were highlighted like plain text:
$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.
Legacy tree-sitter folding allowed for folding within commented sections. We can no longer do this In the screenshot, hovering over line 30 toggles the folding twistie. Hovering over line 36 does not (it used to)
At some locations, not all, pressing [enter] at the end of a line erroneously indents the cursor on the new line
At some locations, not all, pressing [enter] at the end of a line erroneously indents the cursor on the new line
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 <?php ... ?>
tags?
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.
In the screenshot, hovering over line 30 toggles the folding twistie. Hovering over line 36 does not (it used to)
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.
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.
It's possible. The root language layer is tree-sitter-php
, which identifies and distinguishes the HTML parts from the PHP parts. The HTML grammar is then injected into the HTML parts.
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.
folding a commented-out function is not a feature I can reasonably provide you in the near future.
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.
That's a huge shame. I've found that massively useful for my professional work every since I began using Atom.
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 config.cson
indefinitely and you'll always get the TextMate-style grammar.
The older Tree-sitter grammars are the ones that will be going away, but PHP never even had one of those.
Hello. I'd like to report the followings.
@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 /***/
are handled properly. Thanks for the report!
@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 source.php
scope of PHP regions. Since that grammar has no highlights query — it's a long story! — we incorrectly assumed we could skip scope annotation altogether. But all injection language layers need the opportunity to apply their root scope.
The fix is now present in #859.
/** @var array<array<mixed>> $arrOfArr */ /** @var array<array<string>> $arrOfArrOfStr */ /** @var array<array<object>> $arrOfArrOfObj */
Are these legal PHPDoc constructs?
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 phpstan
knows what @return array<array<array<int>>>
means. There is a fix pending at https://github.com/claytonrcarter/tree-sitter-phpdoc/pull/29, but CI isn't cooperating at the moment.
I pushed this commit about an hour ago. Now each <?php … ?>
range should have the source.php
scope and either a meta.embedded.line.php
scope or a meta.embedded.block.php
scope depending on whether it spans more than one line. This is pretty important if your syntax theme does something special with meta.embedded
, and it's a feature I lean on a lot to be able to tell PHP, EJS, ERB, etc., from their surrounding HTML.
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 meta.embedded
stuff without it. But we'll see!
I had to push a couple of fixes to the new system I described above. I also noticed that tree-sitter-php
has seen some major updates even since I last updated our copy about a month ago, so I brought us in sync with their master branch.
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.
I'm trying out Windows.Pulsar-1.113.2024012105-win
with Use Tree-sitter Parsers
enabled and the php files are gray-text. In dev tools elements are appearing like this:
When I comment the line, it's behaving as html:
When I disabled use of Tree-Sitter Parsers the file appears and behaves as expected. Element is showing:
@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 could not reproduce
the above behavior.
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.
@savetheclocktower I installed on a fresh user profile with no packages installed except what comes with it, and
could not reproduce
the above behavior.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?
* I will next try backing up my app data and settings and doing a fresh install on my user profile. * I will then try to reinstall packages on fresh install
BTW I'm using the folder as a standalone/portable vs installation using the executable.
In fairness, I have noticed issues with hot-swapping CI releases into place where a regular release used to be. I think it's partly related to window state. You can clear window state by launching from the command line (at least on macOS) with the --clear-window-state
flag.
I freely admit that the startup process in Pulsar is a mystery to me, and I'd like to understand it better. But I'm pretty sure “window state” encompasses some things about text-buffer storage that feel magical when they work (like undo history persisting across window launches!) but which have personally (I think) been the cause of very strange performance issues when I upgrade versions.
I notice people deleting/moving their whole .pulsar
directory pretty often, and I think that should be a last resort. If clearing window state doesn't work in the future, try just backing up/deleting the files inside blob-store
, compile-cache
, and storage
before starting over with a fresh profile. I seriously doubt this has to do with any of your community packages.
@savetheclocktower After moving folders and copying back config files and packages, it this particular issue seems to be working fine, thanks for the help!
Minor nitpick I noticed today: I think that these lines in the PHP highlights are too aggressive.
((name) @constant.other.php
(#match? @constant.other.php "^_?[A-Z][A-Z\\d_]+$"))
What I found was that FOO
was scoped as constant.other.php
in all of the following cases:
use FOO\Bar;
use Bar\FOO;
FOO::bar();
Bar::FOO();
class FOO extends Controller {}
I would expect it to only be scoped as a constant in the following cases:
const FOO = 1;
$a = FOO;
bar(FOO);
// etc
Granted, it's pretty uncommon to name classes and namespaces in all caps. I only noticed it when using SDK
as a namespace.
There is a fix pending at https://github.com/claytonrcarter/tree-sitter-phpdoc/pull/29, but CI isn't cooperating at the moment.
This has been merged to master
BTW, in case you feel motivated to fix /** @var array<array<mixed>> $arrOfArr */
et al.
I'll comment out that query for now. I forgot I'd written that. User-defined constants honestly should go into variable
, but with enough detail in the scope name that a theme can choose to color them like other constants if they want.
I didn't even realize the const
statement existed; I was brought up in the define
days. I'll make sure the FOO
in const FOO = 1
is scoped as variable.constant.other.php
or something like that.
This has been merged to
master
BTW, in case you feel motivated to fix/** @var array<array<mixed>> $arrOfArr */
et al.
Thanks! I'll put that on my to-do list also.
@savetheclocktower Sorry for the late reply, I've been busy.
/** @var array<array<mixed>> $arrOfArr */ /** @var array<array<string>> $arrOfArrOfStr */ /** @var array<array<object>> $arrOfArrOfObj */
Are these legal PHPDoc constructs?
I don't know if they're fully legal, but Pulsar 1.112 it's handling them better anyway.
Are you using One Dark?
No, I'm using Dark One Dark
. I also had to reinstall Pulsar 1.112 because I tried to disable the new syntax but I didn't manage to make it. Yes, I followed the open post and restarted Pulsar many times, but the only way to get the old syntax was to downgrade the package.
I also discovered another issue with PHP string heredoc syntax. This is occurring with Pulsar 1.112, but I think it's reproducible also in 1.113.
Take the following sample code:
<?php
class ClassName extends AnotherClass
{
function __construct(argument)
{
// code...
}
public function FunctionName(): string
{
$str = <<<EOT
String in heredoc syntax.
EOT;
return $str;
}
}
And try to collapse the class. I get this:
Updated to 1.113.2024012906 today and PHP/HTML/JS mixed files are still losing their non-PHP syntax decorations at some random point through a session. Freshly opening a new instance starts out fine, once the decorations are lost, they stay lost until Pulsar is shutdown and restarted. This affects individual files only. eg. If I have x2 files open and one breaks, the other is fine. This state persists between opening and closing the file(s) within the session. View->Developer->Reload Window restores the correct syntax decorations. Can confirm that there are no new error messages in the console when the decorations stop working.
My only theory on that — that an unusual parse state causes a failure — must be incorrect, or else there would be a message in the console.
If you're willing to help debug this, I'd like you to install tree-sitter-tools. The next time this happens, you could run the Open Inspector For Editor command and verify:
ERROR
nodes (assuming the buffer contents are valid PHP at that point)source.php
layer in the drop-down menu near the top leftsource.php
layer, you see annotations in the editor that appear to correspond to the areas of your buffer that contain PHPI'd like you to install [tree-sitter-tools]. Done. Ready for the next occurrence but if I run it on a working file, I get this:
That means you're not using a modern Tree-sitter grammar. Are you using one of the configuration overrides that I included in the issue description?
Not using any overrides, just the default installation and config
And neither of those snippets from the issue description is present when you open your config.cson
file?
the phrase "useLegacyTreeSitter" is not present in my config.cson
Then there's some other reason why the modern grammar isn't loading. Try reloading the window and attempting that Open Inspector for Editor command on a PHP file again. If it gives you the same message, look in the console for a message about the grammar failing to load (you can type php
into the box near the top to filter the console output).
It works fine on a pure PHP file. It seems tree-sitter isn't loading on mixed syntax files? But not all mixed syntax files. It seems I have some that work fine.
I'd like you to install [tree-sitter-tools]. Done. Ready for the next occurrence but if I run it on a working file, I get this:
If I add a blank line to the top of this file (above <h2><?=$heading?></h2>
) then Tree-sitter loads on a "Reload Window"
I think I know what's going on.
We define a firstLineRegex
and a contentRegex
to help Pulsar choose a PHP grammar when analyzing the file content, but we probably don't need to do that. The TextMate grammar doesn't do it. And if a file fails those checks, the modern grammar will be ranked lower than the TextMate grammar.
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 grammar-selector
package and uncheck the “Hide Duplicate Text Mate Grammars” setting. Then both the modern grammar and the TextMate grammar will be shown in the list, and you can explicitly choose between them whenever you want. If the modern grammar fails on you, you might even be able to “reset” it more quickly than reloading the window — try switching to the TextMate PHP grammar, then back to the Tree-sitter grammar.
Pushed a commit to remove those patterns.
Inconsistency in comments rendering.
Splitting out the /***
to /* **
corrects the first and last lines but not the code being commented out. Opening and closing the file then corrects the highlighting.
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 tree-sitter-phpdoc
in this commit so that constructs like
/** @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.
So we've just released 1.114. @m1ga has reported an issue in #931 that I'm not able to reproduce myself. If anyone subscribed to this issue can reproduce it, I'd be eternally grateful for your help in troubleshooting it.
v1.114 seems much better and more stable however I came across this problem. While typing the following, the colourisation dropped back down to plain text. After completing the stanza (ie. adding the final ";"), the colourisation came back again.
I just tried to reproduce with something similar and found:
test.php
PHP
correctlyli
in my case - maybe it treats all of your code as text because it starts differently? Maybe a matter of quotes and which lines they are on?With no <?php
it does highlight some, based on what I assume would be templated html-first logic:
With <?php
and no semi-colon:
My file is a *.php like yours but starts with <h2>Contents</h2>
.
Having closed my file and reopened it, I can't get it to reproduce.
Not sure if this error log helps? (there's x3 of the first one)
Uncaught (in promise) Error: Unbalanced content!
at interpret (/opt/Pulsar/resources/app.asar/node_modules/language-php/lib/main.js:35)
at Object.content (/opt/Pulsar/resources/app.asar/node_modules/language-php/lib/main.js:145)
at LanguageLayer._populateInjections (/opt/Pulsar/resources/app.asar/src/wasm-tree-sitter-language-mode.js:4099)
at LanguageLayer._performUpdate (/opt/Pulsar/resources/app.asar/src/wasm-tree-sitter-language-mode.js:3840)
at LanguageLayer.update (/opt/Pulsar/resources/app.asar/src/wasm-tree-sitter-language-mode.js:3445)
at WASMTreeSitterLanguageMode.bufferDidFinishTransaction (/opt/Pulsar/resources/app.asar/src/wasm-tree-sitter-language-mode.js:306)
at TextBuffer.emitDidChangeTextEvent (/opt/Pulsar/resources/app.asar/node_modules/text-buffer/lib/text-buffer.js:2373)
at TextBuffer.transact (/opt/Pulsar/resources/app.asar/node_modules/text-buffer/lib/text-buffer.js:1337)
at TextEditor.transact (/opt/Pulsar/resources/app.asar/src/text-editor.js:2467)
at /opt/Pulsar/resources/app.asar/src/text-editor.js:1799
at TextEditor.mergeSelections (/opt/Pulsar/resources/app.asar/src/text-editor.js:4053)
at TextEditor.mergeIntersectingSelections (/opt/Pulsar/resources/app.asar/src/text-editor.js:4015)
at TextEditor.mutateSelectedText (/opt/Pulsar/resources/app.asar/src/text-editor.js:1798)
at TextEditor.insertText (/opt/Pulsar/resources/app.asar/src/text-editor.js:1747)
at TextEditor.object.<computed> [as insertText] (/opt/Pulsar/resources/app.asar/node_modules/underscore-plus/lib/underscore-plus.js:77)
at TextEditorComponent.didTextInput (/opt/Pulsar/resources/app.asar/src/text-editor-component.js:1845)
/opt/Pulsar/resources/app.asar/node_modules/language-php/lib/main.js:35
Uncaught (in promise) Error: Unbalanced content!
at interpret (/opt/Pulsar/resources/app.asar/node_modules/language-php/lib/main.js:35)
at Object.content (/opt/Pulsar/resources/app.asar/node_modules/language-php/lib/main.js:145)
at LanguageLayer._populateInjections (/opt/Pulsar/resources/app.asar/src/wasm-tree-sitter-language-mode.js:4099)
at LanguageLayer._performUpdate (/opt/Pulsar/resources/app.asar/src/wasm-tree-sitter-language-mode.js:3840)
at LanguageLayer.update (/opt/Pulsar/resources/app.asar/src/wasm-tree-sitter-language-mode.js:3445)
at WASMTreeSitterLanguageMode.bufferDidFinishTransaction (/opt/Pulsar/resources/app.asar/src/wasm-tree-sitter-language-mode.js:306)
at TextBuffer.emitDidChangeTextEvent (/opt/Pulsar/resources/app.asar/node_modules/text-buffer/lib/text-buffer.js:2373)
at TextBuffer.transact (/opt/Pulsar/resources/app.asar/node_modules/text-buffer/lib/text-buffer.js:1337)
at TextEditor.transact (/opt/Pulsar/resources/app.asar/src/text-editor.js:2467)
at BracketMatcher.insertText (/opt/Pulsar/resources/app.asar/node_modules/bracket-matcher/lib/bracket-matcher.js:97)
at TextEditor.object.<computed> [as insertText] (/opt/Pulsar/resources/app.asar/node_modules/underscore-plus/lib/underscore-plus.js:76)
at TextEditorComponent.didTextInput (/opt/Pulsar/resources/app.asar/src/text-editor-component.js:1845)
I got a highlighting bug on a blade file. It's happened twice (2nd time after restarting Pulsar).
To try to reproduce:
abc.blade.php
xyz.blade.php
No highlighting on 2nd file. The first instance had an error:
Uncaught (in promise) TypeError: Cannot read property 'length' of null
at C:\Users\{Redacted}\Portable_Apps\Windows.Pulsar-1.114.0-win\resources\app.asar\node_modules\autocomplete-plus\lib\subsequence-provider.js:105:35
at async Promise.all (index 6)
at async Promise.all (index 2)
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 PHP
. No error occurred in the 2nd instance of bug. After restart the file highlights properly.
I got a highlighting bug on a blade file. It's happened twice (2nd time after restarting Pulsar).
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 tree-sitter
and language-php
is not listed though it is installed.
@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 use
clause should be yellow.
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
: