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

Make optional the Intellisense offer of super()-calls when overwriting inherited methods #2531

Closed
Diogo-Rossi opened this issue Apr 4, 2022 · 4 comments

Comments

@Diogo-Rossi
Copy link

The feature of intellisense offering super()-calls when overwriting inherited methods is very useful (requested in #668).

But can it remain as optional? (activated in settings)

Because sometimes we may don't want to add the super call.
Like when implementing interfaces/abstract classes, for instance, as in this example.

@erictraut
Copy link
Contributor

This doesn't seem like a good candidate for a setting. It's not really a personal preference, but rather something that is contextual. The same user might want it in some cases but not in others. Switching a setting between these cases would be cumbersome.

In cases where you don't want to call through to the base class method, why not simply overwrite the code when you add the actual implementation?

@Diogo-Rossi
Copy link
Author

It may be tedious to always remove the super call when implementing a lot of abstract classes.

But it is really just one line that we have to always remove in the methods. So, maybe you're right and it is not worth it.
If you judge this so, you can close the issue.

BTW, switching a specific setting so often is not so cumbersome when we install this extension.

@erictraut
Copy link
Contributor

What code would you want pylance to emit when the base method is abstract? The method requires at least one statement in its implementation to avoid a syntax error. Would you prefer that it emit pass? Or """ You must provide an implementation here """? In any case, it must emit some statement that you will need to replace. The statement return super().<method>(<args>) seems like it's as good as the other options, no?

@Diogo-Rossi
Copy link
Author

You're right. I was thinking about "no emission at all", as we always have to implement something in abstract methods,
and that would force us to do it to avoid syntax error.

But the return super(). statement is fine.
It is useful most of time, and can easily be erased when it is needed (shift + home, del do the job)

Sorry to bother you with that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants