sublimehq / sublime_text

Issue tracker for Sublime Text
https://www.sublimetext.com
804 stars 39 forks source link

Typing a close-parenthesis doesn't exit autocompletion of function #579

Open Draknek opened 9 years ago

Draknek commented 9 years ago

I have a function updateCameraTarget with one optional parameter immediate.

If I type out updateCameraTarget(), the opening bracket opens the function autocomplete and the closing bracket replaces the autocomplete prompt for the immediate parameter with a closing bracket.

image

Operating System: Windows 8.1, Ubuntu 10.04 Sublime Text 3 (version 3065)

I think this is a core Sublime Text issue, but I only have one language set up with function autocompletion, so I can't be certain it's not something specific to Haxe (the haxe-sublime-bundle package).

FichteFoll commented 9 years ago

?immediate : Bool looks like a selection to me. If that's the case, typing anything will obviously replace it, unless a package overloaded the keybinding with some context restrictions. Your best action would be to press the right key once before typing ) (or just press it twice instead).

I highly doubt this is a ST issue, so which autocomplete plugin(s) are you using and what languages do(es) it/they work for?

Draknek commented 9 years ago

Isn't the autocompletion behaviour built-in? Obviously the information being autocompleted needs to come from a language-specific plugin, but there's generic API commands for has_next_field and the like.

The specific language I'm using is Haxe, with this package: https://github.com/clemos/haxe-sublime-bundle.

gerardroche commented 9 years ago

If you don't want the selected field, i.e. ?immediate : Bool, press del and then ).

I think Sublime Text could be a bit smarter in this case and do what you say.

Draknek commented 9 years ago

Yes it's work-around-able, but my problem is that I'm typing () with muscle memory: by the time I've had the thought "oh I need to press delete between the opening and closing parentheses" I've already made the mistake.

I tried writing a key binding to fix this behaviour, but it looked like there wasn't a specific command to do what I'd need to do, so I'd need to run three commands from the same binding which doesn't seem to be possible (clear the autocomplete state, delete the selected text, move the cursor to after the closing parenthesis).

FichteFoll commented 9 years ago

Just so I get this right, you get the second line in your screenshot after typing ( and it autocompletes the parameter.

What do you expect to happen when you press ) immediately after that?

Draknek commented 9 years ago

That's correct.

I expect the editor to notice that I'm closing the function call, get rid of the autocomplete placeholder, and put the cursor after the ) character.

FichteFoll commented 9 years ago

I was experimenting with key binding contexts but then I realized that doing what I needed was not supported, so instead I quickly wrote an eval context myself.

In order to do what you want, you'll need to create the following keybinding and files:

    { "keys": [")"], "command": "run_macro_file", "args": {"file": "Packages/User/Delete Right and Move Right.sublime-macro"}, "context": [
        { "key": "setting.auto_match_enabled", "operator": "equal", "operand": true },
        { "key": "selection_empty", "operator": "not_equal", "operand": true, "match_all": true},
        { "key": "eval: view.substr(sel.end())", "operator": "equal", "operand": ")", "match_all": true}
    ] },

Packages/User/Delete Right and Move Right.sublime-macro

[
  { "command": "clear_fields" }, // this one is optional
  { "command": "right_delete" },
  { "command": "move",
    "args": { "by": "characters", "forward": true } }
]

Packages/User/eval_context.py

import sublime
import re
import sublime_plugin

def check_value(value, operator, operand):
    if operator == sublime.OP_EQUAL:
        return value == operand
    elif operator == sublime.OP_NOT_EQUAL:
        return value != operand
    elif operator == sublime.OP_REGEX_MATCH:
        return re.match(operand, value) is not None
    elif operator == sublime.OP_NOT_REGEX_MATCH:
        return re.match(operand, value) is None
    elif operator == sublime.OP_REGEX_CONTAINS:
        return re.search(operand, value) is not None
    elif operator == sublime.OP_NOT_REGEX_CONTAINS:
        return re.search(operand, value) is None
    else:
        raise Exception('Unsupported operator: ' + str(operator))

class EvalContextListener(sublime_plugin.EventListener):

    glob = {}

    def on_query_context(self, view, key, operator, operand, match_all):
        if not key.startswith('eval:'):
            return None

        expression = key[5:]

        window = view.window()

        for sel in view.sel():
            loc = {"window": window, "view": view, "sel": sel}
            value = eval(expression, self.glob, loc)
            result = check_value(value, operator, operand)

            if not match_all:
                return result
            elif not result:
                return False

        return True
FichteFoll commented 9 years ago

On another note, I'd suggest the Haxe plugin to insert completions if the form (${1:completion})$0 so that pressing tab will move you to the end of the function call.

