Remove built-in `rope_rename` plugin

python-lsp / python-lsp-server

Fork of the python-language-server project, maintained by the Spyder IDE team and the community

MIT License

1.75k stars 186 forks source link

Remove built-in `rope_rename` plugin #515

Closed doolio closed 3 months ago

doolio commented 5 months ago

This functionality is better served by the pylsp-rope plugin.

Fixes #255

krassowski commented 5 months ago

That would be a major version bump, right? Some downstreams depend on the rope rename support being there by default.

doolio commented 5 months ago

Yes, I should think so. This PR relates to this comment from @ccordoba12 who I trust will decide when this could be considered for release with the appropriate version bump.

ccordoba12 commented 5 months ago

That would be a major version bump, right? Some downstreams depend on the rope rename support being there by default.

A couple of questions:

Do you know which projects depend on it?
I think there shouldn't be major issues for downstream projects because the only thing they should be doing is enabling rope_rename with a config option, which would stop working. But renames could be handled by enabling jedi_rename instead.

Shane-XB-Qian commented 5 months ago

That would be a major version bump, right? Some downstreams depend on the rope rename support being there by default.

A couple of questions:

Do you know which projects depend on it?

I think there shouldn't be major issues for downstream projects because the only thing they should be doing is enabling rope_rename with a config option, which would stop working. But renames could be handled by enabling jedi_rename instead.

if it was enabled by default, then could you also make this refactor keep 'rename' feat be enabled by default? though it would be on 'jedi'. // since perhaps someone maybe no 'rope' installed, or not like (or cannot) to install 'rope'.

-- shane.xb.qian

doolio commented 5 months ago

if it was enabled by default

No, it is not enabled by default

https://github.com/python-lsp/python-lsp-server/blob/d72690557f8997d13027802b639fb142b7304ba7/pylsp/plugins/rope_rename.py#L15-L17

and not even listed as a configuration option in CONFIGURATION.md which is what brought it to my attention.

Edit: Nor for that matter is jedi_rename!

lieryan commented 5 months ago

rope_rename is not currently enabled by default, so the only configuration that breaks is for people who explicitly enabled rope_rename, which I don't suppose there will be many people using them.

pylsp-rope currently do not support renaming. I can add this to pylsp-rope, but will need to make sure that the version constraints is such that people don't install pylsp-rope with rename support on older python-lsp-server that also has conflicting rename support.

ccordoba12 commented 5 months ago

rope_rename is not currently enabled by default, so the only configuration that breaks is for people who explicitly enabled rope_rename, which I don't suppose there will be many people using them.

Agreed. That's why I think if we make a note in the release notes about the removal of that plugin and that people can switch to jedi_rename now and pylsp-rope in the future should be enough.

pylsp-rope currently do not support renaming. I can add this to pylsp-rope

That would be great, thanks @lieryan!

but will need to make sure that the version constraints is such that people don't install pylsp-rope with rename support on older python-lsp-server that also has conflicting rename support.

Ok, so that means you probably need to have this in a published release of python-lsp-server so that pylsp-rope can depend on it, right?

doolio commented 3 months ago

No problem. I presume nothing further is required from me with this PR?

ccordoba12 commented 3 months ago

Nop, it isn't.