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

PHP syntax highlighting without <?php #14166

Closed
felixfbecker opened this issue Oct 21, 2016 · 10 comments
Closed

PHP syntax highlighting without <?php #14166

felixfbecker opened this issue Oct 21, 2016 · 10 comments
Assignees
Labels
feature-request Request for new features or functionality markdown Markdown support issues *out-of-scope Posted issue is not in scope of VS Code php PHP support issues

Comments

@felixfbecker
Copy link
Contributor

felixfbecker commented Oct 21, 2016

VS Code currently highlights something that has the language ID php only if it is wrapped in <?php tags, and everything outside like HTML. That is wrong in a few ways. The code outside of <?php is not "php" language, it is whatever output you are using. So for a CLI application, code outside would just get printed to STDOUT, while in a webserver it would get send to the client, which means the language is dependend on the Content-Type header (it does not have to be text/html). Only the code inside <?php tags should be considered PHP, the outside can actually be any language, like html or plaintext. As this is not defined anywhere, VS Code should autodetect and fallback to HTML as that is most used. The inside is then "injected" as PHP like JavaScript in a <script/> tag or CSS in a <style/> tag. Also see #1751.

This causes many issues:

  1. PHP in markdown blocks don't get highlighted if they use the ```php tag and do not contain <?php. If you switch to preview or view the file on GitHub or any other editor, everything works fine.
    image
  2. If you eval() to evaluate code (which does not require <?php tags) and the PHP debugger breaks inside that code, VS Code does a sourceRequest. The reported code does not get syntax highlighted. This means the PHP debugger needs to dynamically add this tag, which also means that every breakpoint request, every stackframe location needs to be shifted one line down.
  3. Like many language servers, the PHP LS includes a declaration line in the HoverResponse. This declaration line is only colored if a <?php is included, which I would like to remove because it doesn't make sense: I declared the language to be PHP, so it should be highlighted as PHP. Other clients like Eclipse behave correctly here. Issue: Avoid prepending <?php in hover messages felixfbecker/php-language-server#105
    hover
@felixfbecker
Copy link
Contributor Author

felixfbecker commented Nov 7, 2016

I would like to remove the <?php in the next release of PHPLS because all other clients work correctly here. Is there an approximate ETA for a fix in VS Code? Is it worth to hold back so users don't lose highlighting in VS Code?

@mousetraps
Copy link
Contributor

mousetraps commented Nov 8, 2016

@felixfbecker started looking into this. Quick question, regarding your first bullet point - just to confirm - are you actually getting syntax highlighting for php blocks in the markdown editor (not preview)? Because even when I add <?php, preview properly renders, but the editor does not.

image

@mousetraps mousetraps added this to the November 2016 milestone Nov 8, 2016
@felixfbecker
Copy link
Contributor Author

@mousetraps Yes, correct. The markdown preview is not affected. Thanks for looking into this

@mjbvz
Copy link
Collaborator

mjbvz commented Nov 11, 2016

@felixfbecker, @mousetraps There's another bug tracking php syntax highlighting not working at all in markdown files: #3746

I do not know what is different about the php here. We do properly recognize the fenced code block as a php fenced code block, but the language highlighter never seems to kick in.

@joaomoreno
Copy link
Member

@mousetraps Removing this from January.

@vscodebot
Copy link

vscodebot bot commented Sep 12, 2018

This issue is being closed to keep the number of issues in our inbox on a manageable level, we are closing issues that are not going to be addressed in the foreseeable future: We look at the number of votes the issue has received and the number of duplicate issues filed. More details here. If you disagree and feel that this issue is crucial: We are happy to listen and to reconsider.

If you wonder what we are up to, please see our roadmap and issue reporting guidelines.

Thanks for your understanding and happy coding!

@vscodebot vscodebot bot closed this as completed Sep 12, 2018
@felixfbecker
Copy link
Contributor Author

I do disagree. Guess I’ll just remove the tag then since no other client has this problem.

@roblourens
Copy link
Member

I understand the argument but I don't know that it's likely to change soon.

@aeschli can you help me decide whether this fits into any planned work we have?

@aeschli
Copy link
Contributor

aeschli commented Sep 13, 2018

I also understand the argument. But it looks like markdown worked around this as I see this working. I think it's ok to close, unless there are other issues blocked by this.

@mousetraps
Copy link
Contributor

mousetraps commented Sep 13, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality markdown Markdown support issues *out-of-scope Posted issue is not in scope of VS Code php PHP support issues
Projects
None yet
6 participants