gpui: Fix keybindings not working as expected

[CharlesChen0823]

Closes #18796

I think the whole problem is using map_while will discard all action for the depth is up the current context, which is not expect. we should only discard action for the same context when setting keybindings to null.

the code is might not well, wouldyou have time for improve this?

@ConradIrwin

Release Notes:

• N/A

[CharlesChen0823]

currenlty default vim.json config has two keybindings: "ctrl-x":null and "ctrl-w":null, not working as origin expected, i have no idea.

[ConradIrwin]

The null handling is definitely tricky...

Given a context stack like: Workspace > Pane > Editor; if you set "ctrl-t":null at Editor, it should also disable ctrl-t X at Workspace etc.

Is the change you're making here so that if you have "Ctrl-t:null at Workspace, it also disables ctrl-t X in Editor?

I worry that this means you then can't override "ctrl-t" in your keybindings if it is disabled in the editor bindings?

[CharlesChen0823]

If I understand correctly, for a context stack like: Workspace > Pane > Editor;

• origin implementation: if set "ctrl-t" : null at Editor, it will disable ctrl-t at Pane and Workspace. but you cannot rebind ctrl-t at Pane and Workspace
• so if user unbind any keybingdings at Editor, then he cannot using this keybindings for all context.

The changed i did, if user disable ctrl-t at Editor, user can bind ctrl-t for Pane or Workspace, it only disable for Editor context. But here has some problem:

1. if user bind ctrl-t for Pane and Workspace, the ctrl-t action work at Pane, then work at Workspace.
2. the orgin unbint ctrl-t and ctrl-x for vim_mode, cannot work stable.

[ConradIrwin]

The rule should be that:

• matches lower in the tree take precedence over matches higher
• at a given level in the tree the last definition wins (so user config overrides default config; and bindings further down the file take precedence of those earlier).

I think # 18798 is caused by this. The bindings do not work because they conflict with bindings on the Editor (which is lower in the tree than AssistantPanel). Settings "context": "AssistantPanel > Editor" would probably fix them.

I don't think I understand the bug in #18796, so I am not sure if it is the same.

I also think we should be open to changing how this works, because it's pretty confusing right now; but I'm not sure I have a good idea for what that looks like. One idea would be to make the user file and the settings file distinct so that the precedence would become:

• the user key settings file is considered first, then the default settings.
• within each file, matches lower in the tree take precedence over matches higher, and in case of multiple matches at the same level the last write wins.

I think that would also resolve the problems, but not sure?

[CharlesChen0823]

I don't think I understand the bug in https://github.com/zed-industries/zed/issues/18796, so I am not sure if it is the same.

as the comments, when user unbind ctrl-t at Editor context, then user cannot bind ctrl-t for context like Workspace or Pane.

That is what i say, any keybindings unbind in context Editor, will disable for rebinding for other context.

[ConradIrwin]

I think this is actually working as intended. More changes could happen to make the system easier to understand, but I don't think we should rush into small tweaks as it'll make the overall rules harder to grasp.