Your habit of pressing ) immediately after ( is user preference and should not be the default, imo. That's why ST is so extensible, so that you can build things like that on your own.

gerardroche commented 9 years ago

I think it also solves what I describe below, mostly. If the selection does not include the comma-space before the field then it also needs to be deleted e.g.: (selection between pipes)

date_sunrise('2014-11-26', 'yyyy-mm-dd', |latitude|, longitude, zenith, gmt_offset)

I was writing the post below as you posted the above so I'll post it anyways. But your code looks like it solve the problem.


I think optional fields are tricky and could certainly be improved. I've been wrangling with this for a while, still not sure what the ideal solution is. ) as signal that you're finished with the snippet might work well in most cases.

An expanded example with multiple optional fields has several issues:

{"trigger":"date_sunrise()\tdate Function","contents":"date_sunrise(${1:time}${2:, ${3:format}${4:, ${5:latitude}${6:, ${7:longitude}${8:, ${9:zenith}${10:, ${11:gmt_offset}}}}}})"}

The first field is required. the rest are optional.

date_sunrise tab

date_sunrise(|time|, format, latitude, longitude, zenith, gmt_offset)

At this point pressing ) needs to delete everything inside the parens and put the cursor after ).

But what if we fill in the required date:

date_sunrise('2014-11-26|', format, latitude, longitude, zenith, gmt_offset)

At this point, if pressing ) were to delete remaining fields and go to the end it would be unexpected because you might want the ) as part of the date field string.

Nonetheless, tab gives you:

date_sunrise('2014-11-26'|, format, latitude, longitude, zenith, gmt_offset|)

The current selection also contains the comma because it's part of the optional group of fields. From here you could tab to the next field fill it in and continue on with as many as you want. Then the fastest way to exit the field cycle and get to the end is del then ). Having to only press ) would be better.

Let's include one more: tab

date_sunrise('2014-11-26', |format|, latitude, longitude, zenith, gmt_offset)

Fill it in:

date_sunrise('2014-11-26', 'yyyy-mm-dd|', latitude, longitude, zenith, gmt_offset)

tab

date_sunrise('2014-11-26', 'yyyy-mm-dd'|, latitude, longitude, zenith, gmt_offset|)

You can probably by now see why the comma-space is required as part of the selection. It gives a chance to del, then ) to get to the end of the completion.

If ) were to remove remaining fields I think it might solve this issue too.

Draknek commented 9 years ago

Instead of requiring this eval context, this is a simpler rule which works for me:

{ "keys": [")"], "command": "run_macro_file", "args": {"file": "Packages/User/close-paren-close-autocomplete.sublime-macro"}, "context": [
    { "key": "setting.auto_match_enabled", "operator": "equal", "operand": true },
    { "key": "selection_empty", "operator": "not_equal", "operand": true, "match_all": true},
    //{ "key": "eval: view.substr(sel.end())", "operator": "equal", "operand": ")", "match_all": true}
    { "key": "has_next_field", "operator": "not_equal", "operand": true, "match_all": true},
] }
FichteFoll commented 9 years ago

If it works for you, then it's okay of course. I was just making the context more restrictive since I can imagine situations where your binding would trigger errorneously.

I'm closing this then.

clemos commented 9 years ago

Hi, FYI I solved the issue using :

{ "keys": [")"], "command": "run_macro_file", "args": {"file": "Packages/Haxe/Macros/input/CloseParen.sublime-macro"}, "context":
        [
            { "key": "setting.auto_match_enabled", "operator": "equal", "operand": true },
            { "key": "selection_empty", "operator": "not_equal", "operand": true, "match_all": true }
       ]
}

Packages/Haxe/Macros/input/CloseParen.sublime-macro

[
    {
        "command": "insert",
        "args":
        {
            "characters": ""
        }
    },
    {
        "command": "move",
        "args":
        {

            "by": "characters",
            "forward" : true
        }
    }
]

I think you should consider adding it as the default ST behaviour, as auto_match + selection indeed feels weird, currently. I haven't been able to detect if the following character is ) though...

clemos commented 9 years ago

I have been able to detect ), using another approach. The macro now inserts ), and then checks for ), in which case it removes it.

[
    {
        "command": "insert",
        "args":
        {
            "characters": ")"
        }
    },
    {
        "command": "right_delete",
        "context": [
            { "key": "following_text", "operator": "regex_contains", "operand": "^\\)", "match_all": true }
        ]
    }
]
FichteFoll commented 9 years ago

Contexts work in macros? Cool, didn't know that.

The last macro is okay and could potentially be added to ST, the ones before were to specific. I highly doubt it would be added, but we can keep this issue open I guess.

clemos commented 9 years ago

The (first) key binding is not so specific: I copied it from ST Defaults, and just changed selection_empty to false to cover this case.

FichteFoll commented 9 years ago

Actually, I think I meant the opposite. The binding&macro were too unspecific and certain conditions would have to be met in order to have them function in an intuitive way. (If there was no closing parens after the current selection it would behave "weird" so to speak.)