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

Unable to bind control-space key command #3189

Closed
ghost opened this issue Jul 1, 2016 · 6 comments
Closed

Unable to bind control-space key command #3189

ghost opened this issue Jul 1, 2016 · 6 comments

Comments

@ghost
Copy link

ghost commented Jul 1, 2016

I've tried binding both "\c\ " and "\c\x20", however none of these seem to work.

Fish_key_reader (and bash escape sequence) reports that control-space is

hex:    0  char: \c@
bind \c@ 'do something'

Yet trying to bind \c@ results in

Invalid token '\c@'
fish: bind \c@

Substituting @ for its hex equivalent doesn't seem to work either.

Reproduction Steps:

  1. Try to bind control-space using any of the methods above

Fish version: fish, version 2.3.0

Operating system: OSX 10.11, Homebrew install

Terminal or terminal emulator: OSX terminal app

@krader1961 krader1961 added the bug Something that's not working as intended label Jul 1, 2016
@krader1961 krader1961 added this to the fish-future milestone Jul 1, 2016
@krader1961
Copy link
Contributor

Clearly a bug. You can do the equivalent via bind \x00 'echo hello'. Now press [ctrl-space] or [ctrl-@]. Unfortunately that reveals a second bug since your shell will now be spinning busily writing "hello" to the tty. And there's a third bug. If you do

bind \c\  'echo hello'

Then bind | grep 'echo hello' you'll see the \c has been mapped to an unexpected sequence:

bind \x1c\  'echo hello'

P.S., Note that tmux lets you bind [ctrl-Space]. For example, I have the following in my ~/.tmux.conf:

set-option -g prefix C-Space
bind C-Space send-prefix

@ghost
Copy link
Author

ghost commented Jul 13, 2016

Hm I think I found a way to patch this.

Currently fish relies on the empty binding '' as a fallback when no other pattern is matched, with the action of inserting (R_SELF_INSERT). This, however, precludes the possibility of defining a separate pattern for the null character as a pattern (which is what control-space inputs) since string manipulation in C relies on the fact that strings are null-terminated.

The quick way to fix this is to modify void input_mapping_execute_matching_or_generic(bool) to the following, essentially defining our own fallback instead of relying on one in the keybinding list.

static void input_mapping_execute_matching_or_generic(bool allow_commands)
{
    const wcstring bind_mode = input_get_bind_mode();

    for (int i = 0; i < mapping_list.size(); i++) {
        const input_mapping_t &m = mapping_list.at(i);

        // debug(0, L"trying mapping (%ls,%ls,%ls)\n", escape(m.seq.c_str(), ESCAPE_ALL).c_str(),
        //           m.mode.c_str(), m.sets_mode.c_str());

        if (m.mode != bind_mode) {
            // debug(0, L"skipping mapping because mode %ls != %ls\n", m.mode.c_str(),
            // input_get_bind_mode().c_str());
            continue;
        }

        // if (m.seq.length() == 0) {
        //     generic = &m;
        // } else 
        if (input_mapping_is_match(m)) {
            input_mapping_execute(m, allow_commands);
            return;
        }
    }

    if (&default_selfinsert_mapping) {
        input_mapping_execute(default_selfinsert_mapping, allow_commands);
    } else {
        // debug(0, L"no generic found, ignoring...");
        wchar_t c = input_common_readch(0);
        if (c == R_EOF) input_common_next_ch(c);
    }
}

where we have defined in input_init():

default_selfinsert_mapping = input_mapping_t(L"", commands_vector, DEFAULT_BIND_MODE, DEFAULT_BIND_MODE);

Once the default insert action has been decoupled from the binding list, we have to modify input_mapping_is_match(const input_mapping_t&) to properly deal with null patterns.

/**
   Try reading the specified function mapping
*/
static bool input_mapping_is_match(const input_mapping_t &m)
{
    wint_t c = -1;
    int j = 0;
    bool atLeastOneMatch = false;

    //debug(0, L"trying mapping %ls\n", escape(m.seq.c_str(), ESCAPE_ALL).c_str());
    const wchar_t *str = m.seq.c_str();

    do
    {
        bool timed = (j > 0 && iswcntrl(str[0]));

        c = input_common_readch(timed);
        //printf("%lu\n", c);
        if (str[j] != c)
        {
            break;
        }
        else
        {
            atLeastOneMatch = true;
        }
        j++;
    }
    while (str[j] != L'\0');

    if (str[j] == L'\0' && atLeastOneMatch) //Ensures that a pattern of \0 only matches an input of \0
    {
        //debug(0, L"matched mapping %ls (%ls)\n", escape(m.seq.c_str(), ESCAPE_ALL).c_str(), m.command.c_str());
        /* We matched the entire sequence */
        return true;
    }
    else
    {
        int k;
        /*
          Return the read characters
        */
        input_common_next_ch(c);
        for (k=j-1; k>=0; k--)
        {
            input_common_next_ch(m.seq[k]);
        }
    }

    return false;
}

Now you should be able to bind \x00 'echo test' (fixing the invalid token for \c@ remains to be done though).

@krader1961 Could you test the below patch file and if it works, merge into the repo?

FISH-3189.patch.zip

@faho faho modified the milestones: next-2.x, fish-future Aug 4, 2016
@krader1961 krader1961 modified the milestones: next-minor, fish 2.5.0 Jan 2, 2017
@krader1961 krader1961 added enhancement and removed bug Something that's not working as intended labels Apr 1, 2017
@krader1961 krader1961 modified the milestones: fish-future, 2.6.0 Apr 28, 2017
@nimashoghi
Copy link

Are there any updates on this? I'm an ex-PowerShell user, and I find myself constantly using ctrl+space to match the current behavior of Fish's tab. I was hoping that I'd be able to bind ctrl+space to complete, but I ran into this issue.

@ridiculousfish
Copy link
Member

The basic problem here is that fish builtins can't distinguish between an empty string and a string containing the null character. I don't think we should be baking in fish's confusion here either.

I think the simplest approach is to give the nul-string a key name (as in bind --key-names).

@ridiculousfish
Copy link
Member

Fixed in 533ee65 . This can now be done via bind -k nul.

ridiculousfish added a commit that referenced this issue Sep 15, 2019
@floam
Copy link
Member

floam commented Sep 15, 2019

Awesome!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants