Fix: Cancel syntax test scope completions

[deathaxe]

This PR fixes #146 by resetting is_completing_scope if the completion popup is closed.

The following situations reset the scope completion now:

• deactivating the view
• move away from completion by cursor ("move")
• move away from completion by mouse ("drag_select")

[r-stein]

I don't think the point-keybinding solution is better, because someone may (for some reasons) want to insert put (or whatever) and writes it and presses . and it inserts punctuation. instead of put. .

[deathaxe]

This can "easily" be fixed by adding the meta.scope to the context selector. Unfortunately @FichteFoll is very strict with applying those metas (not including leading and tailing whitespace) which results in IMHO too tiny meta blocks. Means in this case, if the cursor is at the eol, the meta.scope is already popped off and can't be used in the selector at the moment.

It requires the syntax file to be modified to include the tailing \n of a line into the meta.scope as it is done with comments in many default syntax packages already.

The syntax files are not too easy so I leave it to @FichteFoll who will modify the content of the PR to match his needs and desires anyway 😉

So this is a proof of concept only.

[deathaxe]

... I leave it to @FichteFoll who will modify ...

Ok, here's a little fix, which seems to work.

[r-stein]

No, that's nothing you can fix via scopes. If you insert put and the auto completions contains punction it will be selected and hence be inserted when you press . . I don't think overwritting the . keybinding to commit the auto-completion is a good idea and people wouldn't expect that behavior.

[deathaxe]

The one who suggested it first is totally opposed in the end. Odd.

[deathaxe]

... but I can reproduce what you describe and understand your concerns!

Using ctrl+. is no option. No-one would know that and it is absolutely uncommon, I think.

We'd finally end up in modifying the key binding to use tab key and need to create a separate text command, which checks whether the base-scope is reached to decide, whether to trigger the next completion or not.

This way the behavior wouldn't change.

[FichteFoll]

I'm with @r-stein on this. Overriding the . binding to commit the current completion before inserting the next dot is pretty much the opposite of what I would want here, since the dot would be the character to signal that the exact current text should be kept as is and a new segment should be started, as opposed to explicitly committing the completion with tab or enter. @r-stein initially suggested to use a different binding, ctrl+., which does basically the same but has to be requested explicitly since it's not the default. Except, as you mentioned, nobody would use it since it's unitntuitive.

I'd rather extend on https://github.com/SublimeText/PackageDev/pull/152/commits/c8f400f93a390cf1ce8ed125cdab31dcf16d5b15.

[deathaxe]

Therefor I went on to create the text command I mentioned in the latest 2 commits, which revert to the current behavior but without a global pending state which can hang around forever somehow. The functionality is even pretty much the same as with the previous EventListener.

I honestly find an EventListener which checks each command ever called, just to commit this completion a bit misplaced. Especially as it hacks around to get access to the view event listener, which I find bad practice.

[FichteFoll]

You're right, this is cleaner from an architectural standpoint and doesn't involve having to mess with command listeners and inferring state from a separete view event listener instance, because those can't hook on_text_command.

It has some disadvantages, however:

1. We extend the meta.scope scope further than semantically necessary, but that's a sacrifice I can make easily.
2. It only works with the keyboard, not when a user selects the completion with the mouse.
3. It doesn't consider the user potentially rebinding their commit_completion bindings, although I doubt anyone would realistically do this.

Disregarding 1. and 3. due to their insignificance, I have to wonder how many people are using the mouse to select completions and whether we want to support this user pattern. Considering that the popup will still open once you enter a dot after the completion was inserted, I don't consider this suppoort to be necessary. Also, if they're using the mouse to select a completion, surely they can manage having to press an additional key and us trying to optimize that away would not help them.

I'll play around with this some time later.

[deathaxe]

We extend the meta.scope scope further than semantically necessary, ...

This is a general issue of meta.scopes when they are required for key bindings or something like that at the end of line or end of the sematical scope.

You mention some things I did not have in mind. Seems any solution has its pros and cons. Not sure which one would be the better way.

[r-stein]

I wasn't really suggesting something, but shared my keybinding for that specific purpose. I was hoping someone may do something useful with it, but had nothing specific in my mind.

[deathaxe]

This enabled us to try a new way to solve the solution and find/discuss the pro and cons of both. I found this very constructive in the end.