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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong variable color in PHPStorm #151

Closed
mariojackson opened this issue Apr 24, 2020 · 7 comments · Fixed by #121 or #153
Closed

Wrong variable color in PHPStorm #151

mariojackson opened this issue Apr 24, 2020 · 7 comments · Fixed by #121 or #153

Comments

@mariojackson
Copy link

mariojackson commented Apr 24, 2020

Hi Sven!

It seems like the default color for a variable in PHP differs from other languages.

The color in the picture seen below is most probably the default color of Jetbrains its own Darcula theme.

Screenshot 2020-04-24 at 13 50 25

I just made a fresh installation of the current PHPStorm 2020.1 along with the Nord plugin today.

It seems that by default the inheritance in PHP for variables is disabled. When enabling manually or just enabling and then disabling again, the color for the variable correctly turns to white and stays like that.

Edit:
This is also true for constants and parameters.

@arcticicestudio
Copy link
Contributor

Hi Mario 👋, thanks again for another contribution 👍

Looks like this is also related to #120 (#121) again. The theme must be changed to use explicit color definitions instead of relaying on color inheritances for languages that are especially bundled with PhpStorm. I'll quickly install a trial version and check which syntax highlighting plugins require additional support.

arcticicestudio added a commit that referenced this issue Apr 27, 2020
PhpStorm [1] ships with support for specific languages, frameworks and
libraries for PHP development and of course advanced highlighting for
PHP. Due to the same problems documented in GH-120 (and mitigated in
GH-121) some syntax theme keys required to be replaced with explicit
definitions instead of reyling on color inheritances.

Therefore this commit adds explicit support for PhpStorm's bundled
language syntax:

1. Main support for "PHP" [2][3]
2. Laravel "Blade" Temapltes [4][5][6]
3. "Twig" template engine [7][8][9]
4. "Smarty" templates [10][11]

[1]: https://www.jetbrains.com/phpstorm
[2]: https://www.php.net
[3]: https://plugins.jetbrains.com/plugin/6610-php
[4]: https://laravel.com/docs/7.x/blade
[5]: https://www.jetbrains.com/help/phpstorm/blade-page.html
[6]: https://plugins.jetbrains.com/plugin/7569-blade
[7]: https://twig.symfony.com
[8]: https://www.jetbrains.com/help/phpstorm/symfony-twig.html
[9]: https://plugins.jetbrains.com/plugin/7303-twig
[10]: https://www.smarty.net
[11]: https://www.jetbrains.com/help/phpstorm/smarty.html

Related to GH-120,GH-121
GH-151

Co-authored-by: Sven Greb <development@svengreb.de>
@arcticicestudio
Copy link
Contributor

arcticicestudio commented Apr 27, 2020

I've submitted #153, let me know if there are styles you might think that can be improved. Since I've never used PHP it's possible that I've "misinterpreted" some of the syntax elements, e.g. I'm not sure if the Query(...) on lines 38-40 are constructors or class instantiators. Also on line 37 there's $$def which the syntax plugin calls a "Variable variable" and on line 35 a "magic" attribute (some kind of special class field?).

@mariojackson
Copy link
Author

mariojackson commented Apr 27, 2020

You always inspire me with your quality of work, great documentation and thoughtfulness. I already started to make better git messages myself since reading yours, by including a title and description instead of writing only a short one-liner :).

I'm also not working that often with PHP nowadays but the above picture does look good to me. The colors are adjusted to be similar like in Go for example.

  • The new Query(...) part is the call of a constructor
  • Line 37 is a variable variable name. I think it's whether confusing and only saw it very seldom in real projects - and where I saw them were pretty old ones. You can basically change the name of a variable dynamically.
  • Line 35: Magic methods are methods which are pre-occupied by PHP like __toString()

Thank you for fixing the problem so quickly Sven!

@arcticicestudio
Copy link
Contributor

arcticicestudio commented Apr 27, 2020

Thanks for the compliments 😊
I see it that way, the history and changes of a Git repository should tell the project's story and should also be the main persistence layer for it. The probability is very low for GitHub (and that would also be a disaster), but when there are some problems with the database and issues/PRs are lost, some parts of the story of a project are also gone. There have been many projects, including really large and popular ones, whose repositor(y/ies) were suddenly gone (reasons like argues between maintainers, too much stress etc.) and so all documentation through issues/PRs was gone (when GitHub popup warns before deleting a repository they really mean it that way regarding the "permanent data loss" section). Writing the project's change reasons right into commits ensures the story is safe: even if you deleted your local repository, someone else has a clone of it like e.g. in my case a simple service running on my private servers that mirrors repositories.
Only part I miss is a way to include data like images in commits (base64 might kill it 😄) without the need to commit them right into the repository. Also if you like the idea maybe you like to check out https://github.com/MichaelMure/git-bug, a fantastic project of an “Distributed, offline-first bug tracker embedded in Git, with bridges“ (written in Go 💙)

