pulsar icon indicating copy to clipboard operation
pulsar copied to clipboard

PHP Highlighting broken for "foreach"

Open AzzaAzza69 opened this issue 8 months ago • 12 comments

Thanks in advance for your bug report!

  • [x] Have you reproduced issue in safe mode?
  • [x] Have you used the debugging guide to try to resolve the issue?
  • [x] Have you checked our FAQs to make sure your question isn't answered there?
  • [x] Have you checked to make sure your issue does not already exist?
  • [x] Have you checked you are on the latest release of Pulsar?

What happened?

See notes below: Image

Pulsar version

1.126

Which OS does this happen on?

🐧 Debian based (Linux Mint, Ubuntu, etc.)

OS details

Windows 11

Which CPU architecture are you running this on?

x86_64/AMD64

What steps are needed to reproduce this?

It appears to be the '+' OR the '-' in the array of strings. I've tried changing it to a variable declaration and it still doesn't highlight properly, eg. $a=['-','+']; foreach($a as $sMode)

BUT when I change the character/s to A and B, the highlighting is ok, changing either of them back breaks it again. I have a feeling this has been a problem for a while due to a fellow developer mentioning last year that his highlighting didn't work properly in this project.

Additional Information:

No response

AzzaAzza69 avatar Mar 12 '25 13:03 AzzaAzza69

After finding this: "Go to your settings, search for the grammar-selector package, then click on its settings button. Uncheck “Hide Duplicate Grammars.” Now you'll be able to click on the “PHP” in your bottom status bar and see both PHP grammars in the resulting list. If things don't look right, you can switch to the other grammar and see if that improves things." I can confirm the highlighting DOES work with the "TextMate" grammer.

AzzaAzza69 avatar Mar 12 '25 14:03 AzzaAzza69

Here's the code sample I've prepared based on your screenshot:

<?php

class ClassName extends AnotherClass
{
  
  private function method() {
    // comment
    foreach(['-', '+'] as $sMode){
      
    }    
  }
}

But this looks fine on my machine:

Image

Does it happen in isolation? What if you have a file with nothing but this?

<?php
foreach(['-', '+'] as $sMode){
}

(Be sure to switch back to the “modern Tree-sitter” grammar before testing this.)

I'm 99.9% sure this is a quirk in tree-sitter-php, but I'd love to be able to reproduce it locally so that I can know for sure.

savetheclocktower avatar Mar 12 '25 15:03 savetheclocktower

Nah, things are going sideways on an earlier line in your screenshot. Show me the rest of the line that starts with

$this->q->param('pmrManualType')

and the full line that starts with $sSql above it. (See how $sSql is highlighted like a variable early in the screenshot, but then the next meaningful line doesn't highlight $this the same way? And then the next $sSql is also highlighted like plain text? Something is going wrong elsewhere.)

I don't expect that you can necessarily just send me an entire source file, but if you can reduce this to something that isn't sensitive or proprietary but still exhibits the bug, I can probably fix this (or at least identify it as a bug in tree-sitter-php upstream).

savetheclocktower avatar Mar 12 '25 15:03 savetheclocktower

Its not doing it anymore (I have been working on the file) but the rest of the line is: ... }

					$this->q->param('prmManualType')->setString( (isset($o->manualType) ? $o->manualType : '') );
					if($o->changeMode=='='){
						$sSql.=' WHERE id=:prmId';
						$this->q->param('prmId')->setInteger($o->id);
					}
					$this->q->sql=$sSql;
					$this->q->exec();
				}

Maybe it's the '='?

The code does have various parts where strings go over multiple lines, eg. $this->q->sql="SELECT field1,field2,'constant' AS field3 FROM table WHERE something ORDER BY something ";

AzzaAzza69 avatar Mar 12 '25 19:03 AzzaAzza69

I'd also mention (in case it is something to do with the above), that a yellow "1 Deprecation" has appeared in the status bar...which expands to:

Deprecated calls atom core (1 deprecation) Use customElements.define instead of document.registerElement see https://javascript.info/custom-elements

HTMLDocument.registerElement - file:///C:/Program%20Files/Pulsar/resources/app.asar/static/index.js:100:12 registerElement - C:\Users\azzaazza69.pulsar\packages\git-plus\node_modules\space-pen\lib\space-pen.js:66:88 Builder.openTag - C:\Users\azzaazza69.pulsar\packages\git-plus\node_modules\space-pen\lib\space-pen.js:297:27 Builder.tag - C:\Users\azzaazza69.pulsar\packages\git-plus\node_modules\space-pen\lib\space-pen.js:274:12 Function.div - C:\Users\azzaazza69.pulsar\packages\git-plus\node_modules\space-pen\lib\space-pen.js:83:49 Function.content - C:\Users\azzaazza69.pulsar\packages\git-plus\node_modules\atom-space-pen-views\lib\select-list-view.js:22:19

