Avoid prepending <?php in hover messages

[kaloyan-raev]

At the moment the first MarkedString in hover messages is of language "php". According to LSP specification the client should render this as if it was a markdown string like:

```php
php code block
```

The language server prepends <?php to ensure that the VSCode colorize the PHP code.

I haven't seen any markdown editor to require wrapping PHP code in PHP tags to colorize it. It seems to be a specific implementation detail in VSCode. For all other clients this <?php tag does not bring any information to users, it just stands there like a buggy code hickup.

I propose one of the following solutions:

• Don't prepend <?php at all and report the issue to VSCode.
• Prepend <?php only if the client is VSCode.

[felixfbecker]

If you think about it, VS Code is correct here. Any code outside of <?php tags in PHP is HTML, and should be colorized like HTML. I met this issue before in my debug adapter. The only semantically-correct solution would be to do have a separate language ID for the PHP code, like phpcode.

[kaloyan-raev]

Any code outside of <?php tags in PHP is HTML, and should be colorized like HTML.

Exactly! (Although the type of the text outside the PHP tags depends on the HTTP response content type). So only the code inside the PHP tags is PHP code, not the PHP tags themselves. PHP tags are necessary for the PHP interpreter to know where the PHP code starts and finishes. However, the Markdown viewer/editor does not need them, because it has php and for this purpose. The text between php and is only PHP code and should be treated like this.

If you really believe that VSCode is the ultimate source of truth for Markdown then I suggest to go for the second proposed solution:

• Prepend <?php only if the client is VSCode.

[felixfbecker]

Hm, I see your point. In a CLI, anything outside <?php will actually just get printed to STDOUT and doesn't have to be HTML.

I don't want to implement any client-dependent behaviour. The LS must function universal for every client. Besides, I don't even know how to find out if it's VS Code who launched us.

[kaloyan-raev]

Eclipse Che adds "clientName": "EclipseChe" to InitializeParams when sending the Initialize Request. This additional param is actually provided by the Typefox Java binding and is a custom extension of LSP. We can propose it becomes part of the protocol.

[felixfbecker]

I don't think this is a good idea. It encourages servers to behave differently depending on the client. The server should always behave predictably, independent of the client, that is the whole point of a standardized LSP.

[felixfbecker]

https://github.com/Microsoft/vscode/issues/14166

[kaloyan-raev]

I agree that we should avoid implementing client-specific logic where possible. Thanks for reporting the issue to VSCode. Let's see if and when they plan to fix it.

[WouterSioen]

Is there any work on this planned yet? If not, I wouldn't mind to pick this issue up.