Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing syntax highlighting issue for flexible HEREDOC and NOWDOC in PHP 7.3 #360

Merged
merged 2 commits into from Jul 2, 2019

Conversation

@mohamamdreza1000
Copy link
Contributor

commented Apr 17, 2019

The previous pull request got closed by my mistake as I was trying to force pushing and I couldn't make it open. I'm sorry for that 馃槥. I'm new to GitHub.

Thanks to @wecc, @Ingramz. It is now ready to be merged.

Description of the Change

As already reported in 346 flexible HEREDOC and NOWDOC as introduced in PHP 7.3 will cause syntax highlighting to be failed.

Benefits

We can have syntax highlighting to be working even if we use flexible HEREDOC and NOWDOC.

Possible Drawbacks

The only change which I made was the regex of HEREDOC and NOWDOC endings, so I do not think any particular issue it might cause. As it is most likely how PHP mechanism will detect the end of these two blocks.

So :

$example = foo(
  <<<HEREDOC
  This is just a cool test.
  HEREDOC,
  $bar
);
$rest_of_the_code = 'hello';

Should now have syntax highlighting to be worked properly.

@rsese

This comment has been minimized.

Copy link
Member

commented Apr 17, 2019

Thanks @mohamamdreza1000! Someone from the team will take a look as soon as they can,

And thanks @Ingramz for your help moving this forward 馃檱

@rsese rsese added the triaged label Apr 17, 2019

@mrliptontea

This comment has been minimized.

Copy link

commented Jun 7, 2019

Hi,

Any chance to move this forward soon?

I tested this with VS Code and worked perfectly 馃憣

@GoldenJoe

This comment has been minimized.

Copy link

commented Jun 8, 2019

Seconded, fixing this bug would make HEREDOC so much easier.

@rsese

This comment has been minimized.

Copy link
Member

commented Jun 10, 2019

This is on the team's list of items to review but we can't promise any specific timeframe for when it will be reviewed - but I'll mention to the other maintainers that folks checked in about this PR.

@as-cii as-cii self-assigned this Jul 2, 2019

@as-cii

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2019

I am not super familar with PHP, but this looks reasonable.

Thanks @mohamamdreza1000 and @Ingramz for pushing this forward! 鈿★笍

@as-cii as-cii merged commit 6fc92ce into atom:master Jul 2, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@JulianHQ JulianHQ referenced this pull request Aug 14, 2019
1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can鈥檛 perform that action at this time.