refactor(config): share modal action validation
- commit
- e70f71d9ab1b62724b97339372378d57f6e39cc7
- author
- brice · · 2026-07-29 05:04 UTC
- parent
- c7fcb6d
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +86 -59
M internal/infrastructure/config/page_mode_config_test.go +70 -0
| @@ -122,6 +122,76 @@ func TestValidatePageMode_EmptyActionKeysFail(t *testing.T) { | |||
| 122 | 122 | assert.Contains(t, err.Error(), "workspace.page_mode.actions.page-scroll-left must have at least one key binding") | |
| 123 | 123 | } | |
| 124 | 124 | ||
| 125 | + | func TestValidateModalModeActions_RegressionTable(t *testing.T) { | |
| 126 | + | tests := []struct { | |
| 127 | + | name string | |
| 128 | + | modePath string | |
| 129 | + | timeout int | |
| 130 | + | actions map[string]ActionBinding | |
| 131 | + | want string | |
| 132 | + | }{ | |
| 133 | + | { | |
| 134 | + | name: "page negative timeout", | |
| 135 | + | modePath: "page_mode", | |
| 136 | + | timeout: -1, | |
| 137 | + | actions: map[string]ActionBinding{"page-scroll-left": {Keys: []string{"h"}}}, | |
| 138 | + | want: "workspace.page_mode.timeout_ms must be non-negative", | |
| 139 | + | }, | |
| 140 | + | { | |
| 141 | + | name: "pane empty actions", | |
| 142 | + | modePath: "pane_mode", | |
| 143 | + | timeout: 0, | |
| 144 | + | actions: nil, | |
| 145 | + | want: "workspace.pane_mode.actions cannot be empty", | |
| 146 | + | }, | |
| 147 | + | { | |
| 148 | + | name: "tab empty keys", | |
| 149 | + | modePath: "tab_mode", | |
| 150 | + | timeout: 0, | |
| 151 | + | actions: map[string]ActionBinding{"new-tab": {Keys: []string{}}}, | |
| 152 | + | want: "workspace.tab_mode.actions.new-tab must have at least one key binding", | |
| 153 | + | }, | |
| 154 | + | { | |
| 155 | + | name: "page duplicate vim binding", | |
| 156 | + | modePath: "page_mode", | |
| 157 | + | timeout: 0, | |
| 158 | + | actions: map[string]ActionBinding{ | |
| 159 | + | "page-scroll-left": {Keys: []string{"h"}}, | |
| 160 | + | "page-scroll-right": {Keys: []string{"h"}}, | |
| 161 | + | }, | |
| 162 | + | want: "duplicate key binding 'h' found in page_mode actions", | |
| 163 | + | }, | |
| 164 | + | { | |
| 165 | + | name: "page canonical vim set ok", | |
| 166 | + | modePath: "page_mode", | |
| 167 | + | timeout: 0, | |
| 168 | + | actions: map[string]ActionBinding{ | |
| 169 | + | "page-scroll-left": {Keys: []string{"h"}}, | |
| 170 | + | "page-scroll-down": {Keys: []string{"j"}}, | |
| 171 | + | "page-scroll-up": {Keys: []string{"k"}}, | |
| 172 | + | "page-scroll-right": {Keys: []string{"l"}}, | |
| 173 | + | "page-scroll-down-fast": {Keys: []string{"shift+j"}}, | |
| 174 | + | "page-scroll-up-fast": {Keys: []string{"shift+k"}}, | |
| 175 | + | "confirm": {Keys: []string{"enter"}}, | |
| 176 | + | "cancel": {Keys: []string{"escape"}}, | |
| 177 | + | }, | |
| 178 | + | want: "", | |
| 179 | + | }, | |
| 180 | + | } | |
| 181 | + | ||
| 182 | + | for _, tt := range tests { | |
| 183 | + | t.Run(tt.name, func(t *testing.T) { | |
| 184 | + | errs := validateModalModeActions(tt.modePath, tt.timeout, tt.actions) | |
| 185 | + | if tt.want == "" { | |
| 186 | + | assert.Empty(t, errs) | |
| 187 | + | return | |
| 188 | + | } | |
| 189 | + | require.NotEmpty(t, errs) | |
| 190 | + | assert.Contains(t, errs[0], tt.want) | |
| 191 | + | }) | |
| 192 | + | } | |
| 193 | + | } | |
| 194 | + | ||
| 125 | 195 | func TestSchemaProvider_PageModeKeys(t *testing.T) { | |
| 126 | 196 | provider := &SchemaProvider{} | |
| 127 | 197 | schema := provider.GetSchema() | |
| @@ -122,6 +122,76 @@ func TestValidatePageMode_EmptyActionKeysFail(t *testing.T) { | |||||
| 122 | assert.Contains(t, err.Error(), "workspace.page_mode.actions.page-scroll-left must have at least one key binding") | 122 | assert.Contains(t, err.Error(), "workspace.page_mode.actions.page-scroll-left must have at least one key binding") | ||
| 123 | } | 123 | } | ||
| 124 | 124 | ||||
| 125 | + | func TestValidateModalModeActions_RegressionTable(t *testing.T) { | |||
| 126 | + | tests := []struct { | |||
| 127 | + | name string | |||
| 128 | + | modePath string | |||
| 129 | + | timeout int | |||
| 130 | + | actions map[string]ActionBinding | |||
| 131 | + | want string | |||
| 132 | + | }{ | |||
| 133 | + | { | |||
| 134 | + | name: "page negative timeout", | |||
| 135 | + | modePath: "page_mode", | |||
| 136 | + | timeout: -1, | |||
| 137 | + | actions: map[string]ActionBinding{"page-scroll-left": {Keys: []string{"h"}}}, | |||
| 138 | + | want: "workspace.page_mode.timeout_ms must be non-negative", | |||
| 139 | + | }, | |||
| 140 | + | { | |||
| 141 | + | name: "pane empty actions", | |||
| 142 | + | modePath: "pane_mode", | |||
| 143 | + | timeout: 0, | |||
| 144 | + | actions: nil, | |||
| 145 | + | want: "workspace.pane_mode.actions cannot be empty", | |||
| 146 | + | }, | |||
| 147 | + | { | |||
| 148 | + | name: "tab empty keys", | |||
| 149 | + | modePath: "tab_mode", | |||
| 150 | + | timeout: 0, | |||
| 151 | + | actions: map[string]ActionBinding{"new-tab": {Keys: []string{}}}, | |||
| 152 | + | want: "workspace.tab_mode.actions.new-tab must have at least one key binding", | |||
| 153 | + | }, | |||
| 154 | + | { | |||
| 155 | + | name: "page duplicate vim binding", | |||
| 156 | + | modePath: "page_mode", | |||
| 157 | + | timeout: 0, | |||
| 158 | + | actions: map[string]ActionBinding{ | |||
| 159 | + | "page-scroll-left": {Keys: []string{"h"}}, | |||
| 160 | + | "page-scroll-right": {Keys: []string{"h"}}, | |||
| 161 | + | }, | |||
| 162 | + | want: "duplicate key binding 'h' found in page_mode actions", | |||
| 163 | + | }, | |||
| 164 | + | { | |||
| 165 | + | name: "page canonical vim set ok", | |||
| 166 | + | modePath: "page_mode", | |||
| 167 | + | timeout: 0, | |||
| 168 | + | actions: map[string]ActionBinding{ | |||
| 169 | + | "page-scroll-left": {Keys: []string{"h"}}, | |||
| 170 | + | "page-scroll-down": {Keys: []string{"j"}}, | |||
| 171 | + | "page-scroll-up": {Keys: []string{"k"}}, | |||
| 172 | + | "page-scroll-right": {Keys: []string{"l"}}, | |||
| 173 | + | "page-scroll-down-fast": {Keys: []string{"shift+j"}}, | |||
| 174 | + | "page-scroll-up-fast": {Keys: []string{"shift+k"}}, | |||
| 175 | + | "confirm": {Keys: []string{"enter"}}, | |||
| 176 | + | "cancel": {Keys: []string{"escape"}}, | |||
| 177 | + | }, | |||
| 178 | + | want: "", | |||
| 179 | + | }, | |||
| 180 | + | } | |||
| 181 | + | ||||
| 182 | + | for _, tt := range tests { | |||
| 183 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 184 | + | errs := validateModalModeActions(tt.modePath, tt.timeout, tt.actions) | |||
| 185 | + | if tt.want == "" { | |||
| 186 | + | assert.Empty(t, errs) | |||
| 187 | + | return | |||
| 188 | + | } | |||
| 189 | + | require.NotEmpty(t, errs) | |||
| 190 | + | assert.Contains(t, errs[0], tt.want) | |||
| 191 | + | }) | |||
| 192 | + | } | |||
| 193 | + | } | |||
| 194 | + | ||||
| 125 | func TestSchemaProvider_PageModeKeys(t *testing.T) { | 195 | func TestSchemaProvider_PageModeKeys(t *testing.T) { | ||
| 126 | provider := &SchemaProvider{} | 196 | provider := &SchemaProvider{} | ||
| 127 | schema := provider.GetSchema() | 197 | schema := provider.GetSchema() | ||
M internal/infrastructure/config/validation.go +16 -59
| @@ -203,32 +203,7 @@ func validateWorkspaceStyling(config *Config) []string { | |||
| 203 | 203 | } | |
| 204 | 204 | ||
| 205 | 205 | func validatePaneMode(config *Config) []string { | |
| 206 | - | var validationErrors []string | |
| 207 | - | if config.Workspace.PaneMode.TimeoutMilliseconds < 0 { | |
| 208 | - | validationErrors = append(validationErrors, "workspace.pane_mode.timeout_ms must be non-negative") | |
| 209 | - | } | |
| 210 | - | if len(config.Workspace.PaneMode.Actions) == 0 { | |
| 211 | - | validationErrors = append(validationErrors, "workspace.pane_mode.actions cannot be empty") | |
| 212 | - | } | |
| 213 | - | ||
| 214 | - | seenKeys := make(map[string]string) | |
| 215 | - | for action, binding := range config.Workspace.PaneMode.Actions { | |
| 216 | - | if len(binding.Keys) == 0 { | |
| 217 | - | validationErrors = append(validationErrors, fmt.Sprintf("workspace.pane_mode.actions.%s must have at least one key binding", action)) | |
| 218 | - | } | |
| 219 | - | for _, key := range binding.Keys { | |
| 220 | - | if existingAction, exists := seenKeys[key]; exists { | |
| 221 | - | validationErrors = append(validationErrors, fmt.Sprintf( | |
| 222 | - | "duplicate key binding '%s' found in pane_mode actions '%s' and '%s'", | |
| 223 | - | key, | |
| 224 | - | existingAction, | |
| 225 | - | action, | |
| 226 | - | )) | |
| 227 | - | } | |
| 228 | - | seenKeys[key] = action | |
| 229 | - | } | |
| 230 | - | } | |
| 231 | - | return validationErrors | |
| 206 | + | return validateModalModeActions("pane_mode", config.Workspace.PaneMode.TimeoutMilliseconds, config.Workspace.PaneMode.Actions) | |
| 232 | 207 | } | |
| 233 | 208 | ||
| 234 | 209 | func validateTabBar(config *Config) []string { | |
| @@ -241,53 +216,35 @@ func validateTabBar(config *Config) []string { | |||
| 241 | 216 | } | |
| 242 | 217 | ||
| 243 | 218 | func validateTabMode(config *Config) []string { | |
| 244 | - | var validationErrors []string | |
| 245 | - | if config.Workspace.TabMode.TimeoutMilliseconds < 0 { | |
| 246 | - | validationErrors = append(validationErrors, "workspace.tab_mode.timeout_ms must be non-negative") | |
| 247 | - | } | |
| 248 | - | if len(config.Workspace.TabMode.Actions) == 0 { | |
| 249 | - | validationErrors = append(validationErrors, "workspace.tab_mode.actions cannot be empty") | |
| 250 | - | } | |
| 251 | - | ||
| 252 | - | tabSeenKeys := make(map[string]string) | |
| 253 | - | for action, binding := range config.Workspace.TabMode.Actions { | |
| 254 | - | if len(binding.Keys) == 0 { | |
| 255 | - | validationErrors = append(validationErrors, fmt.Sprintf("workspace.tab_mode.actions.%s must have at least one key binding", action)) | |
| 256 | - | } | |
| 257 | - | for _, key := range binding.Keys { | |
| 258 | - | if existingAction, exists := tabSeenKeys[key]; exists { | |
| 259 | - | validationErrors = append(validationErrors, fmt.Sprintf( | |
| 260 | - | "duplicate key binding '%s' found in tab_mode actions '%s' and '%s'", | |
| 261 | - | key, | |
| 262 | - | existingAction, | |
| 263 | - | action, | |
| 264 | - | )) | |
| 265 | - | } | |
| 266 | - | tabSeenKeys[key] = action | |
| 267 | - | } | |
| 268 | - | } | |
| 269 | - | return validationErrors | |
| 219 | + | return validateModalModeActions("tab_mode", config.Workspace.TabMode.TimeoutMilliseconds, config.Workspace.TabMode.Actions) | |
| 270 | 220 | } | |
| 271 | 221 | ||
| 272 | 222 | func validatePageMode(config *Config) []string { | |
| 223 | + | return validateModalModeActions("page_mode", config.Workspace.PageMode.TimeoutMilliseconds, config.Workspace.PageMode.Actions) | |
| 224 | + | } | |
| 225 | + | ||
| 226 | + | // validateModalModeActions shared timeout/empty-actions/duplicate-key checks for | |
| 227 | + | // pane, tab, and page modal modes. Messages stay path-exact via modePath. | |
| 228 | + | func validateModalModeActions(modePath string, timeoutMS int, actions map[string]ActionBinding) []string { | |
| 273 | 229 | var validationErrors []string | |
| 274 | - | if config.Workspace.PageMode.TimeoutMilliseconds < 0 { | |
| 275 | - | validationErrors = append(validationErrors, "workspace.page_mode.timeout_ms must be non-negative") | |
| 230 | + | if timeoutMS < 0 { | |
| 231 | + | validationErrors = append(validationErrors, fmt.Sprintf("workspace.%s.timeout_ms must be non-negative", modePath)) | |
| 276 | 232 | } | |
| 277 | - | if len(config.Workspace.PageMode.Actions) == 0 { | |
| 278 | - | validationErrors = append(validationErrors, "workspace.page_mode.actions cannot be empty") | |
| 233 | + | if len(actions) == 0 { | |
| 234 | + | validationErrors = append(validationErrors, fmt.Sprintf("workspace.%s.actions cannot be empty", modePath)) | |
| 279 | 235 | } | |
| 280 | 236 | ||
| 281 | 237 | seenKeys := make(map[string]string) | |
| 282 | - | for action, binding := range config.Workspace.PageMode.Actions { | |
| 238 | + | for action, binding := range actions { | |
| 283 | 239 | if len(binding.Keys) == 0 { | |
| 284 | - | validationErrors = append(validationErrors, fmt.Sprintf("workspace.page_mode.actions.%s must have at least one key binding", action)) | |
| 240 | + | validationErrors = append(validationErrors, fmt.Sprintf("workspace.%s.actions.%s must have at least one key binding", modePath, action)) | |
| 285 | 241 | } | |
| 286 | 242 | for _, key := range binding.Keys { | |
| 287 | 243 | if existingAction, exists := seenKeys[key]; exists { | |
| 288 | 244 | validationErrors = append(validationErrors, fmt.Sprintf( | |
| 289 | - | "duplicate key binding '%s' found in page_mode actions '%s' and '%s'", | |
| 245 | + | "duplicate key binding '%s' found in %s actions '%s' and '%s'", | |
| 290 | 246 | key, | |
| 247 | + | modePath, | |
| 291 | 248 | existingAction, | |
| 292 | 249 | action, | |
| 293 | 250 | )) | |
| @@ -203,32 +203,7 @@ func validateWorkspaceStyling(config *Config) []string { | |||||
| 203 | } | 203 | } | ||
| 204 | 204 | ||||
| 205 | func validatePaneMode(config *Config) []string { | 205 | func validatePaneMode(config *Config) []string { | ||
| 206 | - | var validationErrors []string | 206 | + | return validateModalModeActions("pane_mode", config.Workspace.PaneMode.TimeoutMilliseconds, config.Workspace.PaneMode.Actions) |
| 207 | - | if config.Workspace.PaneMode.TimeoutMilliseconds < 0 { | |||
| 208 | - | validationErrors = append(validationErrors, "workspace.pane_mode.timeout_ms must be non-negative") | |||
| 209 | - | } | |||
| 210 | - | if len(config.Workspace.PaneMode.Actions) == 0 { | |||
| 211 | - | validationErrors = append(validationErrors, "workspace.pane_mode.actions cannot be empty") | |||
| 212 | - | } | |||
| 213 | - | ||||
| 214 | - | seenKeys := make(map[string]string) | |||
| 215 | - | for action, binding := range config.Workspace.PaneMode.Actions { | |||
| 216 | - | if len(binding.Keys) == 0 { | |||
| 217 | - | validationErrors = append(validationErrors, fmt.Sprintf("workspace.pane_mode.actions.%s must have at least one key binding", action)) | |||
| 218 | - | } | |||
| 219 | - | for _, key := range binding.Keys { | |||
| 220 | - | if existingAction, exists := seenKeys[key]; exists { | |||
| 221 | - | validationErrors = append(validationErrors, fmt.Sprintf( | |||
| 222 | - | "duplicate key binding '%s' found in pane_mode actions '%s' and '%s'", | |||
| 223 | - | key, | |||
| 224 | - | existingAction, | |||
| 225 | - | action, | |||
| 226 | - | )) | |||
| 227 | - | } | |||
| 228 | - | seenKeys[key] = action | |||
| 229 | - | } | |||
| 230 | - | } | |||
| 231 | - | return validationErrors | |||
| 232 | } | 207 | } | ||
| 233 | 208 | ||||
| 234 | func validateTabBar(config *Config) []string { | 209 | func validateTabBar(config *Config) []string { | ||
| @@ -241,53 +216,35 @@ func validateTabBar(config *Config) []string { | |||||
| 241 | } | 216 | } | ||
| 242 | 217 | ||||
| 243 | func validateTabMode(config *Config) []string { | 218 | func validateTabMode(config *Config) []string { | ||
| 244 | - | var validationErrors []string | 219 | + | return validateModalModeActions("tab_mode", config.Workspace.TabMode.TimeoutMilliseconds, config.Workspace.TabMode.Actions) |
| 245 | - | if config.Workspace.TabMode.TimeoutMilliseconds < 0 { | |||
| 246 | - | validationErrors = append(validationErrors, "workspace.tab_mode.timeout_ms must be non-negative") | |||
| 247 | - | } | |||
| 248 | - | if len(config.Workspace.TabMode.Actions) == 0 { | |||
| 249 | - | validationErrors = append(validationErrors, "workspace.tab_mode.actions cannot be empty") | |||
| 250 | - | } | |||
| 251 | - | ||||
| 252 | - | tabSeenKeys := make(map[string]string) | |||
| 253 | - | for action, binding := range config.Workspace.TabMode.Actions { | |||
| 254 | - | if len(binding.Keys) == 0 { | |||
| 255 | - | validationErrors = append(validationErrors, fmt.Sprintf("workspace.tab_mode.actions.%s must have at least one key binding", action)) | |||
| 256 | - | } | |||
| 257 | - | for _, key := range binding.Keys { | |||
| 258 | - | if existingAction, exists := tabSeenKeys[key]; exists { | |||
| 259 | - | validationErrors = append(validationErrors, fmt.Sprintf( | |||
| 260 | - | "duplicate key binding '%s' found in tab_mode actions '%s' and '%s'", | |||
| 261 | - | key, | |||
| 262 | - | existingAction, | |||
| 263 | - | action, | |||
| 264 | - | )) | |||
| 265 | - | } | |||
| 266 | - | tabSeenKeys[key] = action | |||
| 267 | - | } | |||
| 268 | - | } | |||
| 269 | - | return validationErrors | |||
| 270 | } | 220 | } | ||
| 271 | 221 | ||||
| 272 | func validatePageMode(config *Config) []string { | 222 | func validatePageMode(config *Config) []string { | ||
| 223 | + | return validateModalModeActions("page_mode", config.Workspace.PageMode.TimeoutMilliseconds, config.Workspace.PageMode.Actions) | |||
| 224 | + | } | |||
| 225 | + | ||||
| 226 | + | // validateModalModeActions shared timeout/empty-actions/duplicate-key checks for | |||
| 227 | + | // pane, tab, and page modal modes. Messages stay path-exact via modePath. | |||
| 228 | + | func validateModalModeActions(modePath string, timeoutMS int, actions map[string]ActionBinding) []string { | |||
| 273 | var validationErrors []string | 229 | var validationErrors []string | ||
| 274 | - | if config.Workspace.PageMode.TimeoutMilliseconds < 0 { | 230 | + | if timeoutMS < 0 { |
| 275 | - | validationErrors = append(validationErrors, "workspace.page_mode.timeout_ms must be non-negative") | 231 | + | validationErrors = append(validationErrors, fmt.Sprintf("workspace.%s.timeout_ms must be non-negative", modePath)) |
| 276 | } | 232 | } | ||
| 277 | - | if len(config.Workspace.PageMode.Actions) == 0 { | 233 | + | if len(actions) == 0 { |
| 278 | - | validationErrors = append(validationErrors, "workspace.page_mode.actions cannot be empty") | 234 | + | validationErrors = append(validationErrors, fmt.Sprintf("workspace.%s.actions cannot be empty", modePath)) |
| 279 | } | 235 | } | ||
| 280 | 236 | ||||
| 281 | seenKeys := make(map[string]string) | 237 | seenKeys := make(map[string]string) | ||
| 282 | - | for action, binding := range config.Workspace.PageMode.Actions { | 238 | + | for action, binding := range actions { |
| 283 | if len(binding.Keys) == 0 { | 239 | if len(binding.Keys) == 0 { | ||
| 284 | - | validationErrors = append(validationErrors, fmt.Sprintf("workspace.page_mode.actions.%s must have at least one key binding", action)) | 240 | + | validationErrors = append(validationErrors, fmt.Sprintf("workspace.%s.actions.%s must have at least one key binding", modePath, action)) |
| 285 | } | 241 | } | ||
| 286 | for _, key := range binding.Keys { | 242 | for _, key := range binding.Keys { | ||
| 287 | if existingAction, exists := seenKeys[key]; exists { | 243 | if existingAction, exists := seenKeys[key]; exists { | ||
| 288 | validationErrors = append(validationErrors, fmt.Sprintf( | 244 | validationErrors = append(validationErrors, fmt.Sprintf( | ||
| 289 | - | "duplicate key binding '%s' found in page_mode actions '%s' and '%s'", | 245 | + | "duplicate key binding '%s' found in %s actions '%s' and '%s'", |
| 290 | key, | 246 | key, | ||
| 247 | + | modePath, | |||
| 291 | existingAction, | 248 | existingAction, | ||
| 292 | action, | 249 | action, | ||
| 293 | )) | 250 | )) | ||