Anyway, drifted away from the topic again 😃
I'll merge the PR and review some other PRs and issues that might also be included in the next release version before deploying it to the JetBrains plugin registry.
Thanks again for your feedback, also learned something about PHP again (tried their docs, but in my opinion they need a full refactor).

@mariojackson
Copy link
Author

Thank you for the git-bug suggestion, it seems pretty interesting and since it has a terminal UI and I'm a terminal guy and since it's written in Go, it seems even more interesting to me. Will definitely check it out :).

arcticicestudio added a commit that referenced this issue Apr 28, 2020
Add explicit support for bundled PhpStorm language syntax

PhpStorm [1] ships with support for specific languages, frameworks and
libraries for PHP development and of course advanced highlighting for
PHP. Due to the same problems documented in GH-120 (and mitigated in
GH-121) some syntax theme keys required to be replaced with explicit
definitions instead of relying on color inheritances.

Therefore this commit adds explicit support for PhpStorm's bundled
language syntax:

1. Main support for "PHP" [2] and the official JetBrains plugin [3]
2. Laravel "Blade" Templates [4]
   See JetBrains official "Blade" documentation [5] and the plugin [6]
   for more details.
3. "Twig" template engine [7]
   See JetBrains official "Twig" documentation [8] and the plugin [9]
   for more details.
4. "Smarty" templates [10]
   See JetBrains official "Smarty" documentation [11] for more details.

[1]: https://www.jetbrains.com/phpstorm
[2]: https://www.php.net
[3]: https://plugins.jetbrains.com/plugin/6610-php
[4]: https://laravel.com/docs/7.x/blade
[5]: https://www.jetbrains.com/help/phpstorm/blade-page.html
[6]: https://plugins.jetbrains.com/plugin/7569-blade
[7]: https://twig.symfony.com
[8]: https://www.jetbrains.com/help/phpstorm/symfony-twig.html
[9]: https://plugins.jetbrains.com/plugin/7303-twig
[10]: https://www.smarty.net
[11]: https://www.jetbrains.com/help/phpstorm/smarty.html

Related to GH-120, GH-121
Closes GH-151

Co-authored-by: Sven Greb <development@svengreb.de>
@arcticicestudio arcticicestudio removed their assignment Apr 28, 2020
arcticicestudio added a commit that referenced this issue Sep 22, 2020
Add explicit support for bundled PhpStorm language syntax

PhpStorm [1] ships with support for specific languages, frameworks and
libraries for PHP development and of course advanced highlighting for
PHP. Due to the same problems documented in GH-120 (and mitigated in
GH-121) some syntax theme keys required to be replaced with explicit
definitions instead of relying on color inheritances.

Therefore this commit adds explicit support for PhpStorm's bundled
language syntax:

1. Main support for "PHP" [2] and the official JetBrains plugin [3]
2. Laravel "Blade" Templates [4]
   See JetBrains official "Blade" documentation [5] and the plugin [6]
   for more details.
3. "Twig" template engine [7]
   See JetBrains official "Twig" documentation [8] and the plugin [9]
   for more details.
4. "Smarty" templates [10]
   See JetBrains official "Smarty" documentation [11] for more details.

[1]: https://www.jetbrains.com/phpstorm
[2]: https://www.php.net
[3]: https://plugins.jetbrains.com/plugin/6610-php
[4]: https://laravel.com/docs/7.x/blade
[5]: https://www.jetbrains.com/help/phpstorm/blade-page.html
[6]: https://plugins.jetbrains.com/plugin/7569-blade
[7]: https://twig.symfony.com
[8]: https://www.jetbrains.com/help/phpstorm/symfony-twig.html
[9]: https://plugins.jetbrains.com/plugin/7303-twig
[10]: https://www.smarty.net
[11]: https://www.jetbrains.com/help/phpstorm/smarty.html

Related to GH-120, GH-121
Closes GH-151

Co-authored-by: Sven Greb <development@svengreb.de>
@arcticicestudio
Copy link
Contributor

@mariojackson The change is now available in the freshly shipped version 0.12.0 that is also shipped to the JetBrains Plugin Registry. The update is currently processed and should be available within the next 24h from Nord's plugin page.

@mariojackson
Copy link
Author

Nice! Thank you Sven!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment