sublimehq / Packages

Syntax highlighting files shipped with Sublime Text and Sublime Merge
https://sublimetext.com
Other
2.95k stars 587 forks source link

[Python] Consider relaxing completion cancelation rules #2404

Closed rchl closed 3 years ago

rchl commented 4 years ago

In python files completions are inhibited quite aggressively, I feel. Not sure what is the reasoning for this behavior so I would suggest to reconsider current behavior and potentially relax it.

Expected behavior

It should be possible to trigger completion popup both in:

def |
def a|

Actual behavior

It's only possible to trigger completions popup in case A.

Steps to reproduce

  1. Insert code (where | is the caret position):
    
    def aaa:
    pass

def a|


2. Try to invoke completions (using ctrl+space or whatever is the relevant shortcut for your platform)

The behavior is controlled by:
https://github.com/sublimehq/Packages/blob/40ec1f2f9b56fb55d739e17ecd003cc9e8c9b096/Python/Completion%20Rules.tmPreferences#L8-L9
and it seems like this logic was there more or less from the beginning.
FichteFoll commented 4 years ago

Not sure what is the reasoning for this behavior

I suspect it's because when you're creating the function, you are usually creating this name as the reference and then AC might be getting in the way when there's nothing you'd want to complete it to.

That said, wait does LSP offer when defining a function? Does it also do that for class names?

rchl commented 4 years ago

Good point, when creating a new class or function definition you are typically creating a new name and completions are not really relevant. Except when you want to type one of the pre-defined names like __init__, __eq__, and many others. LSP provides completions for those.

So maybe the rules could be changed to not cancel completions for identifiers starting with _. Or __ but then the completions would still not work when typing just one _.

Does it also do that for class names?

It doesn't seem to provide any completions for class names (makes sense).

rwols commented 4 years ago

I'm in favor of __ being an exception.

def __

It'd be nice to see a list of dunder methods to select from.

michaelblyons commented 4 years ago

I'm in favor of __ being an exception.

def __

I'm in favor of it not being an exception while typing. If that's manageable for most circumstances, sure, make it an exception. If not, please don't. Edit: Disregard. I misunderstood.

It'd be nice to see a list of dunder methods to select from.

Yes. If those are ST-native completions for those who have not been converted to the Lord-and-Savior Protocol, so much the better.

rchl commented 4 years ago

@michaelblyons Even native behavior would benefit from this change. ST is shipping with __ snippet that can only be triggered when def is typed but disappears once def _ is typed which makes it harder to select it.

It's not that much as far as built-in functionality goes but still something.

michaelblyons commented 4 years ago

@michaelblyons Even native behavior would benefit from this change. ST is shipping with __ snippet that can only be triggered when def is typed but disappears once def _ is typed which makes it harder to select it.

It's not that much as far as built-in functionality goes but still something.

Oh, yes. All of your points I agree with. It's only the flash of illegal.invalid that I object to, if you happen to tap _ twice while you are typing a function name instead of picking a snippet or completion item from the autocomplete. Edit: Disregard.

I think having a list of dunder methods in a .sublime-completions is great. Probably only inside classes, though? I think the scopes will support that.

rchl commented 4 years ago

It's only the flash of illegal.invalid that I object to, if you happen to tap _ twice while you are typing a function name instead of picking a snippet or completion item from the autocomplete.

I don't get that part. What flash of illegal.invalid are you talking about?

@rwols

I'm in favor of __ being an exception.

Only that in that case you would get completions when typing def, then those would disappear on typing def _ and then show up again on typing def __. A bit weird?

michaelblyons commented 4 years ago

Disregard basically everything I wrote. 😊 I heavily misunderstood @rwols comment.

FichteFoll commented 4 years ago

I'm in favor of __ being an exception.

Sounds good to me.

predragnikolic commented 4 years ago

I'm in favor of __ being an exception.

If a class extends another class, AC can also suggest parent class methods(which we can then override). But if we allow only completions that starts with __ we wont see parent methods anymore. :)

image

FichteFoll commented 4 years ago

True, this is a use case I didn't consider. Seeing that Python is, to my knowledge, the only package that prevents completions for function and class definition, I say we can probably simply remove it and align with the other packages, unless someone has a good argument for why this is useful and why it should be added to all other packages as well.

FichteFoll commented 4 years ago

Okay, that's not true. A quick ripgrep reveals the following packages also have this:

I guess allowing AC for functions while disabling for types isn't unheard of. Thoughts?

rchl commented 4 years ago

I guess allowing AC for functions while disabling for types isn't unheard of.

What are you calling "types" here?

FichteFoll commented 4 years ago

Classes, enums, structs and similar. Things you'd not like to have overlapping names unlike functions in languages that are object-oriented and allow overriding or use overloading.

rchl commented 3 years ago

I guess allowing AC for functions while disabling for types isn't unheard of. Thoughts?

There is one case I can think of where allowing type would make sense. When you are defining a new class and specifying the super-class in parenthesis:

class Foo(|)

LSP can provide suggestions here.

(At least I guess that falls under "types").

FichteFoll commented 3 years ago

When you are defining a new class and specifying the super-class in parenthesis:

We are only talking about definitions here, not usage. Either way, I opened #2620 to address this.