-
Notifications
You must be signed in to change notification settings - Fork 13
Add devsense-php-ls
LSP support
#34
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
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Exciting! I was not familiar with the phptools LSP, but I would love for Zed to support it! I'm not sure want to immediately ship with it as the default, both because it's untested, but also because we prefer to not auto-download proprietary binaries by default. That said, I know user's experience with |
devsense-php-ls
Language Serverdevsense-php-ls
LSP support
Thank you @notpeter - we're excited to have our LSP in Zed. It surely needs to be tested first, and we're keen to improve or update features to fit in Zed's user experience. Some features need to be implemented outside the LS in Rust code - but that's for another PR. (for example the activation can be added as a Zed command). |
Seeing the "Resolving code actions" flash in the status bar, every time you move the cursor, itself is pretty annoying 🙂 |
cannot wait to see this merged! |
Thanks @rabol for quick testing! |
# Conflicts: # src/php.rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of nits, but other than that, LGTM
Co-authored-by: Piotr Osiewicz <[email protected]>
Co-authored-by: Piotr Osiewicz <[email protected]>
Co-authored-by: Piotr Osiewicz <[email protected]>
Thanks! I'll cut a new extension version shortly. |
Thank you @osiewicz - we're looking forward to it! |
I'm already using it with my premium license — it's amazing! I configured the license by adding an environment variable in my .bashrc: export DEVSENSE_PHP_LS_LICENSE='{...}' Could this pose any security risks? |
@matheus-de-araujo thank you for the kind words! If the variable gets "stolen", someone else may use your premium license and/or see your name. Do you have any recommendations? We're preparing some updates so it will be easier to activate it though. |
Zed currently has no hidden or secure license storage. All plugin settings — including license keys — are stored in plain JSON in:
In contrast, VS Code uses a secret global memento (globalState/SecretStorage) managed internally in a SQLite DB, not exposed in the user's settings Suggested improvement:Add a command like Zed: Enter PHP License that saves the key to a protected file (e.g., I don’t know if this would be technically feasible. |
@matheus-de-araujo good idea - that's what we do in VSCode.
|
@jakubmisek any chance you could provide a little snippet showing what to add to the zed config to get phptools stubs configured? |
Sure, we'll prepare complete documentation soon. For now, you can drop most of the vscode-like settings into the "initialization_options": "lsp": {
"phptools": {
"initialization_options": {
"php.stubs": ["*", "zip", "zlib", "pcntl", "com", "composer", "wordpress"]
}
}
} "php.stubs" gets array of stubs where
|
@jakubmisek Thank you, it was I wish this could be standardized for all Zed extensions. |
@pjv thanks for the question - I'll take a look at "settings" as well, since it makes more sense to me personally. |
@jakubmisek sorry to keep banging away inside this PR, and let me know if i should be opening new issues somewhere else. I’m trying to configure a custom php_stubs directory and this seems not to be working for me. Can you see where I've gone wrong? ![]() |
@pjv it's alright here for me. You can start a new issue at https://github.com/DEVSENSE/phptools-docs/issues if you want though.
|
@jakubmisek No, I don’t see any error in the logs and it tells me it’s indexing my directory:
But when I look at some code that has, for example, calls to woocommerce classes and functions whose stubs are definitely in that directory (working correctly with the intelliphense LSP), phptools is giving me errors like this: ![]() |
@jakubmisek is there any way i could try to track down why the extension seems not to be able to make use of the stub files that the log says it is indexing? Do you have any easy-enough way to reproduce this issue?
…above was incorrect; i had temporarily turned phptools off and was looking at the project with intelliphense turned on. in fact, adding the stubs project to the workspace does NOT fix the warnings. And… LSP server logs looks like: ![]() |
@pjv got it, use the URI path format for now: "php.workspace.includePath": "file:///Users/pjv/development/php_stubs" Adding the folder to the project does not work in Zed, because it treats it as another separate folder and starts a new instance of language server (this is probably be design in Zed, although it behaves differently than in VSCode) |
@jakubmisek yup, that’s working. Thank you. Looking forward to the full documentation. This is a great addition to Zed. |
Edit: I fixed it just after by replacing the double quotes by single quotes.
Hello. export DEVSENSE_PHP_LS_LICENSE="{json_signature}" And i got json_signature from Thanks in advance |
@kevinwombat you could try adding this to your settings.json instead. I did, and it works for me, after adding backslashes to the quotes as such:
|
@kevinwombat echo $DEVSENSE_PHP_LS_LICENSE As sugggested by @jsondergaard , "lsp": { "phptools": { "initialization_options": { "0": "{json_signature}" } } } We'll make this process a bit simpler in a future update, I promise |
I assume {
"inlay_hints": {
"enabled": true,
"show_parameter_hints": true
},
"lsp": {
"phptools": {
"initialization_options": {
"php.inlayHints.parameters.enabled": true
}
}
}
} |
@ADmad yes,
Edit: there was a legacy workaround for VSCode only - implemented properly and enabled for Zed and other LSP editors (devsense.-php-ls 1.0.17769). Works in Zed now: |
Thank you, I do get variable name and return type inlays now as expected 👍. A slight issue with return type inlays though. Double clicking on the return type inlay doesn't add the return type into the code, as documented for vscode. |
@ADmad not sure why InlayHint edits are not working - do they work in other languages in Zed? |
@jakubmisek I only work with PHP so can't say about other languages :) |
@josecanciani your setting is correct and Zed should even restart language server automatically upon saving the settings.json file. You can even shorten it just one language server of your choice: "languages": {
"PHP": {
"language_servers": ["phptools"]
}
}, Do you see it in LSP logs "dev: open language server logs"? Is the path to your setting.json correct, if you check what's happening in the zed' log ("zed: open log")? |
@jakubmisek I changed it to just have intelephense, but it's still firing devsense (not phpactor though). Logs:
These are the logs for phptools, which I would suppose should not be there: ![]() And these for Intelephense, which is working fine: ![]() BTW, Intelephense stops indexing with error "MaxUriCountError: Maximum URI count of 262143 exceeded.", not related I assume, but if you know what that could be happening, I'm glad to hear :) |
This pull request adds support for
devsense-php-ls
, the language server used by the PHP Tools extension for Visual Studio Code.This language server offers a comprehensive PHP development experience (within the limits of the LSP protocol), including:
Configuration
Once the extension is installed, the language server can be enabled via settings. A separate PR will be submitted to update https://github.com/zed-industries/zed/blob/main/docs/src/languages/php.md after this PR is merged.
Next Steps
After this is merged, I plan to: