PSR-2 class brace syntax highlighting error

[jesseleite]

language-php not properly highlighting opening curly brace when following PSR-2 convention (opening curly brace on it's own line). I tried looking through the grammar file, but I can't make sense of it to PR.

See line 13 in this example:

[winstliu]

I'll see if I can take a look at this.

[winstliu]

Hmm...I can't reproduce this using the following example:

<?php 

class NotPsrTwo {

    public function name()
    {
        //
    }

}

class PsrTwoFormatted
{
    public function name()
    {
        # code...
    }
}

[Ingramz]

@50Wliu make sure you are using "One Dark" syntax theme

[winstliu]

Woah, that's super weird. I can reproduce now and I think I see the problem as well. Thanks.

[jesseleite]

I didn't realize it was only visible on certain syntax themes. Is the issue with language-php, or with the theme?

[winstliu]

language-php. In the second case the { is included under the meta.class.php scope while in the first case it isn't.

[winstliu]

The crazy thing about this is that the scope logger seems to be ignoring meta.class.php - it's only visible if you actually inspect the element. The regex also seems to be structurally sound...(?=[;{]).

Not even the specs are catching this! I really don't know what to do at this point.

Dev Tools: but

Test used:

describe 'classes', ->
    it 'tokenizes them', ->
      tokens = grammar.tokenizeLines """
        <?php
        class PsrTwoFormatted
        {
        }
      """

      expect(tokens[2][0]).toEqual value: '{', scopes: ['text.html.php', 'meta.embedded.block.php', 'source.php', 'punctuation.section.scope.begin.php']

[jesseleite]

I really don't know either. This is all a bit over my head. Could it have anything to do with lack of multi-line m regex flag or something?

Ha, it's just one character... but sucks because PSR-2 is used by Symfony, Laravel, etc. so this is a very common convention in the PHP world these days.

[winstliu]

I think m is enabled by default, or at least I haven't had to mess with it when doing multiline stuff in Atom. I'll try it anyway though tomorrow morning.

[jesseleite]

Thanks for your time on this.

[Ingramz]

I took a look at this as well. I could be totally wrong, but my theory is that the issue is actually with the way how lookahead behaves right now (from 'end': '(?=[;{])'). It seems like if the characters we are looking for: ; or { are the first characters on the line, then we are not exiting the current scope meta.class.php, but we continue our work of highlighting in a manner that we exited the scope. This doesn't leak to following lines because we're past the lookahead and some of the exiting is done automatically in the end of line. Put any character in front of the { or ; and it behaves correctly.

I'll try to whip up a minimal possible grammar to see if this exists there as well, if that has the same issue, it is most likely an issue with first-mate and not with the grammar or theme.

An example

<?php 
class Someclass
garbage 1 23 4 5 $asd @£$€!"¤%&/()"''
{public function something(){echo 1;}}

echo "hi";

[Ingramz]

Opened atom/first-mate#56

[Ingramz]

Once atom/atom#8703 from atom/atom#8701 gets merged, this issue will solve

[winstliu]

This will be fixed in the next Atom version. Thanks to @JesseLeite for reporting and @Ingramz for the excellent analysis :100:.

[jesseleite]

Beauty, thank you both.