PR: Move completion plugin files to manager plugin and create a completions.manager.api file

[goanpeca]

To make this migration simpler, this PR:

• Move completion dangling files into manager folder.
• Moves the LSP __init__ content to completions/api.py. (So a provider depends on the manager and not on another provider)
• Moves the spyder.api.completions to completions/api.py (So a provider depends on the manager and not on another provider)
• Fix imports

The completion manager already makes the assumption that it will follow the LSP protocol (plus a couple of specific things like the on mouse move needed for kite). However it is not clear why a completion provider plugin (like fallback, kite, LSP) needs to import things from the languageserver plugin since the manager plugin already made that assumption for every other plugin that will sit on top of the LSP protocol. For this reason, spyder.plugins.completion.manager.api is where all the mixins and constants are now moved into.

The reason for the decoupling: if I deleted the kite or the languageserver or the fallback folder, Spyder should be able to start and run with whatever providers were found using the plugins entry points. kite or the languageserver or the fallback should only depend on spyder.plugins.completion.manager

This will make the PR that actually makes the migration to the API much simpler.

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct: @goanpeca

[pep8speaks]

Hello @goanpeca! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file spyder/plugins/completion/languageserver/tests/conftest.py:

Line 24:1: E402 module level import not at top of file

Comment last updated at 2020-07-29 14:50:05 UTC

[ccordoba12]

Is this really necessary? Why not leave all completion clients (LSP, fallback and Kite) along with its manager in the current completion module?

That way, the only thing you'd need to move is the manager to its own submodule. So the structure would be

• completion
  • manager
  • languageserver
  • kite
  • fallback

We also talked about this with @andfoy and consider it makes more sense to leave everything related to completions inside completion. Although they are different plugins, I think there's no limitation in the current API that would prevent us to do that, right?

[goanpeca]

Is this really necessary?

Yes.

Why not leave all completion clients (LSP, fallback and Kite) along with its manager in the current completion module?

Cause they are used as plugins but they are not written as plugins.

Although they are different plugins, I think there's no limitation in the current API that would prevent us to do that, right?

Yes, there is.

The idea is to avoid special casing things, completions are doing a lot of special casing and assumptions, for no gain just more complexity. Also it is not extendable and there are circular dependencies in the way is written that should not exist.

If you want the folders to be close to one another the name can be comp_fallback, etc.

We also talked about this with @andfoy and consider it makes more sense to leave everything related to completions inside completion.

I also talked to you months ago and you agreed with this and you also agreed it was a hack needed at the time (how it is right now, due to time constraints), but it was actually not the way to go.

[ccordoba12]

The idea is to avoid special casing things, completions are doing a lot of special casing and assumptions, for no gain just more complexity. Also it is not extendable and there are circular dependencies in the way is written that should not exist.

I understand that. But just as you told me that a third-party plugin can contain several smaller Spyder plugins in the same Python package (something I think Tom is purporting to do with his own plugin), I don't understand why it's not possible to have all completion plugins inside a completion module.

If that's really impossible due to the new API, then it's a really inflexible design and we need to change it.

I also talked to you months ago and you agreed with this and you also agreed it was a hack needed at the time (how it is right now), but it was actually not the way to go.

I'm not opposed to disentangle the plugins and make things clearer. I just think it's easier for new contributors and us in the future to leave all plugins related to completions in a completion submodule.

[ccordoba12]

Yes, there is.

Please describe in detail what the real limitation is because I don't understand it, sorry.

[goanpeca]

But just as you told me that a third-party plugin can contain several smaller Spyder plugins in the same Python package

That does not mean they have to be bundled in a single folder, which is what you are proposing.

(something I think Tom is purporting to do with his own plugin),

Yes and no. He is providing several project types but it is just a single plugin. And even if they were separate plugins the folder structure I would suggest would be something like: (which is going on the API docs)

root
    spyder_plugin_1
        ...
    spyder_plugin_2
        ...


# or

root
    spyder
        plugin_1
            ...
        plugin_2
            ...

If that's really impossible due to the new API, then it's a really inflexible design and we need to change it.

It is possible, however it would be a very bad practice and I strongly disagree with it.

[ccordoba12]

root
    spyder_plugin_1
        ...
    spyder_plugin_2

This is what I'm proposing in this case, with root being completion. So completion would be like a third-party package embedded in Spyder. Besides, not all modules below spyder are plugins (an important example being spyder/widgets).

It is possible, however it would be a very bad practice and I strongly disagree with it.

Ok, I understand your point and noted your disagreement. However, @andfoy and I (who are really in charge of maintaining this code), prefer to break good practices to have a code structure that makes sense to us.

So please follow my suggestion above and in this PR only move spyder/completion/decorators.py and spyder/completion/plugin.py to a new module called spyder/completion/manager. Then you can disentangle the code in the completions module in a new PR.

[ccordoba12]

Besides, not all modules below spyder are plugins (an important example being spyder/widgets).

Sorry, this part of my comment is flawed because I was thinking on the way things were organized in Spyder 3.

Still, since it's a matter of preference (not a technical issue) to have the completion plugins at the same level of the rest, I think we should leave things as they are now (with the exception of the manager plugin's creation, as I mentioned above).

[goanpeca]

to a new module called spyder/completion/manager

Fixed, see the issue description for more information on the other files that were moved and the rationale.

Also, this is blocked by #13408.

[ccordoba12]

However it is not clear why a completion provider plugin (like fallback, kite, LSP) needs to import things from the languageserver plugin since the manager plugin already made that assumption for every other plugin that will sit on top of the LSP protocol. For this reason, spyder.plugins.completion.manager.api is where all the mixins and constants are now moved into.

I totally agree with the way you reorganized things (and this is the kind of disentanglement I was talking about). We were aware of this mess with @andfoy but didn't have time to fix it for Spyder 4. Thanks for taking the time to do it for Spyder 5.

I left a couple of minor comments, otherwise looks good to me now.

[ccordoba12]

Thanks @goanpeca!