fix(vim): address mode rename review findings
- commit
- ae52164802218ef7ae0be7ddcc4521e21431c2d3
- author
- brice · · 2026-08-13 11:52 UTC
- committer
- brice · · 2026-08-14 02:31 UTC
- parent
- 23fc389
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
9 files · +71 -16
M docs/config/index.md +1 -1
| @@ -448,7 +448,7 @@ Notes: | |||
| 448 | 448 | - Arrow keys still use the browser engine's native page-navigation path while Vim Mode is active. | |
| 449 | 449 | - Other app-level shortcuts stay suspended until Vim Mode exits, except for the Vim Mode toggle itself. | |
| 450 | 450 | - `workspace.styling.pane_mode_color`, `workspace.styling.transition_duration`, and `workspace.styling.mode_indicator_toaster_enabled` control the pane-local Vim Mode visuals. | |
| 451 | - | - Scroll execution: CEF and WebKit both use the shared `BuildScrollByJS` resolver when the page is ready. Each step starts under the viewport center, walks up through ancestors that can move in the requested direction, and hands scrolling to the document when a nested container reaches its boundary. The application repeater owns held-key cadence; each engine executes one immediate scroll step per tick. Cross-origin frame contents remain best-effort. CEF may use native precision-wheel input only as a pre-frame fallback before the browser/main frame is ready. | |
| 451 | + | - Scroll execution: CEF and WebKit both use the shared `BuildPageScrollByJS` resolver when the page is ready. Each step starts under the viewport center, walks up through ancestors that can move in the requested direction, and hands scrolling to the document when a nested container reaches its boundary. The application repeater owns held-key cadence; each engine executes one immediate scroll step per tick. Cross-origin frame contents remain best-effort. CEF may use native precision-wheel input only as a pre-frame fallback before the browser/main frame is ready. | |
| 452 | 452 | ||
| 453 | 453 | ### Resize Mode | |
| 454 | 454 | ||
| @@ -448,7 +448,7 @@ Notes: | |||||
| 448 | - Arrow keys still use the browser engine's native page-navigation path while Vim Mode is active. | 448 | - Arrow keys still use the browser engine's native page-navigation path while Vim Mode is active. | ||
| 449 | - Other app-level shortcuts stay suspended until Vim Mode exits, except for the Vim Mode toggle itself. | 449 | - Other app-level shortcuts stay suspended until Vim Mode exits, except for the Vim Mode toggle itself. | ||
| 450 | - `workspace.styling.pane_mode_color`, `workspace.styling.transition_duration`, and `workspace.styling.mode_indicator_toaster_enabled` control the pane-local Vim Mode visuals. | 450 | - `workspace.styling.pane_mode_color`, `workspace.styling.transition_duration`, and `workspace.styling.mode_indicator_toaster_enabled` control the pane-local Vim Mode visuals. | ||
| 451 | - | - Scroll execution: CEF and WebKit both use the shared `BuildScrollByJS` resolver when the page is ready. Each step starts under the viewport center, walks up through ancestors that can move in the requested direction, and hands scrolling to the document when a nested container reaches its boundary. The application repeater owns held-key cadence; each engine executes one immediate scroll step per tick. Cross-origin frame contents remain best-effort. CEF may use native precision-wheel input only as a pre-frame fallback before the browser/main frame is ready. | 451 | + | - Scroll execution: CEF and WebKit both use the shared `BuildPageScrollByJS` resolver when the page is ready. Each step starts under the viewport center, walks up through ancestors that can move in the requested direction, and hands scrolling to the document when a nested container reaches its boundary. The application repeater owns held-key cadence; each engine executes one immediate scroll step per tick. Cross-origin frame contents remain best-effort. CEF may use native precision-wheel input only as a pre-frame fallback before the browser/main frame is ready. |
| 452 | 452 | ||||
| 453 | ### Resize Mode | 453 | ### Resize Mode | ||
| 454 | 454 | ||||
M docs/getting-started.md +1 -1
| @@ -41,7 +41,7 @@ Vim Mode stays local to the active pane and automatically leaves the mode when f | |||
| 41 | 41 | 7. If a page input or editor is already focused, `Ctrl+Y` is passed through instead of stealing the keypress | |
| 42 | 42 | ||
| 43 | 43 | > **Engine behavior**: CEF and WebKit execute Vim Mode scroll steps with the | |
| 44 | - | > shared `BuildScrollByJS` resolver (viewport-center start, nested-scroller | |
| 44 | + | > shared `BuildPageScrollByJS` resolver (viewport-center start, nested-scroller | |
| 45 | 45 | > handoff, document fallback). The application repeater owns held-key cadence; | |
| 46 | 46 | > each engine runs one immediate step per tick. Cross-origin frames are | |
| 47 | 47 | > best-effort. CEF may use native precision-wheel input only before the browser | |
| @@ -41,7 +41,7 @@ Vim Mode stays local to the active pane and automatically leaves the mode when f | |||||
| 41 | 7. If a page input or editor is already focused, `Ctrl+Y` is passed through instead of stealing the keypress | 41 | 7. If a page input or editor is already focused, `Ctrl+Y` is passed through instead of stealing the keypress | ||
| 42 | 42 | ||||
| 43 | > **Engine behavior**: CEF and WebKit execute Vim Mode scroll steps with the | 43 | > **Engine behavior**: CEF and WebKit execute Vim Mode scroll steps with the | ||
| 44 | - | > shared `BuildScrollByJS` resolver (viewport-center start, nested-scroller | 44 | + | > shared `BuildPageScrollByJS` resolver (viewport-center start, nested-scroller |
| 45 | > handoff, document fallback). The application repeater owns held-key cadence; | 45 | > handoff, document fallback). The application repeater owns held-key cadence; | ||
| 46 | > each engine runs one immediate step per tick. Cross-origin frames are | 46 | > each engine runs one immediate step per tick. Cross-origin frames are | ||
| 47 | > best-effort. CEF may use native precision-wheel input only before the browser | 47 | > best-effort. CEF may use native precision-wheel input only before the browser | ||
M docs/reference/configuration.md +1 -1
| @@ -102,7 +102,7 @@ | |||
| 102 | 102 | | `workspace.tab_mode.actions.<action>` | []string | see defaults | tab mode key mappings | | |
| 103 | 103 | | `workspace.vim_mode.activation_shortcut` | string | `ctrl+y` | | | |
| 104 | 104 | | `workspace.vim_mode.timeout_ms` | int | `0` | >= 0 | | |
| 105 | - | | `workspace.vim_mode.actions.<action>` | []string | see defaults | vim mode key mappings | | |
| 105 | + | | `workspace.vim_mode.actions.<action>` | object | see defaults | vim mode action object with `keys` ([]string) and `desc` (string) | | |
| 106 | 106 | | `workspace.resize_mode.activation_shortcut` | string | `ctrl+n` | | | |
| 107 | 107 | | `workspace.resize_mode.timeout_ms` | int | `3000` | | | |
| 108 | 108 | | `workspace.resize_mode.actions.<action>` | []string | see defaults | resize mode key mappings | | |
| @@ -102,7 +102,7 @@ | |||||
| 102 | | `workspace.tab_mode.actions.<action>` | []string | see defaults | tab mode key mappings | | 102 | | `workspace.tab_mode.actions.<action>` | []string | see defaults | tab mode key mappings | | ||
| 103 | | `workspace.vim_mode.activation_shortcut` | string | `ctrl+y` | | | 103 | | `workspace.vim_mode.activation_shortcut` | string | `ctrl+y` | | | ||
| 104 | | `workspace.vim_mode.timeout_ms` | int | `0` | >= 0 | | 104 | | `workspace.vim_mode.timeout_ms` | int | `0` | >= 0 | | ||
| 105 | - | | `workspace.vim_mode.actions.<action>` | []string | see defaults | vim mode key mappings | | 105 | + | | `workspace.vim_mode.actions.<action>` | object | see defaults | vim mode action object with `keys` ([]string) and `desc` (string) | |
| 106 | | `workspace.resize_mode.activation_shortcut` | string | `ctrl+n` | | | 106 | | `workspace.resize_mode.activation_shortcut` | string | `ctrl+n` | | | ||
| 107 | | `workspace.resize_mode.timeout_ms` | int | `3000` | | | 107 | | `workspace.resize_mode.timeout_ms` | int | `3000` | | | ||
| 108 | | `workspace.resize_mode.actions.<action>` | []string | see defaults | resize mode key mappings | | 108 | | `workspace.resize_mode.actions.<action>` | []string | see defaults | resize mode key mappings | | ||
M docs/reference/keybindings.md +1 -1
| @@ -57,7 +57,7 @@ Keybinding tables use uppercase letters as visual labels for unshifted letter ke | |||
| 57 | 57 | ||
| 58 | 58 | Vim Mode is an explicit Vim-style scrolling mode for the active pane only. It applies a pane-local accent from `workspace.styling.pane_mode_color`, and when `workspace.styling.mode_indicator_toaster_enabled` is true, shows a persistent bottom-left `VIM MODE` toaster while the mode is active; the toaster hides on exit, when the mode is toggled off, or when the toaster is disabled in config. The mode exits automatically when focus moves into the omnibox, find bar, overlays, or an editable element inside the page. The default `timeout_ms` is `0`, so Vim Mode does not auto-time out unless you configure one. Arrow keys continue to flow through the browser engine's native page-navigation path while Vim Mode is active, while other app-level shortcuts stay suspended until you leave the mode. | |
| 59 | 59 | ||
| 60 | - | CEF and WebKit execute Vim Mode scroll commands (`h/j/k/l`, `Shift+J/K`) with the shared `BuildScrollByJS` resolver. Each step starts under the viewport center, walks up through ancestors that can move in the requested direction, and hands scrolling to the document when a nested container reaches its boundary. Cross-origin frame contents remain best-effort. | |
| 60 | + | CEF and WebKit execute Vim Mode scroll commands (`h/j/k/l`, `Shift+J/K`) with the shared `BuildPageScrollByJS` resolver. Each step starts under the viewport center, walks up through ancestors that can move in the requested direction, and hands scrolling to the document when a nested container reaches its boundary. Cross-origin frame contents remain best-effort. | |
| 61 | 61 | ||
| 62 | 62 | | Action | Keys | | |
| 63 | 63 | |--------|------| | |
| @@ -57,7 +57,7 @@ Keybinding tables use uppercase letters as visual labels for unshifted letter ke | |||||
| 57 | 57 | ||||
| 58 | Vim Mode is an explicit Vim-style scrolling mode for the active pane only. It applies a pane-local accent from `workspace.styling.pane_mode_color`, and when `workspace.styling.mode_indicator_toaster_enabled` is true, shows a persistent bottom-left `VIM MODE` toaster while the mode is active; the toaster hides on exit, when the mode is toggled off, or when the toaster is disabled in config. The mode exits automatically when focus moves into the omnibox, find bar, overlays, or an editable element inside the page. The default `timeout_ms` is `0`, so Vim Mode does not auto-time out unless you configure one. Arrow keys continue to flow through the browser engine's native page-navigation path while Vim Mode is active, while other app-level shortcuts stay suspended until you leave the mode. | 58 | Vim Mode is an explicit Vim-style scrolling mode for the active pane only. It applies a pane-local accent from `workspace.styling.pane_mode_color`, and when `workspace.styling.mode_indicator_toaster_enabled` is true, shows a persistent bottom-left `VIM MODE` toaster while the mode is active; the toaster hides on exit, when the mode is toggled off, or when the toaster is disabled in config. The mode exits automatically when focus moves into the omnibox, find bar, overlays, or an editable element inside the page. The default `timeout_ms` is `0`, so Vim Mode does not auto-time out unless you configure one. Arrow keys continue to flow through the browser engine's native page-navigation path while Vim Mode is active, while other app-level shortcuts stay suspended until you leave the mode. | ||
| 59 | 59 | ||||
| 60 | - | CEF and WebKit execute Vim Mode scroll commands (`h/j/k/l`, `Shift+J/K`) with the shared `BuildScrollByJS` resolver. Each step starts under the viewport center, walks up through ancestors that can move in the requested direction, and hands scrolling to the document when a nested container reaches its boundary. Cross-origin frame contents remain best-effort. | 60 | + | CEF and WebKit execute Vim Mode scroll commands (`h/j/k/l`, `Shift+J/K`) with the shared `BuildPageScrollByJS` resolver. Each step starts under the viewport center, walks up through ancestors that can move in the requested direction, and hands scrolling to the document when a nested container reaches its boundary. Cross-origin frame contents remain best-effort. |
| 61 | 61 | ||||
| 62 | | Action | Keys | | 62 | | Action | Keys | | ||
| 63 | |--------|------| | 63 | |--------|------| | ||
M internal/infrastructure/config/migrate.go +25 -7
| @@ -791,15 +791,12 @@ func (m *Migrator) detectLegacyBrowsingContextRenames(rawUserKeys map[string]any | |||
| 791 | 791 | } | |
| 792 | 792 | ||
| 793 | 793 | func (m *Migrator) detectLegacyVimModeRenames(rawUserKeys map[string]any) []port.KeyChange { | |
| 794 | - | const oldPrefix = "workspace.page_mode" | |
| 795 | - | const newPrefix = "workspace.vim_mode" | |
| 796 | - | ||
| 797 | 794 | changes := make([]port.KeyChange, 0, 3) | |
| 798 | 795 | for oldKey, oldValue := range rawUserKeys { | |
| 799 | - | if !strings.HasPrefix(oldKey, oldPrefix+".") { | |
| 796 | + | newKey, ok := legacyVimModeKey(oldKey) | |
| 797 | + | if !ok { | |
| 800 | 798 | continue | |
| 801 | 799 | } | |
| 802 | - | newKey := newPrefix + strings.TrimPrefix(oldKey, oldPrefix) | |
| 803 | 800 | if _, hasNew := rawUserKeys[newKey]; hasNew { | |
| 804 | 801 | continue | |
| 805 | 802 | } | |
| @@ -843,7 +840,11 @@ func (*Migrator) migrateLegacyVimMode(rawConfig map[string]any) { | |||
| 843 | 840 | vimMode["actions"] = vimActions | |
| 844 | 841 | } | |
| 845 | 842 | for oldAction, value := range legacyActions { | |
| 846 | - | newAction := legacyVimActionName(oldAction) | |
| 843 | + | newKey, ok := legacyVimModeKey("workspace.page_mode.actions." + oldAction) | |
| 844 | + | if !ok { | |
| 845 | + | continue | |
| 846 | + | } | |
| 847 | + | newAction := strings.TrimPrefix(newKey, "workspace.vim_mode.actions.") | |
| 847 | 848 | if _, exists := vimActions[newAction]; !exists { | |
| 848 | 849 | vimActions[newAction] = value | |
| 849 | 850 | } | |
| @@ -851,10 +852,27 @@ func (*Migrator) migrateLegacyVimMode(rawConfig map[string]any) { | |||
| 851 | 852 | delete(workspace, "page_mode") | |
| 852 | 853 | } | |
| 853 | 854 | ||
| 855 | + | // legacyVimModeKey converts one legacy Page Mode config key to its canonical | |
| 856 | + | // Vim Mode key, including action-name normalization. | |
| 857 | + | func legacyVimModeKey(oldKey string) (string, bool) { | |
| 858 | + | const oldPrefix = "workspace.page_mode" | |
| 859 | + | const newPrefix = "workspace.vim_mode" | |
| 860 | + | ||
| 861 | + | if !strings.HasPrefix(oldKey, oldPrefix+".") { | |
| 862 | + | return "", false | |
| 863 | + | } | |
| 864 | + | newKey := newPrefix + strings.TrimPrefix(oldKey, oldPrefix) | |
| 865 | + | const actionsPrefix = newPrefix + ".actions." | |
| 866 | + | if strings.HasPrefix(newKey, actionsPrefix) { | |
| 867 | + | return actionsPrefix + legacyVimActionName(strings.TrimPrefix(newKey, actionsPrefix)), true | |
| 868 | + | } | |
| 869 | + | return newKey, true | |
| 870 | + | } | |
| 871 | + | ||
| 854 | 872 | func legacyVimActionName(action string) string { | |
| 855 | 873 | for _, prefix := range []string{"page-scroll-", "page_scroll_"} { | |
| 856 | 874 | if strings.HasPrefix(action, prefix) { | |
| 857 | - | return "vim-scroll-" + strings.TrimPrefix(action, prefix) | |
| 875 | + | return "vim-scroll-" + strings.ReplaceAll(strings.TrimPrefix(action, prefix), "_", "-") | |
| 858 | 876 | } | |
| 859 | 877 | } | |
| 860 | 878 | return action | |
| @@ -791,15 +791,12 @@ func (m *Migrator) detectLegacyBrowsingContextRenames(rawUserKeys map[string]any | |||||
| 791 | } | 791 | } | ||
| 792 | 792 | ||||
| 793 | func (m *Migrator) detectLegacyVimModeRenames(rawUserKeys map[string]any) []port.KeyChange { | 793 | func (m *Migrator) detectLegacyVimModeRenames(rawUserKeys map[string]any) []port.KeyChange { | ||
| 794 | - | const oldPrefix = "workspace.page_mode" | |||
| 795 | - | const newPrefix = "workspace.vim_mode" | |||
| 796 | - | ||||
| 797 | changes := make([]port.KeyChange, 0, 3) | 794 | changes := make([]port.KeyChange, 0, 3) | ||
| 798 | for oldKey, oldValue := range rawUserKeys { | 795 | for oldKey, oldValue := range rawUserKeys { | ||
| 799 | - | if !strings.HasPrefix(oldKey, oldPrefix+".") { | 796 | + | newKey, ok := legacyVimModeKey(oldKey) |
| 797 | + | if !ok { | |||
| 800 | continue | 798 | continue | ||
| 801 | } | 799 | } | ||
| 802 | - | newKey := newPrefix + strings.TrimPrefix(oldKey, oldPrefix) | |||
| 803 | if _, hasNew := rawUserKeys[newKey]; hasNew { | 800 | if _, hasNew := rawUserKeys[newKey]; hasNew { | ||
| 804 | continue | 801 | continue | ||
| 805 | } | 802 | } | ||
| @@ -843,7 +840,11 @@ func (*Migrator) migrateLegacyVimMode(rawConfig map[string]any) { | |||||
| 843 | vimMode["actions"] = vimActions | 840 | vimMode["actions"] = vimActions | ||
| 844 | } | 841 | } | ||
| 845 | for oldAction, value := range legacyActions { | 842 | for oldAction, value := range legacyActions { | ||
| 846 | - | newAction := legacyVimActionName(oldAction) | 843 | + | newKey, ok := legacyVimModeKey("workspace.page_mode.actions." + oldAction) |
| 844 | + | if !ok { | |||
| 845 | + | continue | |||
| 846 | + | } | |||
| 847 | + | newAction := strings.TrimPrefix(newKey, "workspace.vim_mode.actions.") | |||
| 847 | if _, exists := vimActions[newAction]; !exists { | 848 | if _, exists := vimActions[newAction]; !exists { | ||
| 848 | vimActions[newAction] = value | 849 | vimActions[newAction] = value | ||
| 849 | } | 850 | } | ||
| @@ -851,10 +852,27 @@ func (*Migrator) migrateLegacyVimMode(rawConfig map[string]any) { | |||||
| 851 | delete(workspace, "page_mode") | 852 | delete(workspace, "page_mode") | ||
| 852 | } | 853 | } | ||
| 853 | 854 | ||||
| 855 | + | // legacyVimModeKey converts one legacy Page Mode config key to its canonical | |||
| 856 | + | // Vim Mode key, including action-name normalization. | |||
| 857 | + | func legacyVimModeKey(oldKey string) (string, bool) { | |||
| 858 | + | const oldPrefix = "workspace.page_mode" | |||
| 859 | + | const newPrefix = "workspace.vim_mode" | |||
| 860 | + | ||||
| 861 | + | if !strings.HasPrefix(oldKey, oldPrefix+".") { | |||
| 862 | + | return "", false | |||
| 863 | + | } | |||
| 864 | + | newKey := newPrefix + strings.TrimPrefix(oldKey, oldPrefix) | |||
| 865 | + | const actionsPrefix = newPrefix + ".actions." | |||
| 866 | + | if strings.HasPrefix(newKey, actionsPrefix) { | |||
| 867 | + | return actionsPrefix + legacyVimActionName(strings.TrimPrefix(newKey, actionsPrefix)), true | |||
| 868 | + | } | |||
| 869 | + | return newKey, true | |||
| 870 | + | } | |||
| 871 | + | ||||
| 854 | func legacyVimActionName(action string) string { | 872 | func legacyVimActionName(action string) string { | ||
| 855 | for _, prefix := range []string{"page-scroll-", "page_scroll_"} { | 873 | for _, prefix := range []string{"page-scroll-", "page_scroll_"} { | ||
| 856 | if strings.HasPrefix(action, prefix) { | 874 | if strings.HasPrefix(action, prefix) { | ||
| 857 | - | return "vim-scroll-" + strings.TrimPrefix(action, prefix) | 875 | + | return "vim-scroll-" + strings.ReplaceAll(strings.TrimPrefix(action, prefix), "_", "-") |
| 858 | } | 876 | } | ||
| 859 | } | 877 | } | ||
| 860 | return action | 878 | return action | ||
M internal/infrastructure/config/migrate_test.go +37 -0
| @@ -1198,6 +1198,10 @@ desc = "Scroll down" | |||
| 1198 | 1198 | [workspace.page_mode.actions.page_scroll_up] | |
| 1199 | 1199 | keys = ["u"] | |
| 1200 | 1200 | desc = "Scroll up" | |
| 1201 | + | ||
| 1202 | + | [workspace.page_mode.actions.page_scroll_down_fast] | |
| 1203 | + | keys = ["d"] | |
| 1204 | + | desc = "Scroll down fast" | |
| 1201 | 1205 | `), 0o644)) | |
| 1202 | 1206 | ||
| 1203 | 1207 | migrator := NewMigrator() | |
| @@ -1213,11 +1217,44 @@ desc = "Scroll up" | |||
| 1213 | 1217 | assert.Contains(t, string(migrated), "activation_shortcut = 'ctrl+alt+y'") | |
| 1214 | 1218 | assert.Contains(t, string(migrated), "[workspace.vim_mode.actions.vim-scroll-down]") | |
| 1215 | 1219 | assert.Contains(t, string(migrated), "[workspace.vim_mode.actions.vim-scroll-up]") | |
| 1220 | + | assert.Contains(t, string(migrated), "[workspace.vim_mode.actions.vim-scroll-down-fast]") | |
| 1216 | 1221 | assert.Contains(t, string(migrated), "keys = ['u']") | |
| 1217 | 1222 | assert.NotContains(t, string(migrated), "page_mode") | |
| 1218 | 1223 | assert.NotContains(t, string(migrated), "page-scroll-down") | |
| 1219 | 1224 | } | |
| 1220 | 1225 | ||
| 1226 | + | func TestMigrator_DetectChanges_CanonicalizesLegacyVimActionNames(t *testing.T) { | |
| 1227 | + | home := t.TempDir() | |
| 1228 | + | t.Setenv("HOME", home) | |
| 1229 | + | t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) | |
| 1230 | + | t.Setenv("XDG_DATA_HOME", filepath.Join(home, ".local", "share")) | |
| 1231 | + | t.Setenv("XDG_STATE_HOME", filepath.Join(home, ".local", "state")) | |
| 1232 | + | t.Setenv("XDG_CACHE_HOME", filepath.Join(home, ".cache")) | |
| 1233 | + | ||
| 1234 | + | configFile, err := GetConfigFile() | |
| 1235 | + | require.NoError(t, err) | |
| 1236 | + | require.NoError(t, os.MkdirAll(filepath.Dir(configFile), 0o755)) | |
| 1237 | + | require.NoError(t, os.WriteFile(configFile, []byte(` | |
| 1238 | + | [workspace.page_mode.actions.page_scroll_down_fast] | |
| 1239 | + | keys = ["d"] | |
| 1240 | + | desc = "Scroll down fast" | |
| 1241 | + | `), 0o644)) | |
| 1242 | + | ||
| 1243 | + | changes, err := NewMigrator().DetectChanges() | |
| 1244 | + | require.NoError(t, err) | |
| 1245 | + | ||
| 1246 | + | foundRename := false | |
| 1247 | + | for _, change := range changes { | |
| 1248 | + | if change.Type == port.KeyChangeRenamed && | |
| 1249 | + | change.OldKey == "workspace.page_mode.actions" && | |
| 1250 | + | change.NewKey == "workspace.vim_mode.actions" { | |
| 1251 | + | foundRename = true | |
| 1252 | + | } | |
| 1253 | + | } | |
| 1254 | + | assert.True(t, foundRename, "DetectChanges must report the canonical Vim Mode actions key") | |
| 1255 | + | assert.Equal(t, "vim-scroll-down-fast", legacyVimActionName("page_scroll_down_fast")) | |
| 1256 | + | } | |
| 1257 | + | ||
| 1221 | 1258 | func TestMigrator_Migrate_MergesLegacyPageModeActionsIntoExistingVimMode(t *testing.T) { | |
| 1222 | 1259 | home := t.TempDir() | |
| 1223 | 1260 | t.Setenv("HOME", home) | |
| @@ -1198,6 +1198,10 @@ desc = "Scroll down" | |||||
| 1198 | [workspace.page_mode.actions.page_scroll_up] | 1198 | [workspace.page_mode.actions.page_scroll_up] | ||
| 1199 | keys = ["u"] | 1199 | keys = ["u"] | ||
| 1200 | desc = "Scroll up" | 1200 | desc = "Scroll up" | ||
| 1201 | + | ||||
| 1202 | + | [workspace.page_mode.actions.page_scroll_down_fast] | |||
| 1203 | + | keys = ["d"] | |||
| 1204 | + | desc = "Scroll down fast" | |||
| 1201 | `), 0o644)) | 1205 | `), 0o644)) | ||
| 1202 | 1206 | ||||
| 1203 | migrator := NewMigrator() | 1207 | migrator := NewMigrator() | ||
| @@ -1213,11 +1217,44 @@ desc = "Scroll up" | |||||
| 1213 | assert.Contains(t, string(migrated), "activation_shortcut = 'ctrl+alt+y'") | 1217 | assert.Contains(t, string(migrated), "activation_shortcut = 'ctrl+alt+y'") | ||
| 1214 | assert.Contains(t, string(migrated), "[workspace.vim_mode.actions.vim-scroll-down]") | 1218 | assert.Contains(t, string(migrated), "[workspace.vim_mode.actions.vim-scroll-down]") | ||
| 1215 | assert.Contains(t, string(migrated), "[workspace.vim_mode.actions.vim-scroll-up]") | 1219 | assert.Contains(t, string(migrated), "[workspace.vim_mode.actions.vim-scroll-up]") | ||
| 1220 | + | assert.Contains(t, string(migrated), "[workspace.vim_mode.actions.vim-scroll-down-fast]") | |||
| 1216 | assert.Contains(t, string(migrated), "keys = ['u']") | 1221 | assert.Contains(t, string(migrated), "keys = ['u']") | ||
| 1217 | assert.NotContains(t, string(migrated), "page_mode") | 1222 | assert.NotContains(t, string(migrated), "page_mode") | ||
| 1218 | assert.NotContains(t, string(migrated), "page-scroll-down") | 1223 | assert.NotContains(t, string(migrated), "page-scroll-down") | ||
| 1219 | } | 1224 | } | ||
| 1220 | 1225 | ||||
| 1226 | + | func TestMigrator_DetectChanges_CanonicalizesLegacyVimActionNames(t *testing.T) { | |||
| 1227 | + | home := t.TempDir() | |||
| 1228 | + | t.Setenv("HOME", home) | |||
| 1229 | + | t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) | |||
| 1230 | + | t.Setenv("XDG_DATA_HOME", filepath.Join(home, ".local", "share")) | |||
| 1231 | + | t.Setenv("XDG_STATE_HOME", filepath.Join(home, ".local", "state")) | |||
| 1232 | + | t.Setenv("XDG_CACHE_HOME", filepath.Join(home, ".cache")) | |||
| 1233 | + | ||||
| 1234 | + | configFile, err := GetConfigFile() | |||
| 1235 | + | require.NoError(t, err) | |||
| 1236 | + | require.NoError(t, os.MkdirAll(filepath.Dir(configFile), 0o755)) | |||
| 1237 | + | require.NoError(t, os.WriteFile(configFile, []byte(` | |||
| 1238 | + | [workspace.page_mode.actions.page_scroll_down_fast] | |||
| 1239 | + | keys = ["d"] | |||
| 1240 | + | desc = "Scroll down fast" | |||
| 1241 | + | `), 0o644)) | |||
| 1242 | + | ||||
| 1243 | + | changes, err := NewMigrator().DetectChanges() | |||
| 1244 | + | require.NoError(t, err) | |||
| 1245 | + | ||||
| 1246 | + | foundRename := false | |||
| 1247 | + | for _, change := range changes { | |||
| 1248 | + | if change.Type == port.KeyChangeRenamed && | |||
| 1249 | + | change.OldKey == "workspace.page_mode.actions" && | |||
| 1250 | + | change.NewKey == "workspace.vim_mode.actions" { | |||
| 1251 | + | foundRename = true | |||
| 1252 | + | } | |||
| 1253 | + | } | |||
| 1254 | + | assert.True(t, foundRename, "DetectChanges must report the canonical Vim Mode actions key") | |||
| 1255 | + | assert.Equal(t, "vim-scroll-down-fast", legacyVimActionName("page_scroll_down_fast")) | |||
| 1256 | + | } | |||
| 1257 | + | ||||
| 1221 | func TestMigrator_Migrate_MergesLegacyPageModeActionsIntoExistingVimMode(t *testing.T) { | 1258 | func TestMigrator_Migrate_MergesLegacyPageModeActionsIntoExistingVimMode(t *testing.T) { | ||
| 1222 | home := t.TempDir() | 1259 | home := t.TempDir() | ||
| 1223 | t.Setenv("HOME", home) | 1260 | t.Setenv("HOME", home) | ||
M internal/infrastructure/config/schema_provider.go +2 -2
| @@ -490,9 +490,9 @@ func (*SchemaProvider) getWorkspaceKeys(defaults *Config) []entity.ConfigKeyInfo | |||
| 490 | 490 | }, | |
| 491 | 491 | { | |
| 492 | 492 | Key: "workspace.vim_mode.actions.<action>", | |
| 493 | - | Type: "[]string", | |
| 493 | + | Type: "object", | |
| 494 | 494 | Default: "(see defaults)", | |
| 495 | - | Description: "Key bindings for vim mode actions", | |
| 495 | + | Description: "Vim mode action object with keys ([]string) and desc (string)", | |
| 496 | 496 | Section: SectionWorkspace, | |
| 497 | 497 | }, | |
| 498 | 498 | // Resize mode | |
| @@ -490,9 +490,9 @@ func (*SchemaProvider) getWorkspaceKeys(defaults *Config) []entity.ConfigKeyInfo | |||||
| 490 | }, | 490 | }, | ||
| 491 | { | 491 | { | ||
| 492 | Key: "workspace.vim_mode.actions.<action>", | 492 | Key: "workspace.vim_mode.actions.<action>", | ||
| 493 | - | Type: "[]string", | 493 | + | Type: "object", |
| 494 | Default: "(see defaults)", | 494 | Default: "(see defaults)", | ||
| 495 | - | Description: "Key bindings for vim mode actions", | 495 | + | Description: "Vim mode action object with keys ([]string) and desc (string)", |
| 496 | Section: SectionWorkspace, | 496 | Section: SectionWorkspace, | ||
| 497 | }, | 497 | }, | ||
| 498 | // Resize mode | 498 | // Resize mode | ||
M internal/infrastructure/config/vim_mode_config_test.go +1 -1
| @@ -212,7 +212,7 @@ func TestSchemaProvider_VimModeKeys(t *testing.T) { | |||
| 212 | 212 | assert.Equal(t, SectionWorkspace, key.Section) | |
| 213 | 213 | case "workspace.vim_mode.actions.<action>": | |
| 214 | 214 | foundActions = true | |
| 215 | - | assert.Equal(t, "[]string", key.Type) | |
| 215 | + | assert.Equal(t, "object", key.Type) | |
| 216 | 216 | assert.Equal(t, SectionWorkspace, key.Section) | |
| 217 | 217 | } | |
| 218 | 218 | } | |
| @@ -212,7 +212,7 @@ func TestSchemaProvider_VimModeKeys(t *testing.T) { | |||||
| 212 | assert.Equal(t, SectionWorkspace, key.Section) | 212 | assert.Equal(t, SectionWorkspace, key.Section) | ||
| 213 | case "workspace.vim_mode.actions.<action>": | 213 | case "workspace.vim_mode.actions.<action>": | ||
| 214 | foundActions = true | 214 | foundActions = true | ||
| 215 | - | assert.Equal(t, "[]string", key.Type) | 215 | + | assert.Equal(t, "object", key.Type) |
| 216 | assert.Equal(t, SectionWorkspace, key.Section) | 216 | assert.Equal(t, SectionWorkspace, key.Section) | ||
| 217 | } | 217 | } | ||
| 218 | } | 218 | } | ||
M internal/ui/input/shortcuts.go +2 -2
| @@ -266,7 +266,7 @@ func NewShortcutSet(ctx context.Context, workspace *entity.WorkspaceConfig, sess | |||
| 266 | 266 | Int("pane", len(set.PaneMode)). | |
| 267 | 267 | Int("resize", len(set.ResizeMode)). | |
| 268 | 268 | Int("session", len(set.SessionMode)). | |
| 269 | - | Int("page", len(set.VimMode)). | |
| 269 | + | Int("vim", len(set.VimMode)). | |
| 270 | 270 | Msg("shortcuts registered") | |
| 271 | 271 | ||
| 272 | 272 | return set | |
| @@ -305,7 +305,7 @@ func (s *ShortcutSet) buildResizeModeShortcuts(ctx context.Context, cfg *entity. | |||
| 305 | 305 | // buildVimModeShortcuts populates vim mode shortcuts from config. | |
| 306 | 306 | func (s *ShortcutSet) buildVimModeShortcuts(ctx context.Context, cfg *entity.WorkspaceConfig) { | |
| 307 | 307 | log := logging.FromContext(ctx) | |
| 308 | - | s.buildModeShortcuts(ctx, cfg.VimMode.GetKeyBindings(), s.VimMode, "page") | |
| 308 | + | s.buildModeShortcuts(ctx, cfg.VimMode.GetKeyBindings(), s.VimMode, "vim") | |
| 309 | 309 | if binding, ok := ParseKeyString(cfg.VimMode.ActivationShortcut); ok { | |
| 310 | 310 | s.VimMode[binding] = ActionEnterVimMode | |
| 311 | 311 | log.Trace(). | |
| @@ -266,7 +266,7 @@ func NewShortcutSet(ctx context.Context, workspace *entity.WorkspaceConfig, sess | |||||
| 266 | Int("pane", len(set.PaneMode)). | 266 | Int("pane", len(set.PaneMode)). | ||
| 267 | Int("resize", len(set.ResizeMode)). | 267 | Int("resize", len(set.ResizeMode)). | ||
| 268 | Int("session", len(set.SessionMode)). | 268 | Int("session", len(set.SessionMode)). | ||
| 269 | - | Int("page", len(set.VimMode)). | 269 | + | Int("vim", len(set.VimMode)). |
| 270 | Msg("shortcuts registered") | 270 | Msg("shortcuts registered") | ||
| 271 | 271 | ||||
| 272 | return set | 272 | return set | ||
| @@ -305,7 +305,7 @@ func (s *ShortcutSet) buildResizeModeShortcuts(ctx context.Context, cfg *entity. | |||||
| 305 | // buildVimModeShortcuts populates vim mode shortcuts from config. | 305 | // buildVimModeShortcuts populates vim mode shortcuts from config. | ||
| 306 | func (s *ShortcutSet) buildVimModeShortcuts(ctx context.Context, cfg *entity.WorkspaceConfig) { | 306 | func (s *ShortcutSet) buildVimModeShortcuts(ctx context.Context, cfg *entity.WorkspaceConfig) { | ||
| 307 | log := logging.FromContext(ctx) | 307 | log := logging.FromContext(ctx) | ||
| 308 | - | s.buildModeShortcuts(ctx, cfg.VimMode.GetKeyBindings(), s.VimMode, "page") | 308 | + | s.buildModeShortcuts(ctx, cfg.VimMode.GetKeyBindings(), s.VimMode, "vim") |
| 309 | if binding, ok := ParseKeyString(cfg.VimMode.ActivationShortcut); ok { | 309 | if binding, ok := ParseKeyString(cfg.VimMode.ActivationShortcut); ok { | ||
| 310 | s.VimMode[binding] = ActionEnterVimMode | 310 | s.VimMode[binding] = ActionEnterVimMode | ||
| 311 | log.Trace(). | 311 | log.Trace(). | ||