Peekmo / atom-autocomplete-php

Autocomplete for PHP in atom editor
MIT License
136 stars 34 forks source link

Idea - Don't insert parameter list when one is already present (usability). #136

Closed ghost closed 9 years ago

ghost commented 9 years ago

Hello

This is an idea regarding a usability enhancement, take the following existing code:

$this->foo($param1, $param2);

I've now refactored foo to be called something different, or I just want a different method, I remove foo:

$this->|($param1, $param2);

The cursor is now after $this-> and I'm receiving autocompletion. Now I select the new name of the method (or another method name):

$this->bar($barParam1, $barParam2)($param1, $param2);

Oops... I now have two argument lists. Perhaps it might be a good idea to not insert the parameter list ($barParam1, $barParam2) when the prefix that is going to be replaced is already followed by an opening paranthesis? This might seem like a minor change, but I think it might be a good usability improvement. Also, when you're not refactoring and just writing code, such as $this->, you'll pretty much never have the argument list already present, so it won't interfere with that behavior.

The same could apply to methods, built-in functions, as well as new class(constructor), etc. (I think pretty much whenever there is an argument list in the snippet and there already is an opening paranthesis after the prefix).

What do you think?

Peekmo commented 9 years ago

I agree.

Instead of not "adding", it will be to "remove" the inserted characters (because that's autocomplete-plus which inserts the text (on method "onDidInsertSuggestions" I guess (https://github.com/atom/autocomplete-plus/wiki/Provider-API#defining-a-provider)). Do you see a better way to do that?

Anyway, that's a good idea

ghost commented 9 years ago

We could perhaps just show the full function snippet as displayText (so it always shows up with its parameter list in the autocompletion box), and set a version without the parameter list in snippet when adding it as suggestion so the actual inserted snippet is different? (I'm not really that familiar with the actual API yet.)

PS: Feel free to take on any of the tickets I'm creating (that is, when you agree with them ;-), I'm creating them mostly as a reminder for things that need to be fixed or should be improved. (I will of course also tackle some of these tickets myself, as I've been doing so far.)

Peekmo commented 9 years ago

Oh, I didn't see the new option "displayText". In fact, it could be used.

For the tickets, I don't have so much time those last days, I will work on some of them of course ;)