Skip to content

Correctly handle null value for ropeFolder config #604

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

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

osiewicz
Copy link
Contributor

@osiewicz osiewicz commented Dec 1, 2024

In Zed we've tweaked defaults of pylsp for rope as we do not want it to create any files in users projects by default; we've happily passed null as an initialization option for ropeFolder only to find out that it is not handled correctly by plugin shim on pylsp side.

The faulty logic lies in the fact that dict.get returns None by default when a value is not present in dictionary, whereas it is also a valid user-provided value. Thus, when we check whether there's an user-supplied ropeFolder, we end up falling back to ropes default when said user provided value is null.

This PR fixes this issue by explicitly checking whether there's an user-provided ropeFolder and using that (regardless of it's value) if it's set.

@ccordoba12 ccordoba12 changed the title rope: correctly handle null value for ropeFolder config Correctly handle null value for ropeFolder config Feb 6, 2025
@ccordoba12 ccordoba12 added the bug Something isn't working label Feb 6, 2025
@ccordoba12 ccordoba12 added this to the v1.12.1 milestone Feb 6, 2025
In Zed we've tweaked defaults of pylsp for rope as we do not want it to create any files in users projects by default; we've happily passed null as an initialization option for ropeFolder only to find out that it is not handled correctly by plugin shim on pylsp side.

The faulty logic lies in the fact that dict.get returns None by default when a value is not present in dictionary, whereas it is also a valid user-provided value. Thus, when we check whether there's an user-supplied ropeFolder, we end up falling back to ropes default when said user provided value is null.
@ccordoba12 ccordoba12 force-pushed the rope_allow_null_values branch from bdd6c84 to 2aea31e Compare February 6, 2025 16:41
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @osiewicz!

@ccordoba12 ccordoba12 merged commit 0508463 into python-lsp:develop Feb 6, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants