mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
Keep keymap action descriptions stable while navigating (#39992)
## What changed - Use one binding-aware description for each keymap action instead of swapping in separate text for the selected item. - Keep the action menu content unchanged as the selection moves between items. - Update key and key-chord replacement descriptions to name the affected binding directly. ## Testing - Extend the keymap action menu test to move through every item and verify that the rendered picker remains unchanged. GitOrigin-RevId: bbd2bd08ca444c76c28c5213ae077ad85bc50f27
This commit is contained in:
committed by
copyberry
parent
f6519a355a
commit
45a3edc02a
@@ -115,8 +115,7 @@ fn open_capture_action(
|
||||
|
||||
fn action_menu_item(
|
||||
name: &str,
|
||||
description: &str,
|
||||
selected_description: String,
|
||||
description: String,
|
||||
context: &str,
|
||||
action: &str,
|
||||
intent: KeymapEditIntent,
|
||||
@@ -124,8 +123,7 @@ fn action_menu_item(
|
||||
) -> SelectionItem {
|
||||
SelectionItem {
|
||||
name: name.to_string(),
|
||||
description: Some(description.to_string()),
|
||||
selected_description: Some(selected_description),
|
||||
description: Some(description),
|
||||
actions: vec![open_capture_action(
|
||||
context.to_string(),
|
||||
action.to_string(),
|
||||
@@ -203,8 +201,7 @@ pub(crate) fn build_keymap_action_menu_params(
|
||||
0 => {
|
||||
items.push(action_menu_item(
|
||||
"Set key",
|
||||
"Capture a key for this unbound action.",
|
||||
"Capture one key and bind this action.".to_string(),
|
||||
"Capture a key for this unbound action.".to_string(),
|
||||
&context,
|
||||
&action,
|
||||
KeymapEditIntent::ReplaceAll,
|
||||
@@ -214,8 +211,7 @@ pub(crate) fn build_keymap_action_menu_params(
|
||||
1 => {
|
||||
items.push(action_menu_item(
|
||||
"Replace binding",
|
||||
"Capture a replacement key.",
|
||||
format!("Capture one key and replace `{current_binding}`."),
|
||||
format!("Capture a replacement key for `{current_binding}`."),
|
||||
&context,
|
||||
&action,
|
||||
KeymapEditIntent::ReplaceAll,
|
||||
@@ -223,8 +219,7 @@ pub(crate) fn build_keymap_action_menu_params(
|
||||
));
|
||||
items.push(action_menu_item(
|
||||
"Add alternate binding",
|
||||
"Keep the current binding and add another key.",
|
||||
format!("Capture one key and keep `{current_binding}` as an alternate."),
|
||||
format!("Keep `{current_binding}` and add another key."),
|
||||
&context,
|
||||
&action,
|
||||
KeymapEditIntent::AddAlternate,
|
||||
@@ -237,9 +232,6 @@ pub(crate) fn build_keymap_action_menu_params(
|
||||
items.push(SelectionItem {
|
||||
name: "Replace one binding...".to_string(),
|
||||
description: Some("Choose which existing binding to replace.".to_string()),
|
||||
selected_description: Some(
|
||||
"Pick one current binding, then capture its replacement.".to_string(),
|
||||
),
|
||||
actions: vec![Box::new(move |tx| {
|
||||
tx.send(AppEvent::OpenKeymapReplaceBindingMenu {
|
||||
context: replace_one_context.clone(),
|
||||
@@ -250,8 +242,7 @@ pub(crate) fn build_keymap_action_menu_params(
|
||||
});
|
||||
items.push(action_menu_item(
|
||||
"Replace all bindings",
|
||||
"Replace every current binding with one key.",
|
||||
format!("Capture one key and replace `{current_binding}`."),
|
||||
format!("Replace `{current_binding}` with one key."),
|
||||
&context,
|
||||
&action,
|
||||
KeymapEditIntent::ReplaceAll,
|
||||
@@ -259,8 +250,7 @@ pub(crate) fn build_keymap_action_menu_params(
|
||||
));
|
||||
items.push(action_menu_item(
|
||||
"Add alternate binding",
|
||||
"Keep current bindings and add another key.",
|
||||
format!("Capture one key and keep `{current_binding}`."),
|
||||
format!("Keep `{current_binding}` and add another key."),
|
||||
&context,
|
||||
&action,
|
||||
KeymapEditIntent::AddAlternate,
|
||||
@@ -271,8 +261,7 @@ pub(crate) fn build_keymap_action_menu_params(
|
||||
if active_binding_count == 0 {
|
||||
items.push(action_menu_item(
|
||||
"Set key chord",
|
||||
"Capture two consecutive keys for this action.",
|
||||
"Capture a two-stroke key chord.".to_string(),
|
||||
"Capture two consecutive keys for this action.".to_string(),
|
||||
&context,
|
||||
&action,
|
||||
KeymapEditIntent::ReplaceAll,
|
||||
@@ -281,8 +270,7 @@ pub(crate) fn build_keymap_action_menu_params(
|
||||
} else {
|
||||
items.push(action_menu_item(
|
||||
"Replace with key chord",
|
||||
"Replace current bindings with a two-stroke key chord.",
|
||||
format!("Capture two keys and replace `{current_binding}`."),
|
||||
format!("Replace `{current_binding}` with a two-stroke key chord."),
|
||||
&context,
|
||||
&action,
|
||||
KeymapEditIntent::ReplaceAll,
|
||||
@@ -290,8 +278,7 @@ pub(crate) fn build_keymap_action_menu_params(
|
||||
));
|
||||
items.push(action_menu_item(
|
||||
"Add alternate key chord",
|
||||
"Keep current bindings and add a two-stroke key chord.",
|
||||
format!("Capture two keys and keep `{current_binding}`."),
|
||||
format!("Keep `{current_binding}` and add a two-stroke key chord."),
|
||||
&context,
|
||||
&action,
|
||||
KeymapEditIntent::AddAlternate,
|
||||
@@ -301,8 +288,6 @@ pub(crate) fn build_keymap_action_menu_params(
|
||||
items.push(SelectionItem {
|
||||
name: "Remove custom binding".to_string(),
|
||||
description: custom_binding.then(|| "Restore the default keymap binding.".to_string()),
|
||||
selected_description: custom_binding
|
||||
.then(|| "Delete the root override and use the default keymap again.".to_string()),
|
||||
disabled_reason: remove_disabled_reason,
|
||||
disabled_gutter_marker: Some("–"),
|
||||
actions: vec![Box::new(move |tx| {
|
||||
@@ -360,10 +345,7 @@ pub(crate) fn build_keymap_replace_binding_menu_params(
|
||||
[
|
||||
SelectionItem {
|
||||
name: binding.clone(),
|
||||
description: Some("Replace this binding.".to_string()),
|
||||
selected_description: Some(format!(
|
||||
"Capture a new key to replace `{binding}`."
|
||||
)),
|
||||
description: Some(format!("Replace `{binding}` with another key.")),
|
||||
actions: vec![open_capture_action(
|
||||
context.clone(),
|
||||
action.clone(),
|
||||
@@ -377,8 +359,7 @@ pub(crate) fn build_keymap_replace_binding_menu_params(
|
||||
},
|
||||
SelectionItem {
|
||||
name: format!("{binding} (key chord)"),
|
||||
description: Some("Replace this binding with a key chord.".to_string()),
|
||||
selected_description: Some(format!("Capture two keys to replace `{binding}`.")),
|
||||
description: Some(format!("Replace `{binding}` with a two-stroke key chord.")),
|
||||
actions: vec![open_capture_action(
|
||||
context.clone(),
|
||||
action.clone(),
|
||||
@@ -1278,6 +1259,20 @@ mod tests {
|
||||
.join("\n");
|
||||
|
||||
assert_snapshot!("keymap_action_menu", snapshot);
|
||||
|
||||
for params in [unbound_params, single_params, multi_params, replace_params] {
|
||||
let item_count = params.items.len();
|
||||
let mut view =
|
||||
ListSelectionView::new(params, app_event_sender(), RuntimeKeymap::defaults().list);
|
||||
let initial = render_picker_from_view(&view, /*width*/ 160).replace('›', " ");
|
||||
for _ in 1..item_count {
|
||||
view.handle_key_event(KeyEvent::from(KeyCode::Down));
|
||||
assert_eq!(
|
||||
render_picker_from_view(&view, /*width*/ 160).replace('›', " "),
|
||||
initial,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -9,24 +9,24 @@ Remove custom binding | Restore the default keymap binding. | enabled
|
||||
Back to shortcuts | Return to the shortcut list. | enabled
|
||||
|
||||
single:
|
||||
Replace binding | Capture a replacement key. | enabled
|
||||
Add alternate binding | Keep the current binding and add another key. | enabled
|
||||
Replace with key chord | Replace current bindings with a two-stroke key chord. | enabled
|
||||
Add alternate key chord | Keep current bindings and add a two-stroke key chord. | enabled
|
||||
Replace binding | Capture a replacement key for `ctrl-enter`. | enabled
|
||||
Add alternate binding | Keep `ctrl-enter` and add another key. | enabled
|
||||
Replace with key chord | Replace `ctrl-enter` with a two-stroke key chord. | enabled
|
||||
Add alternate key chord | Keep `ctrl-enter` and add a two-stroke key chord. | enabled
|
||||
Remove custom binding | Restore the default keymap binding. | enabled
|
||||
Back to shortcuts | Return to the shortcut list. | enabled
|
||||
|
||||
multi:
|
||||
Replace one binding... | Choose which existing binding to replace. | enabled
|
||||
Replace all bindings | Replace every current binding with one key. | enabled
|
||||
Add alternate binding | Keep current bindings and add another key. | enabled
|
||||
Replace with key chord | Replace current bindings with a two-stroke key chord. | enabled
|
||||
Add alternate key chord | Keep current bindings and add a two-stroke key chord. | enabled
|
||||
Replace all bindings | Replace `ctrl-enter, alt-shift-enter` with one key. | enabled
|
||||
Add alternate binding | Keep `ctrl-enter, alt-shift-enter` and add another key. | enabled
|
||||
Replace with key chord | Replace `ctrl-enter, alt-shift-enter` with a two-stroke key chord. | enabled
|
||||
Add alternate key chord | Keep `ctrl-enter, alt-shift-enter` and add a two-stroke key chord. | enabled
|
||||
Remove custom binding | Restore the default keymap binding. | enabled
|
||||
Back to shortcuts | Return to the shortcut list. | enabled
|
||||
|
||||
replace picker:
|
||||
ctrl-enter | Replace this binding. | enabled
|
||||
ctrl-enter (key chord) | Replace this binding with a key chord. | enabled
|
||||
alt-shift-enter | Replace this binding. | enabled
|
||||
alt-shift-enter (key chord) | Replace this binding with a key chord. | enabled
|
||||
ctrl-enter | Replace `ctrl-enter` with another key. | enabled
|
||||
ctrl-enter (key chord) | Replace `ctrl-enter` with a two-stroke key chord. | enabled
|
||||
alt-shift-enter | Replace `alt-shift-enter` with another key. | enabled
|
||||
alt-shift-enter (key chord) | Replace `alt-shift-enter` with a two-stroke key chord. | enabled
|
||||
|
||||
@@ -11,16 +11,15 @@ expression: snapshot
|
||||
Open the transcript overlay.
|
||||
|
||||
› 1. Replace binding
|
||||
Capture one key and replace `ctrl-t`.
|
||||
Capture a replacement key for `ctrl-t`.
|
||||
2. Add alternate binding
|
||||
Keep the current binding and add another
|
||||
key.
|
||||
Keep `ctrl-t` and add another key.
|
||||
3. Replace with key chord
|
||||
Replace current bindings with a two-
|
||||
stroke key chord.
|
||||
Replace `ctrl-t` with a two-stroke key
|
||||
chord.
|
||||
4. Add alternate key chord
|
||||
Keep current bindings and add a two-
|
||||
stroke key chord.
|
||||
Keep `ctrl-t` and add a two-stroke key
|
||||
chord.
|
||||
– Remove custom binding (disabled)
|
||||
No custom root override to remove.
|
||||
5. Back to shortcuts
|
||||
@@ -39,13 +38,13 @@ expression: snapshot
|
||||
Open the transcript overlay.
|
||||
|
||||
› 1. Replace binding
|
||||
Capture one key and replace `ctrl-t`.
|
||||
Capture a replacement key for `ctrl-t`.
|
||||
2. Add alternate binding
|
||||
Keep the current binding and add another key.
|
||||
Keep `ctrl-t` and add another key.
|
||||
3. Replace with key chord
|
||||
Replace current bindings with a two-stroke key chord.
|
||||
Replace `ctrl-t` with a two-stroke key chord.
|
||||
4. Add alternate key chord
|
||||
Keep current bindings and add a two-stroke key chord.
|
||||
Keep `ctrl-t` and add a two-stroke key chord.
|
||||
– Remove custom binding (disabled)
|
||||
No custom root override to remove.
|
||||
5. Back to shortcuts
|
||||
@@ -62,10 +61,10 @@ expression: snapshot
|
||||
Config `tui.keymap.global.open_transcript`
|
||||
Open the transcript overlay.
|
||||
|
||||
› 1. Replace binding Capture one key and replace `ctrl-t`.
|
||||
2. Add alternate binding Keep the current binding and add another key.
|
||||
3. Replace with key chord Replace current bindings with a two-stroke key chord.
|
||||
4. Add alternate key chord Keep current bindings and add a two-stroke key chord.
|
||||
› 1. Replace binding Capture a replacement key for `ctrl-t`.
|
||||
2. Add alternate binding Keep `ctrl-t` and add another key.
|
||||
3. Replace with key chord Replace `ctrl-t` with a two-stroke key chord.
|
||||
4. Add alternate key chord Keep `ctrl-t` and add a two-stroke key chord.
|
||||
– Remove custom binding (disabled) No custom root override to remove.
|
||||
5. Back to shortcuts Return to the shortcut list.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user