halohalospecial / atom-elmjutsu

A bag of tricks for developing with Elm. (Atom package)
https://atom.io/packages/elmjutsu
MIT License
192 stars 24 forks source link

Usability: Default auto-complete length #5

Closed OvermindDL1 closed 8 years ago

OvermindDL1 commented 8 years ago

Default auto-complete length needs to be 2 or higher, not 1. Functions involve typing =, then generally hitting , which unfortunately replaced the = with == due to autocomplete. Perhaps that alone could be fixed to not happen, but for a short-term fix setting the default to 2 will prevent people from going crazy until they figure out to fix it. :-)

halohalospecial commented 8 years ago

Alternatively, the users can set Keymap For Confirming A Suggestion in the package settings for autocomplete-plus to either tab or tab always, enter when suggestion explicitly selected, instead of the default tab and enter.

OvermindDL1 commented 8 years ago

I tried tab always, enter when suggestion explicitly selected, but enter still turns = into == a a somehow, even after restarting atom? o.O

halohalospecial commented 8 years ago

Hmm, that's weird. I had it working with autocomplete min length = 1.

rtfeldman commented 8 years ago

Totally agree. I actually came here to open this same issue, for the same reasons and including the same proposed solution.

I think "it gives a pleasant experience right out the box" is important, and right now the = thing inevitably frustrates a new user and sends then scurrying off to the docs to figure out how to fix it.

+1 to people not having to fix it by default 😉

rtfeldman commented 8 years ago

PS: this thing is 😺

halohalospecial commented 8 years ago

Hi! I did some experiments and this is what I came up with:

What do you think of this approach?

P.S. I got confused by your P.S. @rtfeldman haha

rtfeldman commented 8 years ago

Sounds good!

What I meant by the P.S. is that I'm a fan of this plugin. 😄

halohalospecial commented 8 years ago

Thanks! :blush:

I just published a new release with this change and other bug fixes. I personally find it better than the default autocomplete behavior so far in terms of maintaining the user's flow :smiley: Let me know your experience. Thanks!

OvermindDL1 commented 8 years ago

Hmm, I just updated but = still autocompletes to == a a, maybe I need to restart Atom, doing so, testing again, and it autocompletes to = now, however it does not add a newline and as such the pop-up pops up again. Here I am typing tester = and pressing , a lot. ^.^: https://overminddl1.com/screenshots/work/elmjutsu-autocomplete-bug.gif Bug Demo

halohalospecial commented 8 years ago

That only happens if Enable Autocomplete Snippets is turned on. The latest release should fix it. Thanks @OvermindDL1!

OvermindDL1 commented 8 years ago

Probably fixes it on ubuntu, but on Windows 10 at work it still has the same action, interesting, need to debug in to it again...

And debugged a bit, the 'event' still has originalEvent as undefined?

So, two questions. :-)

  1. Any reason to force it to only test if using 'enter' there? What if someone rebinds the autocomplete key to something else, like caps lock?
  2. Any chance on testing if originalEvent is undefined and if so then just setting usedEnterKeyToAutocomplete = true; regardless? :-)
OvermindDL1 commented 8 years ago

For note, added if (event.originalEvent === undefined) usedEnterKeyToAutocomplete = true; on line 11 and it is working well now (the \n does not mess anything up in CRLF mode either it seems just to note :-) ).

halohalospecial commented 8 years ago

When I looked at the autocomplete-plus code before, they hardcoded the functionality to tab and enter.

halohalospecial commented 8 years ago

I'm still wondering why originalEvent is undefined on your computer. Anyone else using Windows 10? :smiley:

OvermindDL1 commented 8 years ago

That is a fantastic question, not a clue. ^.^

OvermindDL1 commented 8 years ago

Had a few moments, got an idea, debugged up the stack from that callback, found the missing originalEvent, apparently the callback is called twice and the second time it is undefined, however the first time is not entering into the if conditional either because it is a different 'command' ("atom-elixir:autocomplete-enter", in elm-mode? the frick?). The second time through (when originalEvent is undefined) is when the type is "autocomplete-plus:confirm". Walking up the event chain I cannot find originalEvent at all on the event object. Walking through that elixir one, holy-hell-atom-is-inefficient! 164 callbacks called for every single event through every registered plugin and each testing what command they want instead of them just registering to what they are listening for?! No wonder atom is so slow at times being coded like this! o.O

Aaaand yep, atom-elixir is re-dispatching "autocomplete-plus:confirm" via atom.commands.dispatch(atom.views.getView(editor), 'autocomplete-plus:confirm'), thus the originalEvent is not sent out... The hell are they doing this for?

OvermindDL1 commented 8 years ago

Yeesh: https://github.com/msaraiva/atom-elixir/blob/master/lib/elixir-autocomplete-provider.coffee#L16-L34

halohalospecial commented 8 years ago

Ooh, they should probably scope that to just source elixir. Does it work when you disable atom-elixir?

halohalospecial commented 8 years ago

There's a similar issue here https://github.com/msaraiva/atom-elixir/issues/44

OvermindDL1 commented 8 years ago

Ooh, they should probably scope that to just source elixir. Does it work when you disable atom-elixir?

Yep, it does.

There's a similar issue here msaraiva/atom-elixir#44

Awesome... >.>

P.S. Gotta love how github does not quote properly, missing code tags and url...

halohalospecial commented 8 years ago

This will probably work for the atom-elixir issue:

atom-text-editor:not([mini])[data-grammar^="source elixir"]

OvermindDL1 commented 8 years ago

Ah cool, adding that to my local copy at least. Thanks. :-)