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

Autocomplete as you type #15

Merged
merged 37 commits into from
Aug 19, 2013
Merged

Autocomplete as you type #15

merged 37 commits into from
Aug 19, 2013

Conversation

Shadowfiend
Copy link
Member

Vim's ACP has this, and Sublime has this. I would love to see this in Vico. I've tried to implement it using Nu, but I hit a wall. Now that we have the source, it's likely much easier.

@Shadowfiend
Copy link
Member

The idea here is simply that the completion popup be triggered while you're typing? What would be the criteria for bringing it up?

@jordwalke
Copy link
Author

@Shadowfiend : Like in many other editors (Sublime/VisualStudio), as you type characters (longer that X) it searches open documents (or some other configurable index) for words that begin with that prefix, and pops up the completions below. Vim's famous ACP.vim plugin does this in a way that's similar to Sublime. (Emacs has a plugin as well.)

If an open document contains: "Hello There".
And in any other document, you were to type "He" - it would pop up "Hello" as an option. Hitting enter or tab would select that first match. (Most allow configuration of enter vs. tab to select).

@Shadowfiend
Copy link
Member

So in your mind typing a 2-letter prefix would be what sets it off? I've used editors with autocomplete before, just trying to get a feel for when we bring it up at all.

@jordwalke
Copy link
Author

Yes, two letters is usually a great way to quickly filter out a huge number of possibilities. One letter is usually overkill. The key is that it should be two characters after any word break (like punctuation etc). A lot of this could/should be configurable IMHO.

@Shadowfiend
Copy link
Member

Yeah. This is definitely a larger feature; if anyone wants to take up the gauntlet, that would be cool. I've got a few other big features I want to take care of first, so it might be a while before I can look at this.

Before all of that I'm going to try and clear out some of the issues that have already been filed here, though. Most have been pending for a tad too long.

@jordwalke
Copy link
Author

Understandable - thanks for continuing development on this very promising editor. I feel the auto-complete-as-you-type feature is one of the reasons may people are using Sublime and I'd love to see Vico gain traction.

@Shadowfiend
Copy link
Member

Sublime's completion algorithm is also a little more complex than Vico's atm, I think. Still, definitely worth working on (Vico's completion's been annoying me a little lately and I think I have some ideas on how to improve prioritization and such).

@jordwalke
Copy link
Author

@Shadowfiend: Can you see a way to get the autocompletion to pop up as you type? I tried to implement this by simply creating a plugin, but the plugin system isn't powerful enough to trigger the popup window. It definitely required some modifications to the core of Vico.

@Shadowfiend
Copy link
Member

I'll try and have a look over the next few days to see if I can find an easy fix.

Everything about the completion popup currently works as it did before.
Before we only highlighted your typing as you went in the completion
popup, now the text also properly appears in the text view.
We're not all the way there, but most of the time the caret ends up at
either where it should be if the completions are ultimately ignored, or
it ends up at the end of the completed text when the completion is
accepted.
Inserting as snippets was having some very bizarre side-effects at times
when space was being hit. More to come on that front, I think. However,
it didn't seem like snippet insertion was being used anywhere at the
moment anyway, as in no snippets were ever suggested.
The real culprit was that space accepted the completion
instead of ignoring it, which doesn't make sense as you
type. For now, removing space as an accept action.
@Shadowfiend
Copy link
Member

I've been doing some work on this (work so far is in the completion-as-you-type branch), but it looks more difficult than originally expected. Because the completion popup is currently something that runs modally, keeping up the typing in the main window is not particularly straightforward. So I'm going to have to rework the completion popup to work non-modally, which is going to be a little bit of work. I'll see what I can do tonight.

@jordwalke
Copy link
Author