AzzaAzza69 avatar Mar 12 '25 19:03 AzzaAzza69

I'd also mention (in case it is something to do with the above), that a yellow "1 Deprecation" has appeared in the status bar...which expands to:

Deprecated calls atom core (1 deprecation) Use customElements.define instead of document.registerElement see https://javascript.info/custom-elements

Luckily, this isn't relevant.

So it seems like this is a state that the highlighting can get into but can't necessarily get out of. This is sometimes a thing that happens with Tree-sitter grammars if the document is briefly in an “invalid” state (like while you're typing new code) but doesn't necessarily fix all the stuff that was invalidated later in the document after you bring it back into validity by finishing what you were typing.

The next time you see something like this happening, first make sure it's valid PHP (i.e., finish typing your statement). Then try selecting all your code, cutting it (so that the buffer is briefly empty), then pasting it again. If the highlighting fixes itself when you do this, then that's useful information. Closing and reopening the buffer (if you close all tabs for a particular file) would be another way to “reset” the syntax highlighting.

I'm not suggesting this as a workaround (because that'd be ridiculous) but rather as a way of diagnosing what sort of problem this is.

And if you can reproduce it consistently (even if the reproduction instructions are complicated), that'd obviously be fantastic. But I wouldn't blame you if you didn't feel like it!

savetheclocktower avatar Mar 12 '25 19:03 savetheclocktower

Just had it happen again...have been working on project for 20 mins, opened a .php file (2343 lines), wasn't coloured. Tried Ctrl+A, Ctrl+X, Ctrl+V ; no difference Closed tab, re-opened it ; no difference. Closed all tabs, re-opened the single file ; no difference. Changed Grammer to TextMate ; it did re-colour. Changed back to Modern Tree-sitter ; lost colours.

AzzaAzza69 avatar Mar 14 '25 10:03 AzzaAzza69

Found the Developer tools and switched on console log and can see 2 lines after loading the file: "Failed to expand color preview gutter! preview-gutter.js:67" Got the same messages when opening another file.

I uninstalled the package "https://github.com/Vertagon-Softworks/Chromo" (which contains the preview-gutter.js) and the editor re-coloured. I re-installed the package and the editor lost the colours. Either installing/re-installing resets the enviro enough to get things working again OR a plugin can "break" the syntax highlighting within editor?!

AzzaAzza69 avatar Mar 14 '25 10:03 AzzaAzza69

Maybe I spoke to soon, closed down all editors (without package installed), re-opened and it is still refusing to highlight from the fresh start!

AzzaAzza69 avatar Mar 14 '25 10:03 AzzaAzza69

Found the Developer tools and switched on console log and can see 2 lines after loading the file: "Failed to expand color preview gutter! preview-gutter.js:67" Got the same messages when opening another file.

This is almost certainly a red herring.

You're correct to be looking at the console, but what you want to do is look for new errors when you either

  • switch the grammar from TextMate to Modern Tree-sitter; or
  • select-all/cut/paste.

I regret that this is happening in a very large file that you probably cannot send to me. But this is good news, in a strange way; it suggests that this is a stable state that can be reliably recreated. It's just a matter of finding some non-proprietary sample code that will produce this outcome.

savetheclocktower avatar Mar 15 '25 20:03 savetheclocktower

I kept console open while I switched grammer ; no errors. Same after cut+paste. I can't reproduce it even with the same file (after I closed ALL editors)... It mostly works, it's just sometimes the highlighting breaks and I have to close the editor + re-open. Would you have a function I could call in the console to do some kind of debug/state/dump that might help?

AzzaAzza69 avatar Mar 15 '25 20:03 AzzaAzza69

No, but tree-sitter-tools might help. When there's no highlighting, open the inspector and look for ERROR nodes.

I hate giving this advice because, unless this is a rather small file, the tree will be massive and not very fun to search. (I don't think you can even query on ERROR nodes.) But if you can get this to happen on a small file, you can maybe look through the tree and compare it to what you get when you put the same code into the Tree-sitter playground.

I'm going to make sure tree-sitter-php is updated to latest for the upcoming release just in case the root cause has been identified and fixed recently.

savetheclocktower avatar Mar 15 '25 20:03 savetheclocktower