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

Add evap-dev-with-lsp shell #2407

Merged
merged 1 commit into from
Mar 11, 2025
Merged

Conversation

niklasmohrin
Copy link
Member

No description provided.

@richardebeling
Copy link
Member

Marking as draft: intention was to provide access into python dependencies inside the nix shell (for vscode / vim / other lsp setups, e.g. to help with imports), but for our setups, "go to definition" jumping to a django definition doesn't work yet

@richardebeling richardebeling marked this pull request as draft March 10, 2025 23:20
@niklasmohrin
Copy link
Member Author

I played around with it a bit and I think the most robust version would be to get the Python LSP packages in the same way we get other development dependencies - through uv and PyPI.

For reference, the previous attempt used, roughly speaking, nixpkgs.python312.pkgs.python-lsp-server. This is set up to use nixpkgs.python312 as the Python interpreter - one that (correctly so!) doesn't know about our virtualenv. I then tried to use the Python from the venv, but running pylsp with that also didn't automatically pick up our venv.

So (optionally) including the LSP in the venv seems to be the most intended way. If we really don't want to do this, we could also try to override the Python used by nixpkgs' pylsp, but I suspect that this will cause rebuilds of pylsp and it's dependencies (which compile some Rust crates).

I guess locking pylsp is fine, as we are already locking things like black. Let's hope that no weird dependency resolution things come up, and if they do, we can purge pylsp again.

@niklasmohrin niklasmohrin marked this pull request as ready for review March 10, 2025 23:34
Copy link
Member

@richardebeling richardebeling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, tested "go to definition" on django code, "organize imports" (ruff) and django import autocompletion, all work. Thanks! :)

@niklasmohrin niklasmohrin merged commit 3a6312b into e-valuation:main Mar 11, 2025
15 checks passed
@niklasmohrin niklasmohrin deleted the lsp-shell branch March 11, 2025 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants