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

Unexpected keybinding config error #4992

Closed
nurbles opened this issue Mar 18, 2020 · 10 comments
Closed

Unexpected keybinding config error #4992

nurbles opened this issue Mar 18, 2020 · 10 comments
Labels
Issue-Question For questions or discussion Needs-Tag-Fix Doesn't match tag requirements Resolution-Answered Related to questions that have been answered

Comments

@nurbles
Copy link

nurbles commented Mar 18, 2020

Environment

Windows build number: [Version 10.0.18363.720]
Windows Terminal version (if applicable): 0.10.761.0

Any other software? not sure what this is asking

Steps to reproduce

This is happening any time I start terminal -- not a new tab in terminal, but starting the very first terminal window.

Expected behavior

I expect the window to open as it had done earlier in the day -- without popping an error message.

Actual behavior

All of a sudden (from my point of view) starting up terminal presented me with an "Encountered errors while loading user settings" message (I will try to attach a screen shot of the message and a copy of my settings json file). The thing is, I have been working with VS2019, my own software, notepad++, outlook and other stuff all day, with most things (especially terminal) being started and stopped multiple times. I have not even looked at the settings for anything, so there is no reason for them to have changed -- I never did it myself, nor did I authorize anything else to do it for me. My system has not rebooted, though God only knows what Windows Update may have done in the background without telling me...
terminal-error
profiles.json.txt

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Mar 18, 2020
@DHowett-MSFT
Copy link
Contributor

DHowett-MSFT commented Mar 18, 2020

It looks like your profiles.json file has contained some invalid settings for a while, and the new version of Terminal is correctly calling those invalid settings out.

      "keybindings": [
        {
          "command": "closePane",
          "keys": [
            "shift+f4",
            "ctrl+shift+w"
          ]
        },
        {
          "command": "copy",
          "keys": [
            "ctrl+ins",
            "ctrl+shift+c"
          ]
        },

An entry in "keybindings" cannot have multiple keys. That feature is intended for key chording, such that you can configure "shift+f4 ctrl+shift+w" in sequence to trigger closePane.

I have not even looked at the settings for anything

Terminal has never shipped a settings file with multiple keys set for the same action, so I'm going to say that some software on your system must have made this change.

Incidentally, your settings file looks like it's very old (pre-version-0.5). The eminent Scott Hanselman wrote up a good blog post on why it might be a good time to start anew.

@DHowett-MSFT DHowett-MSFT added Issue-Question For questions or discussion Resolution-Answered Related to questions that have been answered and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Mar 18, 2020
@nurbles
Copy link
Author

nurbles commented Mar 18, 2020

Thanks for the info.

I may have added those extra bindings at the direction of a post here. It is unlikely that I 'invented' them myself. (As a rule, I try to avoid directly looking at JSON files almost as much as I avoid XML -- I don't want to go blind!) I remember making a couple changes to the default shell, the font and default rows/columns, but I don't remember doing anything more. I might have, but I don't remember.

As for "starting anew" ... that's the kind of thing I tend to trust the application updates to take care of, even if they need to ask me for "assistance" in what to keep, lose or change.

I guess the error just "appeared" because terminal was silently updated by Windows Update (or something like it) between two runs of the program today. Gotta love it, I suppose -- do I have a choice?

@DHowett-MSFT
Copy link
Contributor

I'd love for us to be able to take care of "starting anew" for you, but this is one of those cases where we explicitly decided that the user's settings file was their property and not ours. We used to touch it a whole bunch, to add and remove things, and that didn't end up being very scalable (or pleasant for anyone!)

Alas, quiet automatic updates are the Store's doing. Sorry about that.

@nurbles
Copy link
Author

nurbles commented Mar 19, 2020

So, are there any instructions or documentation for this "starting anew" procedure? I would guess that the best way should be to uninstall and then reinstall, but since so few programs actually clean any old stuff out during either of those processes, I have little hope that would work.

Related to the error message: It was VERY surprising that I was able to type commands BEHIND the error popup! Apparently the error does not have the keyboard focus, even though the different brightness levels of the shell and error seem to indicate otherwise. I think THAT is an actual bug.

FWIW, I like the terminal for a simple reason: it loads faster than CMD while still supporting (most of) the things I did with CMD. I am not interested in hacking away (and learning a whole new language in the process) directly within a JSON configuration file. I do everything I can to avoid JSON (and XML).

@nurbles
Copy link
Author

nurbles commented Mar 19, 2020

OK, I started "anew" by renaming profiles.json and restarting terminal. Then I moved over the few things I changed (font face/size, rows/columns, default shell). Works great. I have been reading the docs about the configuration file and came across this line about key bindings:

This is an array of key chords and shortcuts to invoke various commands. Each command can have more than one key binding.

This appears to directly conflict with your earlier statement that only one binding is allowed. Given the error message, perhaps the documentation should be corrected to reflect the (new?) reality.

The quote may be found just after Key Bindings on this page: UsingJsonSettings

Thanks again for all your help and to everyone working on this nifty tool!

@zadjii-msft
Copy link
Member

@nurbles No, that documentation is actually saying what we want there. You can have multiple keys bound to the same action, but they should each be in their own bindings. For example:

    "keybindings" :
    [
        { "command": "copy", "keys": ["ctrl+shift+c"] },
        { "command": "copy", "keys": ["ctrl+c"] },
        { "command": "copy", "keys": ["enter"] },
        { "command": "paste", "keys": ["shift+insert"] },
        { "command": { "action": "splitPane", "split":"auto", "splitMode": "duplicate" }, "keys": [ "ctrl+alt+t" ] }
    ]

In this snippet from my keybindings, all three of ctrl+shift+c, ctrl+c and enter are bound to copy

zadjii-msft added a commit that referenced this issue Mar 19, 2020
@nurbles
Copy link
Author

nurbles commented Mar 19, 2020

Ah-ha! Thanks for the clarification. Also, thank's for allowing comments in terminal's JSON file! They are super helpful to remind me how the bits work and what I changed, since I almost never look at the thing.

@nurbles
Copy link
Author

nurbles commented Mar 20, 2020

Just curious... Why does terminal allow multiple bindings for each key, but only defined as a set of separate definitions with an array of bindings that only ever has ONE element? Since I'm being forced to learn about JSON in order to make sure my configuration changes are correct, everything I have found says that the
{ "command": "copy", "keys": ["ctrl+shift+c"] },
syntax is defining an array of "keys" for the "commnd":"copy", but terminal only allows one element in the array. Then this is even more strange:
{ "command": "copy", "keys": ["ctrl+shift+c"] }, { "command": "copy", "keys": ["ctrl+c"] }, { "command": "copy", "keys": ["enter"] },
This is defining a set of three different keys (and chords) for the command copy by using THREE ARRAYS with ONE ELEMENT each! It seems to me that if you are going to use JSON, you might as well actually use it in the way it is defined instead of having a strange restriction like this. I have not tried it yet, but the documentation says that the one element arrays OR non-array settings may be used -- so all the square brackets above could be removed. So I think the documentation (and any sample JSON file(s)) should NOT use the array format when more than one value is not allowed. That's just confusing.

(Sorry about the lack of CR/LF on the second example. I tried numerous ways to get this things to allow it, but it refused.)

@zadjii-msft
Copy link
Member

There's not a great reason here. Basically, we knew that one day, we'd like to support keychords with multiple keystrokes in them, like pressing Ctrl+B,Ctrl+1 to do an action. Since we knew we'd one day need multiple keys in the array, we made it an array to begin with.

Then we realized

  1. We're not going to be able to do chords like this before 1.0
  2. We can trivially parse the JSON differently for a single string value vs an array value

So older versions of the settings file will have an array for the keys property, while newer versions will just use a string, like

    "keybindings" :
    [
        { "command": "copy", "keys": "ctrl+shift+c" },
        { "command": "copy", "keys": "ctrl+c" },
        { "command": "copy", "keys": "enter" },
        { "command": "paste", "keys": "shift+insert" },
        { "command": { "action": "splitPane", "split":"auto", "splitMode": "duplicate" }, "keys": "ctrl+alt+t" }
    ]

Both this and the snippet I have above are treated the same way.


PS: markdown protip: you can use "code fences" to have multiple lines of code in your comments. By using something like:

```
This is code
With multiple lines
```

You can get content that looks like

This is code
With multiple lines

@nurbles
Copy link
Author

nurbles commented Mar 20, 2020

Thanks for the info about the arrays, it makes lots more sense when the multiple keystroke concept is added. Add I had seen before was comments about defining keys and key-chords that (to me) seemed to imply that the chords might need an array.

I guess I don't know git markdown well enough for advance code blocks. I foolishly trusted the 'insert code' button on this form to allow that, but it only inserts a single pair of backticks with the cursor placed between them, I guess for inline code examples. I was fooled into thinking the tool I was given did what I needed. Silly me. (smile)

zadjii-msft added a commit that referenced this issue Mar 27, 2020
* Add a note about Binding multiple keys

From discussion in #4992

* Update doc/user-docs/UsingJsonSettings.md

Co-Authored-By: Josh Soref <jsoref@users.noreply.github.com>

* update the comment here to be a little clearer

Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Question For questions or discussion Needs-Tag-Fix Doesn't match tag requirements Resolution-Answered Related to questions that have been answered
Projects
None yet
Development

No branches or pull requests

3 participants