Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Heredoc containing a closing PHP tag breaks syntax highlighting #10

Closed
jaikdean opened this issue Mar 3, 2014 · 17 comments
Closed

Heredoc containing a closing PHP tag breaks syntax highlighting #10

jaikdean opened this issue Mar 3, 2014 · 17 comments
Labels

Comments

@jaikdean
Copy link

jaikdean commented Mar 3, 2014

I have a file containing the following:

        $this->xml = <<<EOD
<?xml version="1.0"?>
<methodCall>
<methodName>{$this->method}</methodName>
<params>

EOD;

The syntax formatting is picking up the ?> within the heredoc as a closing PHP tag, and as such the syntax highlighting is broken from this point on in the file.

screen shot 2014-03-03 at 10 58 40

@troex
Copy link

troex commented May 14, 2014

Confirm on that, it's a bit annoying

@jaikdean
Copy link
Author

This is also happening when there is comment block syntax (/* and */) inside heredocs.

The file in my screenshot is part of the Symfony SensioDistributionBundle, available here.

screen shot 2014-08-26 at 13 45 18

@meecect
Copy link

meecect commented Sep 15, 2014

yep. I see it too.

@TheJosh
Copy link

TheJosh commented Dec 17, 2015

Likewise for single quotes (') or double quotes (") within a heredoc

@zr9
Copy link
Contributor

zr9 commented Jan 19, 2016

There is 2 problems in atom/core probably.
Because grammar ignore "repository" and "injection" rules.

Good news so i'm can fix it by rewriting without that rules, here is simple trial example
atom_issue_3

But i'm need someone to help me write bug about grammar(En is not my native, so i'm sometimes unclear as hell).

And as well i'm need someone who can explain tricky parts in itself heredoc syntax,
like scope "string.unquoted.heredoc.php" now contain in rule double quote
and scope "string.unquoted.heredoc.nowdoc.php" now contain single quote
so i'm need to know diff and probably some examples of different languages(Js,SQL,Json,Regex,CSS) heredoc to test

@Ingramz
Copy link
Contributor

Ingramz commented Jan 19, 2016

@zr9 we've already concluded that it would be worthwhile fixing the root cause of this problem in first-mate package (see related discussion), so that all other bugs would fix on their own. The current solution is rather elegant (although, not working).

But it's still nice seeing someone giving their go at it.

@zr9
Copy link
Contributor

zr9 commented Jan 19, 2016

@Ingramz Oh i'm not read references, so core bug already submitted. Then np :) But current solution is seems like working, so it can be fine as temporally fix until atom fixed "repository"

@Ingramz
Copy link
Contributor

Ingramz commented Jan 19, 2016

@zr9 sure, that's something people have suggested before.

You can compare your solution to the one in the pull request I submitted above. The changes got undone with later commits, but if you keep track of the commits throughout the pull request, you can sort-of see what was done there.

@zr9
Copy link
Contributor

zr9 commented Jan 19, 2016

@Ingramz Yup almost similar, difference in "interpolation" since injection rule is not work, so interpolation need to get moved inside "heredoc_interior" after that it get fully working in current state, because at first heredoc get covered by main rules, then interpolated by php rules inside. Look on screen i'm posted above, or i'm can attach code if you wish so. So technically in current state full fix possible, and it even will be not dirty, only dirty thing is "interpolation" repeat. And more when repository will be fixed in core, it continue to work, so not get broken.

@zr9
Copy link
Contributor

zr9 commented Jan 19, 2016

@Ingramz Tested, looks totally fine, here is screenshots.

heredoc_1
heredoc_2

Except of color issues, but that is atom/core other bug(or not bug).
Scopes for brief look fine too.

@zr9
Copy link
Contributor

zr9 commented Jan 20, 2016

@Ingramz So should i'm push commit to fix this one? Cos there is no seems any activity on repository fix in mate

@Ingramz
Copy link
Contributor

Ingramz commented Jan 20, 2016

@zr9 sure, you can do it.

@zr9
Copy link
Contributor

zr9 commented Jan 20, 2016

@Ingramz Okay then once @50Wliu merge things, i'm clean file, and then update this one

@dv336699
Copy link

@zr9 did you sent a PR? having the same issue here.
Thanks

@zr9
Copy link
Contributor

zr9 commented Jun 13, 2016

@diego-vieira Not yet, have a lot less time now, so slowdown a much, but still have plans to sent PR, or you can do so if you wish to.

@kwladyka
Copy link

screen shot 2016-07-24 at 23 55 18

I can confirm problem still occur.

@winstliu
Copy link
Contributor

Fixed by #184.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

10 participants