Awesome. Could you help me understand how you would get it to work non-modally? Would you paint it directly on top of the text in the save text view (full disclaimer, I haven't done iOS programming in a long time). If so, maybe we could also think about ways to let plugins (or other features) do other types of painting on top of the text. I'd love to get inline compile/lint errors highlighted directly on top of the text (though I imagine there's a much easier way to do that).

@jordwalke
Copy link
Author

I saw this error when Makeing : Apologies if this is something nooby (I'm guessing it's unrelated).

=== BUILD NATIVE TARGET finish_installation OF PROJECT Sparkle WITH CONFIGURATION Release ===
Check dependencies

** INTERNAL ERROR: Uncaught exception **
** INTERNAL ERROR: Uncaught exception **
Exception: *** -[__NSArrayM insertObject:atIndex:]: object cannot be nil
Exception: *** -[__NSArrayM insertObject:atIndex:]: object cannot be nil
Stack:
Stack:
  0  0x00007fff911b1f3a __exceptionPreprocess (in CoreFoundation)
  1  0x00007fff8d914d5e objc_exception_throw (in libobjc.A.dylib)
  2  0x00007fff91158ab8 -[__NSArrayM insertObject:atIndex:] (in CoreFoundation)
  3  0x0000000102d61337 -[XCDependencyCommand createStartedCommandInvocation] (in DevToolsCore)
  4  0x0000000102e0f81b -[Xcode3BuildTask main] (in DevToolsCore)
  5  0x00007fff88ed86b4 -[__NSOperationInternal start] (in Foundation)
  6  0x00007fff88eeb912 ____NSOQSchedule_block_invoke_2 (in Foundation)
  7  0x00007fff90a36a82 _dispatch_call_block_and_release (in libdispatch.dylib)
  8  0x00007fff90a37961 _dispatch_worker_thread2 (in libdispatch.dylib)
  9  0x00007fff8b8b33da _pthread_wqthread (in libsystem_c.dylib)
 10  0x00007fff8b8b4b85 start_wqthread (in libsystem_c.dylib)

make: *** [/Users/jwalke/Desktop/vico/build/DEBUG/sparkle/Sparkle.stamp] Abort trap: 6

@Shadowfiend
Copy link
Member

Er… That looks odd. Try re-making? May also want to make sure you've updated the sparkle submodule or whatever… I forget what the crazy git syntax for that is.

As for getting it to work non-modally—what I'm thinking is that we'd let the ViTextView deal with the non-control keys as usual, but also forward them and control keys to the completion controller… Or something like that. That way we'd get typing in the main window as usual, plus handling by the completion controller of whatever else is needed. Still not 100% sure about that, we may need to do some key map shuffling or whatever. The main thing is that the text view would not wait for the completion controller to report a result back before handling typing again, it would instead continue handling typing and the completion controller would process enter/tab and send a message to the text view indicating a completion was selected.

The biggest problem right now is that the text view and completion stuff aren't just used in the regular text views, they're also used in the command box. That's where things get hairy, because we may not necessarily want the two to behave the same. The options there get nasty, because they basically involve using a separate subclass of ViTextView for command line stuff. That's how I see it from the work I've done so far. There may be another alternative.

We're going to try a proper implementation of complete as
you type, which will involve slightly differing behaviors for
the completion popup depending on which mode it's invoked
in, so we need this preference in place.
@ghost ghost assigned Shadowfiend Jul 27, 2013
We missed a spot, so depending on the path you took the
completion controller could still aggressively choose to
close the window based on only having 1 completion, even
if completion was initiated in a non-aggressive way.
We pass in the text view's key manager when initializing
the completion controller, and it goes on to pass keystrokes
back to the text view. This way, we're updating the contents
of the text view as the user types, in addition to filtering the
completion view itself.

This replaces the old
completionController:appendedStringWithoutCompleting:
hack, which was totally broken anyway.
We also only trigger it once at a time. So if the completion
window is already up, we don't try to bring it back up, which
will reset its state. Instead, we track if we've currently got
a window up (which is easy since it runs modally) and don't
try again until it's gone.
We now check in the central presentCompletionsOf: method
instead of checking only when checking the autocomplete
trigger, since we've changed how completions work in
general. We also update the flag in presentCompletionsOf:
as well.
We add a selector, accept_if_not_autocompleting:, to
the completion controller. When invoked via autocomplete,
the completion controller gets a C option, which we check
for and then set on an instance variable. That variable is
then checked when invoking accept_if_not_autocompleting:
to see if the selected entry should be accepted or not.

The effect of this is that invoking autocomplete via C-N will
accept on space, since you explicitly asked for the
completion popup, while if it pops up via autocomplete it will
only accept on enter/tab. The additional effect is that this is
customizable behavior -- the relevant keys can simply be
remapped to accept: or accept_if_not_autocompleting:
as desired.
Now that we're filling in keys to the text view as they're
being procesed by the completion controller as well, we
need the range to reflect the fact that all prefix/filter text in
the completion controller needs to be replaced with
whatever completion was selected.
Because all keys are sent to the text view anyway, there's
no reason at all to handle the terminating key after the
completion popup goes away, as it's already been handled.
For now, we would end up with bad completions anyway
because we always use a ViWordCompletion when
autocompleting.
When we complete aggressively, we go through a separate
return flow if there is only one completion available. We
weren't setting up the range properly for that situation.
@jordwalke
Copy link
Author

That first issue issue was definitely related to my key mappings.

I'm still having trouble having escape and closing the completion window using a single press of escape (when it popped up automatically). I've disable every line in my site.nu file. Any other suggestions?

There's also a strange behavior when completing at the beginning of a word. I really like how you still allow completions at the beginning of text (Sublime gets this wrong - vim's ACP gets this right - this plugin gets it right too).

If you have:

apple
<cursor>parachute

Then typing a makes aparachute appear in the completion list. I believe only apple should show up (that's how other completers seem to work and it ends up being a reasonable behavior).

@Shadowfiend
Copy link
Member

Heh, frankly, I haven't tested the behavior at the beginning of a word at all, and it's more accidental than intentional. That implies we'd want the current word to be considered whatever the current word is up to the cursor. If you don't mind, I'd rather fix the snippet issue, consider this particular feature complete, and then consider that change in behavior as a separate issue.

@Shadowfiend
Copy link
Member

It does seem like you're right about escape. I could have sworn I had that working, but apparently it was too late and my mind was fooling me. More to come.

@jordwalke
Copy link
Author

Sounds fine!

This ensures anyone else who relies on it in the rest of the
input handling chain (which is blocked by the completion
popup's modality) will have the right value. In this case, we
can just set final_location to the caret, since the completion
popup method takes care of updating the caret appropriately
when/if the text insertion step wraps up.
This ensures we don't cancel the current snippet when we're
autocompleting while inside one, but it also ensures we
don't do the same when we explicitly invoke the completion
box, which was an existing bug.
Before, when the user cancelled the completion popup
during autocomplete, we tried to pass on the cancellation
key combo to the existing key manager. Unfortunately we
did this after the completion controller had been reset, which
meant there was no key manager to send it to. We now
save the key manager before resetting and then pass it the
keys after we run the reset. This is so that the key manager
receives the keys after the completion controller is fully
dismissed.
@Shadowfiend
Copy link
Member

Ok, I think the snippet issue is fixed. I also think the escape-while-autocompleting issues are actually legitimately fixed -.-

We were passing the keys that cancelled the popup to the existing key
manager in situations where we were running
accept_if_not_autocompleting: and the completion popup *was*
autocompleting. This typically meant we'd get two spaces instead of just
one when we'd hit space, which should cancel the popup but not accept
the top completion when autocompleting.
@Shadowfiend
Copy link
Member

Fixed a couple of more bugs. However, the . command is still busted with autocomplete (in fact, it was busted with completion before as well—a consequence of the fact that we're tracking key sequences in insert mode for . rather than tracking the final string that was inserted).

virtualInput indicates input that is not part of a macro or
dot replay that is still not being inputted directly from the
keyboard. Initially, we'll be using this to input the completion
from the completion popup in a way that allows the dot
command to work properly. The autocompletion popup doesn't trigger
when virtualInput is set to YES, much like it won't trigger when
replayingInput is set to YES. However, we WILL record keys inputted
virtually when virtualInput is set to YES (whereas we do not when
replayingInput is set to YES).
Completions are now inserted by virtually playing the keys
that the completion would add to the current text as actual
keys typed by the user. We set virtualInput to YES while we
do this. This allows the keys to be recorded for the dot
command to replay, which fixes dot commands with
autocomplete.
@Shadowfiend
Copy link
Member

Breakthrough. Completely redid how completions are inserted, which fixes the . command for both auto and manually-triggered completion. Boomtimes.

@jordwalke
Copy link
Author

Trying now!

@jordwalke
Copy link
Author

I would definitely be proud of your . redo logic combined with autocomplete. I couldn't get it to do the wrong thing after a couple of minutes of trying. Yours could possibly be the very first autocompletion in any Vim-like environment to get that right. Even Vim itself gets that wrong (admittedly because of a bug in Vim that makes it impossible to do correctly).

I just tried some more testing and all the issues seem to be ironed out except for the snippets.

One question:
How tightly coupled is all of the logic to the popup window? I suspect we'll eventually want to be able to paint the popup in very different ways (since the popup makes the application window inactive - the close/minimize OSX dots in the upper left corner change color as the completion pops up). But since the general logic here feels pretty solid (once the snippets are fixed) it would be nice to be able to use it outside of the popup mechanism.

@Shadowfiend
Copy link
Member

w00t. Good to hear.

What exactly is the issue with the snippets? I didn't get a chance to test it, but it's possible the new completion logic that fixes . re-broke that aspect, which would be sad-face.

As for the logic, it's currently very much structured around the fact that the popup window is modal. That said, the core functionality doesn't rely on it; with some rearrangement, it could be done differently. I agree it's a serious annoyance that the popup is modal for a variety of reasons; the reason it's done that way seems to be essentially so that the completion popup is the one getting the keystrokes, so that it can have its own key mappings. Modifying that will be interesting.

We were passing an non-updated range for partial
completions, which could lead to odd behavior when running
partial completions e.g. in the ex field.
We were still doing the old replaceRange-based completion
insertion; this switches it to the new virtual input insertion
style.
@jordwalke
Copy link
Author

Here's a repro of the snippets issue:

Open a .js file.
Type for<Tab> and then start typing anything.

Because the completion dialog is modal, processing for that
particular keystroke stops until the completion popup goes
away in the situation where we show the completion popup.
This means we never get a chance to show that the caret
moved to after the currently inserted character. We now
update the caret explicitly before showing the completion
popup so that it updates correctly in the UI. This fixes a
couple of other bugs with snippets, as well.
Before we completed partially in that situation, but that
meant the popup would never go away if there were multiple
matches for the current completion with the same length.

That made triggering a snippet when typing if in a file where
both if and IF were being used, for example, completely
impossible
@Shadowfiend
Copy link
Member

I'm currently running the code with the couple of changes that I've pushed in the last hour, which were addressing some issues directly and indirectly related to snippets, so maybe give that a whirl in case that changed things.

As it is, following your steps, I get a menu prompting me for two different for loop variants and after I choose one things seem to work as expected, with tab moving among the appropriate things. If you tell me what behavior you're seeing and what behavior you expect to see instead, maybe I can get a better idea of what might be happening.

@jordwalke
Copy link
Author

It seems like something in your last update has fixed the snippets behavior! I can't see any glaring issues with this (though I've only given a few minutes).

@Shadowfiend
Copy link
Member

Sweet! I'm messing around with some stuff and running this branch merged with a couple of others, so I'll probably keep using it for a couple of days and if everything is still feeling good, merge it in. With that in mind, please let me know if you run into any serious problems with it so I can reset that process :)

@Shadowfiend Shadowfiend mentioned this pull request Aug 5, 2013
Shadowfiend added a commit that referenced this pull request Aug 19, 2013
Autocomplete as you type

There is now an option, "Complete as you type", that can be set in Editing options.
This means it can be set per-file-type, as well. It runs word-based completion as
currently triggered by C-N after two characters of a word have been typed.

This also implements completion so that typing while a completion popup comes
up also goes into the editor window, which did not used to happen.
@Shadowfiend Shadowfiend merged commit 6634d2b into master Aug 19, 2013
@Shadowfiend Shadowfiend deleted the completion-as-you-type branch August 19, 2013 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants