fix(vimkeys): preserve canonical sequence ownership
- commit
- c67625f54269f35012614b2e7474e6d67dba393c
- author
- brice · · 2026-07-29 05:24 UTC
- committer
- bnema · · 2026-08-14 02:42 UTC
- parent
- 4f688ff
- signature
- unsigned
4 files · +150 -66
M internal/domain/vimkeys/parse.go +3 -59
| @@ -229,11 +229,9 @@ func parseRawSequence(s string) (Sequence, error) { | |||
| 229 | 229 | i += width | |
| 230 | 230 | continue | |
| 231 | 231 | } | |
| 232 | - | if key, width, ok := tryParseSpecialAt(s, i); ok { | |
| 233 | - | seq = append(seq, key) | |
| 234 | - | i += width | |
| 235 | - | continue | |
| 236 | - | } | |
| 232 | + | // Named keys (Space, Esc, Tab, …) are canonical only inside <...>. | |
| 233 | + | // Bare prefixes must not be greedily consumed so printable runs like | |
| 234 | + | // "Spacej" (Shift+s,p,a,c,e,j) round-trip via Sequence.String(). | |
| 237 | 235 | if key, width, ok := tryParseCanonicalAt(s, i); ok { | |
| 238 | 236 | seq = append(seq, key) | |
| 239 | 237 | i += width | |
| @@ -271,60 +269,6 @@ func parseAngleAt(s string, i int) (Key, int, error) { | |||
| 271 | 269 | return key, closeIdx - i + 1, nil | |
| 272 | 270 | } | |
| 273 | 271 | ||
| 274 | - | func tryParseSpecialAt(s string, i int) (Key, int, bool) { | |
| 275 | - | if i >= len(s) { | |
| 276 | - | return Key{}, 0, false | |
| 277 | - | } | |
| 278 | - | rest := s[i:] | |
| 279 | - | if key, width, ok := tryParseNamedSpecialAt(rest); ok { | |
| 280 | - | return key, width, true | |
| 281 | - | } | |
| 282 | - | switch { | |
| 283 | - | case strings.HasPrefix(rest, "Space"): | |
| 284 | - | return Key{Sym: "Space"}, len("Space"), true | |
| 285 | - | case strings.HasPrefix(rest, "Plus"): | |
| 286 | - | return Key{Sym: "+"}, len("Plus"), true | |
| 287 | - | case strings.HasPrefix(rest, "Esc"): | |
| 288 | - | return Key{Sym: "Esc"}, len("Esc"), true | |
| 289 | - | case strings.HasPrefix(rest, "CR"): | |
| 290 | - | return Key{Sym: "CR"}, len("CR"), true | |
| 291 | - | case strings.HasPrefix(rest, "lt"): | |
| 292 | - | return Key{Sym: "<"}, len("lt"), true | |
| 293 | - | default: | |
| 294 | - | return Key{}, 0, false | |
| 295 | - | } | |
| 296 | - | } | |
| 297 | - | ||
| 298 | - | // tryParseNamedSpecialAt matches longer canonical named keys before letter runs. | |
| 299 | - | func tryParseNamedSpecialAt(rest string) (Key, int, bool) { | |
| 300 | - | switch { | |
| 301 | - | case strings.HasPrefix(rest, "BackSpace"): | |
| 302 | - | return Key{Sym: "BackSpace"}, len("BackSpace"), true | |
| 303 | - | case strings.HasPrefix(rest, "PageDown"): | |
| 304 | - | return Key{Sym: "PageDown"}, len("PageDown"), true | |
| 305 | - | case strings.HasPrefix(rest, "PageUp"): | |
| 306 | - | return Key{Sym: "PageUp"}, len("PageUp"), true | |
| 307 | - | case strings.HasPrefix(rest, "Delete"): | |
| 308 | - | return Key{Sym: "Delete"}, len("Delete"), true | |
| 309 | - | case strings.HasPrefix(rest, "Right"): | |
| 310 | - | return Key{Sym: "Right"}, len("Right"), true | |
| 311 | - | case strings.HasPrefix(rest, "Left"): | |
| 312 | - | return Key{Sym: "Left"}, len("Left"), true | |
| 313 | - | case strings.HasPrefix(rest, "Home"): | |
| 314 | - | return Key{Sym: "Home"}, len("Home"), true | |
| 315 | - | case strings.HasPrefix(rest, "Down"): | |
| 316 | - | return Key{Sym: "Down"}, len("Down"), true | |
| 317 | - | case strings.HasPrefix(rest, "Tab"): | |
| 318 | - | return Key{Sym: "Tab"}, len("Tab"), true | |
| 319 | - | case strings.HasPrefix(rest, "End"): | |
| 320 | - | return Key{Sym: "End"}, len("End"), true | |
| 321 | - | case strings.HasPrefix(rest, "Up"): | |
| 322 | - | return Key{Sym: "Up"}, len("Up"), true | |
| 323 | - | default: | |
| 324 | - | return Key{}, 0, false | |
| 325 | - | } | |
| 326 | - | } | |
| 327 | - | ||
| 328 | 272 | func tryParseCanonicalAt(s string, i int) (Key, int, bool) { | |
| 329 | 273 | if i >= len(s) { | |
| 330 | 274 | return Key{}, 0, false | |
| @@ -229,11 +229,9 @@ func parseRawSequence(s string) (Sequence, error) { | |||||
| 229 | i += width | 229 | i += width | ||
| 230 | continue | 230 | continue | ||
| 231 | } | 231 | } | ||
| 232 | - | if key, width, ok := tryParseSpecialAt(s, i); ok { | 232 | + | // Named keys (Space, Esc, Tab, …) are canonical only inside <...>. |
| 233 | - | seq = append(seq, key) | 233 | + | // Bare prefixes must not be greedily consumed so printable runs like |
| 234 | - | i += width | 234 | + | // "Spacej" (Shift+s,p,a,c,e,j) round-trip via Sequence.String(). |
| 235 | - | continue | |||
| 236 | - | } | |||
| 237 | if key, width, ok := tryParseCanonicalAt(s, i); ok { | 235 | if key, width, ok := tryParseCanonicalAt(s, i); ok { | ||
| 238 | seq = append(seq, key) | 236 | seq = append(seq, key) | ||
| 239 | i += width | 237 | i += width | ||
| @@ -271,60 +269,6 @@ func parseAngleAt(s string, i int) (Key, int, error) { | |||||
| 271 | return key, closeIdx - i + 1, nil | 269 | return key, closeIdx - i + 1, nil | ||
| 272 | } | 270 | } | ||
| 273 | 271 | ||||
| 274 | - | func tryParseSpecialAt(s string, i int) (Key, int, bool) { | |||
| 275 | - | if i >= len(s) { | |||
| 276 | - | return Key{}, 0, false | |||
| 277 | - | } | |||
| 278 | - | rest := s[i:] | |||
| 279 | - | if key, width, ok := tryParseNamedSpecialAt(rest); ok { | |||
| 280 | - | return key, width, true | |||
| 281 | - | } | |||
| 282 | - | switch { | |||
| 283 | - | case strings.HasPrefix(rest, "Space"): | |||
| 284 | - | return Key{Sym: "Space"}, len("Space"), true | |||
| 285 | - | case strings.HasPrefix(rest, "Plus"): | |||
| 286 | - | return Key{Sym: "+"}, len("Plus"), true | |||
| 287 | - | case strings.HasPrefix(rest, "Esc"): | |||
| 288 | - | return Key{Sym: "Esc"}, len("Esc"), true | |||
| 289 | - | case strings.HasPrefix(rest, "CR"): | |||
| 290 | - | return Key{Sym: "CR"}, len("CR"), true | |||
| 291 | - | case strings.HasPrefix(rest, "lt"): | |||
| 292 | - | return Key{Sym: "<"}, len("lt"), true | |||
| 293 | - | default: | |||
| 294 | - | return Key{}, 0, false | |||
| 295 | - | } | |||
| 296 | - | } | |||
| 297 | - | ||||
| 298 | - | // tryParseNamedSpecialAt matches longer canonical named keys before letter runs. | |||
| 299 | - | func tryParseNamedSpecialAt(rest string) (Key, int, bool) { | |||
| 300 | - | switch { | |||
| 301 | - | case strings.HasPrefix(rest, "BackSpace"): | |||
| 302 | - | return Key{Sym: "BackSpace"}, len("BackSpace"), true | |||
| 303 | - | case strings.HasPrefix(rest, "PageDown"): | |||
| 304 | - | return Key{Sym: "PageDown"}, len("PageDown"), true | |||
| 305 | - | case strings.HasPrefix(rest, "PageUp"): | |||
| 306 | - | return Key{Sym: "PageUp"}, len("PageUp"), true | |||
| 307 | - | case strings.HasPrefix(rest, "Delete"): | |||
| 308 | - | return Key{Sym: "Delete"}, len("Delete"), true | |||
| 309 | - | case strings.HasPrefix(rest, "Right"): | |||
| 310 | - | return Key{Sym: "Right"}, len("Right"), true | |||
| 311 | - | case strings.HasPrefix(rest, "Left"): | |||
| 312 | - | return Key{Sym: "Left"}, len("Left"), true | |||
| 313 | - | case strings.HasPrefix(rest, "Home"): | |||
| 314 | - | return Key{Sym: "Home"}, len("Home"), true | |||
| 315 | - | case strings.HasPrefix(rest, "Down"): | |||
| 316 | - | return Key{Sym: "Down"}, len("Down"), true | |||
| 317 | - | case strings.HasPrefix(rest, "Tab"): | |||
| 318 | - | return Key{Sym: "Tab"}, len("Tab"), true | |||
| 319 | - | case strings.HasPrefix(rest, "End"): | |||
| 320 | - | return Key{Sym: "End"}, len("End"), true | |||
| 321 | - | case strings.HasPrefix(rest, "Up"): | |||
| 322 | - | return Key{Sym: "Up"}, len("Up"), true | |||
| 323 | - | default: | |||
| 324 | - | return Key{}, 0, false | |||
| 325 | - | } | |||
| 326 | - | } | |||
| 327 | - | ||||
| 328 | func tryParseCanonicalAt(s string, i int) (Key, int, bool) { | 272 | func tryParseCanonicalAt(s string, i int) (Key, int, bool) { | ||
| 329 | if i >= len(s) { | 273 | if i >= len(s) { | ||
| 330 | return Key{}, 0, false | 274 | return Key{}, 0, false | ||
M internal/domain/vimkeys/parse_test.go +110 -5
| @@ -241,11 +241,13 @@ func TestParseBinding_SpecialNamesInSequence(t *testing.T) { | |||
| 241 | 241 | input string | |
| 242 | 242 | want Sequence | |
| 243 | 243 | }{ | |
| 244 | - | {input: "jEsc", want: Sequence{{Sym: "j"}, {Sym: "Esc"}}}, | |
| 245 | - | {input: "Spacej", want: Sequence{{Sym: "Space"}, {Sym: "j"}}}, | |
| 246 | - | {input: "CRj", want: Sequence{{Sym: "CR"}, {Sym: "j"}}}, | |
| 247 | - | {input: "ltj", want: Sequence{{Sym: "<"}, {Sym: "j"}}}, | |
| 248 | - | {input: "Plusj", want: Sequence{{Sym: "+"}, {Sym: "j"}}}, | |
| 244 | + | {input: "j<Esc>", want: Sequence{{Sym: "j"}, {Sym: "Esc"}}}, | |
| 245 | + | {input: "<Space>j", want: Sequence{{Sym: "Space"}, {Sym: "j"}}}, | |
| 246 | + | {input: "<CR>j", want: Sequence{{Sym: "CR"}, {Sym: "j"}}}, | |
| 247 | + | {input: "<lt>j", want: Sequence{{Sym: "<"}, {Sym: "j"}}}, | |
| 248 | + | {input: "<Plus>j", want: Sequence{{Sym: "+"}, {Sym: "j"}}}, | |
| 249 | + | {input: "j<Tab>", want: Sequence{{Sym: "j"}, {Sym: "Tab"}}}, | |
| 250 | + | {input: "<BackSpace>j", want: Sequence{{Sym: "BackSpace"}, {Sym: "j"}}}, | |
| 249 | 251 | } | |
| 250 | 252 | for _, tt := range tests { | |
| 251 | 253 | t.Run(tt.input, func(t *testing.T) { | |
| @@ -260,6 +262,28 @@ func TestParseBinding_SpecialNamesInSequence(t *testing.T) { | |||
| 260 | 262 | } | |
| 261 | 263 | } | |
| 262 | 264 | ||
| 265 | + | func TestParseBinding_BareNamedPrefixIsLiteral(t *testing.T) { | |
| 266 | + | // Named keys are canonical only in <...>; bare "Spacej" is Shift+s + p + a + c + e + j. | |
| 267 | + | seq := Sequence{ | |
| 268 | + | {Sym: "s", Mods: ModShift}, | |
| 269 | + | {Sym: "p"}, | |
| 270 | + | {Sym: "a"}, | |
| 271 | + | {Sym: "c"}, | |
| 272 | + | {Sym: "e"}, | |
| 273 | + | {Sym: "j"}, | |
| 274 | + | } | |
| 275 | + | if got := seq.String(); got != "Spacej" { | |
| 276 | + | t.Fatalf("seq.String() = %q, want %q", got, "Spacej") | |
| 277 | + | } | |
| 278 | + | got, err := ParseBinding("Spacej") | |
| 279 | + | if err != nil { | |
| 280 | + | t.Fatalf("ParseBinding(%q) error = %v", "Spacej", err) | |
| 281 | + | } | |
| 282 | + | if !got.Equal(seq) { | |
| 283 | + | t.Fatalf("ParseBinding(%q) = %v, want %v", "Spacej", got, seq) | |
| 284 | + | } | |
| 285 | + | } | |
| 286 | + | ||
| 263 | 287 | func TestParseBinding_PrintableGlyphs(t *testing.T) { | |
| 264 | 288 | got, err := ParseBinding("!@#") | |
| 265 | 289 | if err != nil { | |
| @@ -356,6 +380,13 @@ func TestParseBinding_RoundTrip(t *testing.T) { | |||
| 356 | 380 | "<C-d>j", | |
| 357 | 381 | "<Escape>j", | |
| 358 | 382 | "<C-S-a>", | |
| 383 | + | "Spacej", | |
| 384 | + | "Escj", | |
| 385 | + | "Tabx", | |
| 386 | + | "enter", | |
| 387 | + | "escape", | |
| 388 | + | "return", | |
| 389 | + | "esc", | |
| 359 | 390 | } | |
| 360 | 391 | for _, input := range inputs { | |
| 361 | 392 | t.Run(input, func(t *testing.T) { | |
| @@ -374,3 +405,77 @@ func TestParseBinding_RoundTrip(t *testing.T) { | |||
| 374 | 405 | }) | |
| 375 | 406 | } | |
| 376 | 407 | } | |
| 408 | + | ||
| 409 | + | // TestParseBinding_DeterministicRoundTripProperty asserts ParseBinding(seq.String()) == seq | |
| 410 | + | // over a fixed representative set of printable, named, and modifier sequences. | |
| 411 | + | // The corpus is deterministic (no flaky randomness). | |
| 412 | + | func TestParseBinding_DeterministicRoundTripProperty(t *testing.T) { | |
| 413 | + | corpus := []Sequence{ | |
| 414 | + | {{Sym: "j"}}, | |
| 415 | + | {{Sym: "j", Mods: ModShift}}, | |
| 416 | + | {{Sym: "]"}, {Sym: "]"}}, | |
| 417 | + | {{Sym: "y"}, {Sym: "a"}, {Sym: "h"}}, | |
| 418 | + | {{Sym: "g"}, {Sym: "o", Mods: ModShift}}, | |
| 419 | + | {{Sym: "d", Mods: ModCtrl}}, | |
| 420 | + | {{Sym: "a", Mods: ModCtrl | ModShift}}, | |
| 421 | + | {{Sym: "k", Mods: ModAlt}}, | |
| 422 | + | {{Sym: "CR"}}, | |
| 423 | + | {{Sym: "Esc"}}, | |
| 424 | + | {{Sym: "Space"}}, | |
| 425 | + | {{Sym: "Tab"}}, | |
| 426 | + | {{Sym: "BackSpace"}}, | |
| 427 | + | {{Sym: "Delete"}}, | |
| 428 | + | {{Sym: "Left"}}, | |
| 429 | + | {{Sym: "Right"}}, | |
| 430 | + | {{Sym: "Up"}}, | |
| 431 | + | {{Sym: "Down"}}, | |
| 432 | + | {{Sym: "Home"}}, | |
| 433 | + | {{Sym: "End"}}, | |
| 434 | + | {{Sym: "PageUp"}}, | |
| 435 | + | {{Sym: "PageDown"}}, | |
| 436 | + | {{Sym: "Tab", Mods: ModCtrl}}, | |
| 437 | + | {{Sym: "BackSpace", Mods: ModShift}}, | |
| 438 | + | {{Sym: "Delete", Mods: ModAlt}}, | |
| 439 | + | {{Sym: "<"}}, | |
| 440 | + | {{Sym: "+"}}, | |
| 441 | + | {{Sym: "!"}}, | |
| 442 | + | {{Sym: "@"}}, | |
| 443 | + | {{Sym: "#"}}, | |
| 444 | + | {{Sym: "Space"}, {Sym: "j"}}, | |
| 445 | + | {{Sym: "d", Mods: ModCtrl}, {Sym: "j"}}, | |
| 446 | + | {{Sym: "Esc"}, {Sym: "j"}}, | |
| 447 | + | // Literal printable run that formerly collided with greedy "Space" recognition. | |
| 448 | + | { | |
| 449 | + | {Sym: "s", Mods: ModShift}, | |
| 450 | + | {Sym: "p"}, | |
| 451 | + | {Sym: "a"}, | |
| 452 | + | {Sym: "c"}, | |
| 453 | + | {Sym: "e"}, | |
| 454 | + | {Sym: "j"}, | |
| 455 | + | }, | |
| 456 | + | { | |
| 457 | + | {Sym: "e", Mods: ModShift}, | |
| 458 | + | {Sym: "s"}, | |
| 459 | + | {Sym: "c"}, | |
| 460 | + | {Sym: "j"}, | |
| 461 | + | }, | |
| 462 | + | { | |
| 463 | + | {Sym: "t", Mods: ModShift}, | |
| 464 | + | {Sym: "a"}, | |
| 465 | + | {Sym: "b"}, | |
| 466 | + | {Sym: "x"}, | |
| 467 | + | }, | |
| 468 | + | } | |
| 469 | + | for _, seq := range corpus { | |
| 470 | + | canonical := seq.String() | |
| 471 | + | t.Run(canonical, func(t *testing.T) { | |
| 472 | + | got, err := ParseBinding(canonical) | |
| 473 | + | if err != nil { | |
| 474 | + | t.Fatalf("ParseBinding(%q) error = %v", canonical, err) | |
| 475 | + | } | |
| 476 | + | if !got.Equal(seq) { | |
| 477 | + | t.Fatalf("ParseBinding(%q) = %v, want %v", canonical, got, seq) | |
| 478 | + | } | |
| 479 | + | }) | |
| 480 | + | } | |
| 481 | + | } | |
| @@ -241,11 +241,13 @@ func TestParseBinding_SpecialNamesInSequence(t *testing.T) { | |||||
| 241 | input string | 241 | input string | ||
| 242 | want Sequence | 242 | want Sequence | ||
| 243 | }{ | 243 | }{ | ||
| 244 | - | {input: "jEsc", want: Sequence{{Sym: "j"}, {Sym: "Esc"}}}, | 244 | + | {input: "j<Esc>", want: Sequence{{Sym: "j"}, {Sym: "Esc"}}}, |
| 245 | - | {input: "Spacej", want: Sequence{{Sym: "Space"}, {Sym: "j"}}}, | 245 | + | {input: "<Space>j", want: Sequence{{Sym: "Space"}, {Sym: "j"}}}, |
| 246 | - | {input: "CRj", want: Sequence{{Sym: "CR"}, {Sym: "j"}}}, | 246 | + | {input: "<CR>j", want: Sequence{{Sym: "CR"}, {Sym: "j"}}}, |
| 247 | - | {input: "ltj", want: Sequence{{Sym: "<"}, {Sym: "j"}}}, | 247 | + | {input: "<lt>j", want: Sequence{{Sym: "<"}, {Sym: "j"}}}, |
| 248 | - | {input: "Plusj", want: Sequence{{Sym: "+"}, {Sym: "j"}}}, | 248 | + | {input: "<Plus>j", want: Sequence{{Sym: "+"}, {Sym: "j"}}}, |
| 249 | + | {input: "j<Tab>", want: Sequence{{Sym: "j"}, {Sym: "Tab"}}}, | |||
| 250 | + | {input: "<BackSpace>j", want: Sequence{{Sym: "BackSpace"}, {Sym: "j"}}}, | |||
| 249 | } | 251 | } | ||
| 250 | for _, tt := range tests { | 252 | for _, tt := range tests { | ||
| 251 | t.Run(tt.input, func(t *testing.T) { | 253 | t.Run(tt.input, func(t *testing.T) { | ||
| @@ -260,6 +262,28 @@ func TestParseBinding_SpecialNamesInSequence(t *testing.T) { | |||||
| 260 | } | 262 | } | ||
| 261 | } | 263 | } | ||
| 262 | 264 | ||||
| 265 | + | func TestParseBinding_BareNamedPrefixIsLiteral(t *testing.T) { | |||
| 266 | + | // Named keys are canonical only in <...>; bare "Spacej" is Shift+s + p + a + c + e + j. | |||
| 267 | + | seq := Sequence{ | |||
| 268 | + | {Sym: "s", Mods: ModShift}, | |||
| 269 | + | {Sym: "p"}, | |||
| 270 | + | {Sym: "a"}, | |||
| 271 | + | {Sym: "c"}, | |||
| 272 | + | {Sym: "e"}, | |||
| 273 | + | {Sym: "j"}, | |||
| 274 | + | } | |||
| 275 | + | if got := seq.String(); got != "Spacej" { | |||
| 276 | + | t.Fatalf("seq.String() = %q, want %q", got, "Spacej") | |||
| 277 | + | } | |||
| 278 | + | got, err := ParseBinding("Spacej") | |||
| 279 | + | if err != nil { | |||
| 280 | + | t.Fatalf("ParseBinding(%q) error = %v", "Spacej", err) | |||
| 281 | + | } | |||
| 282 | + | if !got.Equal(seq) { | |||
| 283 | + | t.Fatalf("ParseBinding(%q) = %v, want %v", "Spacej", got, seq) | |||
| 284 | + | } | |||
| 285 | + | } | |||
| 286 | + | ||||
| 263 | func TestParseBinding_PrintableGlyphs(t *testing.T) { | 287 | func TestParseBinding_PrintableGlyphs(t *testing.T) { | ||
| 264 | got, err := ParseBinding("!@#") | 288 | got, err := ParseBinding("!@#") | ||
| 265 | if err != nil { | 289 | if err != nil { | ||
| @@ -356,6 +380,13 @@ func TestParseBinding_RoundTrip(t *testing.T) { | |||||
| 356 | "<C-d>j", | 380 | "<C-d>j", | ||
| 357 | "<Escape>j", | 381 | "<Escape>j", | ||
| 358 | "<C-S-a>", | 382 | "<C-S-a>", | ||
| 383 | + | "Spacej", | |||
| 384 | + | "Escj", | |||
| 385 | + | "Tabx", | |||
| 386 | + | "enter", | |||
| 387 | + | "escape", | |||
| 388 | + | "return", | |||
| 389 | + | "esc", | |||
| 359 | } | 390 | } | ||
| 360 | for _, input := range inputs { | 391 | for _, input := range inputs { | ||
| 361 | t.Run(input, func(t *testing.T) { | 392 | t.Run(input, func(t *testing.T) { | ||
| @@ -374,3 +405,77 @@ func TestParseBinding_RoundTrip(t *testing.T) { | |||||
| 374 | }) | 405 | }) | ||
| 375 | } | 406 | } | ||
| 376 | } | 407 | } | ||
| 408 | + | ||||
| 409 | + | // TestParseBinding_DeterministicRoundTripProperty asserts ParseBinding(seq.String()) == seq | |||
| 410 | + | // over a fixed representative set of printable, named, and modifier sequences. | |||
| 411 | + | // The corpus is deterministic (no flaky randomness). | |||
| 412 | + | func TestParseBinding_DeterministicRoundTripProperty(t *testing.T) { | |||
| 413 | + | corpus := []Sequence{ | |||
| 414 | + | {{Sym: "j"}}, | |||
| 415 | + | {{Sym: "j", Mods: ModShift}}, | |||
| 416 | + | {{Sym: "]"}, {Sym: "]"}}, | |||
| 417 | + | {{Sym: "y"}, {Sym: "a"}, {Sym: "h"}}, | |||
| 418 | + | {{Sym: "g"}, {Sym: "o", Mods: ModShift}}, | |||
| 419 | + | {{Sym: "d", Mods: ModCtrl}}, | |||
| 420 | + | {{Sym: "a", Mods: ModCtrl | ModShift}}, | |||
| 421 | + | {{Sym: "k", Mods: ModAlt}}, | |||
| 422 | + | {{Sym: "CR"}}, | |||
| 423 | + | {{Sym: "Esc"}}, | |||
| 424 | + | {{Sym: "Space"}}, | |||
| 425 | + | {{Sym: "Tab"}}, | |||
| 426 | + | {{Sym: "BackSpace"}}, | |||
| 427 | + | {{Sym: "Delete"}}, | |||
| 428 | + | {{Sym: "Left"}}, | |||
| 429 | + | {{Sym: "Right"}}, | |||
| 430 | + | {{Sym: "Up"}}, | |||
| 431 | + | {{Sym: "Down"}}, | |||
| 432 | + | {{Sym: "Home"}}, | |||
| 433 | + | {{Sym: "End"}}, | |||
| 434 | + | {{Sym: "PageUp"}}, | |||
| 435 | + | {{Sym: "PageDown"}}, | |||
| 436 | + | {{Sym: "Tab", Mods: ModCtrl}}, | |||
| 437 | + | {{Sym: "BackSpace", Mods: ModShift}}, | |||
| 438 | + | {{Sym: "Delete", Mods: ModAlt}}, | |||
| 439 | + | {{Sym: "<"}}, | |||
| 440 | + | {{Sym: "+"}}, | |||
| 441 | + | {{Sym: "!"}}, | |||
| 442 | + | {{Sym: "@"}}, | |||
| 443 | + | {{Sym: "#"}}, | |||
| 444 | + | {{Sym: "Space"}, {Sym: "j"}}, | |||
| 445 | + | {{Sym: "d", Mods: ModCtrl}, {Sym: "j"}}, | |||
| 446 | + | {{Sym: "Esc"}, {Sym: "j"}}, | |||
| 447 | + | // Literal printable run that formerly collided with greedy "Space" recognition. | |||
| 448 | + | { | |||
| 449 | + | {Sym: "s", Mods: ModShift}, | |||
| 450 | + | {Sym: "p"}, | |||
| 451 | + | {Sym: "a"}, | |||
| 452 | + | {Sym: "c"}, | |||
| 453 | + | {Sym: "e"}, | |||
| 454 | + | {Sym: "j"}, | |||
| 455 | + | }, | |||
| 456 | + | { | |||
| 457 | + | {Sym: "e", Mods: ModShift}, | |||
| 458 | + | {Sym: "s"}, | |||
| 459 | + | {Sym: "c"}, | |||
| 460 | + | {Sym: "j"}, | |||
| 461 | + | }, | |||
| 462 | + | { | |||
| 463 | + | {Sym: "t", Mods: ModShift}, | |||
| 464 | + | {Sym: "a"}, | |||
| 465 | + | {Sym: "b"}, | |||
| 466 | + | {Sym: "x"}, | |||
| 467 | + | }, | |||
| 468 | + | } | |||
| 469 | + | for _, seq := range corpus { | |||
| 470 | + | canonical := seq.String() | |||
| 471 | + | t.Run(canonical, func(t *testing.T) { | |||
| 472 | + | got, err := ParseBinding(canonical) | |||
| 473 | + | if err != nil { | |||
| 474 | + | t.Fatalf("ParseBinding(%q) error = %v", canonical, err) | |||
| 475 | + | } | |||
| 476 | + | if !got.Equal(seq) { | |||
| 477 | + | t.Fatalf("ParseBinding(%q) = %v, want %v", canonical, got, seq) | |||
| 478 | + | } | |||
| 479 | + | }) | |||
| 480 | + | } | |||
| 481 | + | } | |||
M internal/ui/input/vimkeys_dispatch.go +8 -2
| @@ -5,6 +5,7 @@ import ( | |||
| 5 | 5 | ||
| 6 | 6 | "github.com/bnema/dumber/internal/domain/entity" | |
| 7 | 7 | "github.com/bnema/dumber/internal/domain/vimkeys" | |
| 8 | + | "github.com/rs/zerolog/log" | |
| 8 | 9 | ) | |
| 9 | 10 | ||
| 10 | 11 | // trieOwnsBinding reports whether a raw config binding belongs to the Vim | |
| @@ -26,7 +27,7 @@ func trieOwnsBinding(raw string) (vimkeys.Sequence, bool) { | |||
| 26 | 27 | ||
| 27 | 28 | // buildVimModeTrie inserts trie-owned Vim mode bindings. Action names are | |
| 28 | 29 | // sorted so the first sorted name wins on Insert conflicts (validation already | |
| 29 | - | // rejects duplicates). | |
| 30 | + | // rejects duplicates). Ownership is marked only after a successful Insert. | |
| 30 | 31 | func buildVimModeTrie(cfg *entity.VimModeConfig) (*vimkeys.Trie, map[string]bool) { | |
| 31 | 32 | trie := vimkeys.NewTrie() | |
| 32 | 33 | owned := make(map[string]bool) | |
| @@ -49,10 +50,15 @@ func buildVimModeTrie(cfg *entity.VimModeConfig) (*vimkeys.Trie, map[string]bool | |||
| 49 | 50 | if !ok { | |
| 50 | 51 | continue | |
| 51 | 52 | } | |
| 52 | - | owned[key] = true | |
| 53 | 53 | if err := trie.Insert(seq, action); err != nil { | |
| 54 | + | log.Debug(). | |
| 55 | + | Err(err). | |
| 56 | + | Str("key", key). | |
| 57 | + | Str("action", action). | |
| 58 | + | Msg("page mode trie insert skipped") | |
| 54 | 59 | continue | |
| 55 | 60 | } | |
| 61 | + | owned[key] = true | |
| 56 | 62 | } | |
| 57 | 63 | } | |
| 58 | 64 | return trie, owned | |
| @@ -5,6 +5,7 @@ import ( | |||||
| 5 | 5 | ||||
| 6 | "github.com/bnema/dumber/internal/domain/entity" | 6 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 7 | "github.com/bnema/dumber/internal/domain/vimkeys" | 7 | "github.com/bnema/dumber/internal/domain/vimkeys" | ||
| 8 | + | "github.com/rs/zerolog/log" | |||
| 8 | ) | 9 | ) | ||
| 9 | 10 | ||||
| 10 | // trieOwnsBinding reports whether a raw config binding belongs to the Vim | 11 | // trieOwnsBinding reports whether a raw config binding belongs to the Vim | ||
| @@ -26,7 +27,7 @@ func trieOwnsBinding(raw string) (vimkeys.Sequence, bool) { | |||||
| 26 | 27 | ||||
| 27 | // buildVimModeTrie inserts trie-owned Vim mode bindings. Action names are | 28 | // buildVimModeTrie inserts trie-owned Vim mode bindings. Action names are | ||
| 28 | // sorted so the first sorted name wins on Insert conflicts (validation already | 29 | // sorted so the first sorted name wins on Insert conflicts (validation already | ||
| 29 | - | // rejects duplicates). | 30 | + | // rejects duplicates). Ownership is marked only after a successful Insert. |
| 30 | func buildVimModeTrie(cfg *entity.VimModeConfig) (*vimkeys.Trie, map[string]bool) { | 31 | func buildVimModeTrie(cfg *entity.VimModeConfig) (*vimkeys.Trie, map[string]bool) { | ||
| 31 | trie := vimkeys.NewTrie() | 32 | trie := vimkeys.NewTrie() | ||
| 32 | owned := make(map[string]bool) | 33 | owned := make(map[string]bool) | ||
| @@ -49,10 +50,15 @@ func buildVimModeTrie(cfg *entity.VimModeConfig) (*vimkeys.Trie, map[string]bool | |||||
| 49 | if !ok { | 50 | if !ok { | ||
| 50 | continue | 51 | continue | ||
| 51 | } | 52 | } | ||
| 52 | - | owned[key] = true | |||
| 53 | if err := trie.Insert(seq, action); err != nil { | 53 | if err := trie.Insert(seq, action); err != nil { | ||
| 54 | + | log.Debug(). | |||
| 55 | + | Err(err). | |||
| 56 | + | Str("key", key). | |||
| 57 | + | Str("action", action). | |||
| 58 | + | Msg("page mode trie insert skipped") | |||
| 54 | continue | 59 | continue | ||
| 55 | } | 60 | } | ||
| 61 | + | owned[key] = true | |||
| 56 | } | 62 | } | ||
| 57 | } | 63 | } | ||
| 58 | return trie, owned | 64 | return trie, owned | ||
M internal/ui/input/vimkeys_dispatch_test.go +29 -0
| @@ -121,6 +121,35 @@ func TestBuildPageModeTrie(t *testing.T) { | |||
| 121 | 121 | } | |
| 122 | 122 | } | |
| 123 | 123 | ||
| 124 | + | func TestBuildPageModeTrie_ConflictKeepsFirstSortedWinner(t *testing.T) { | |
| 125 | + | // Same sequence under two raw aliases; first sorted action wins Insert. | |
| 126 | + | // Losing raw key must not be marked owned (insert failed). | |
| 127 | + | cfg := &entity.PageModeConfig{ | |
| 128 | + | Actions: map[string]entity.ActionBinding{ | |
| 129 | + | "zzz-later": {Keys: []string{"C-d"}}, | |
| 130 | + | "aaa-first": {Keys: []string{"<C-d>"}}, | |
| 131 | + | "page-scroll-down": {Keys: []string{"j"}}, | |
| 132 | + | }, | |
| 133 | + | } | |
| 134 | + | ||
| 135 | + | trie, owned := buildPageModeTrie(cfg) | |
| 136 | + | if !owned["<C-d>"] { | |
| 137 | + | t.Fatal("winning raw key <C-d> should be owned after successful Insert") | |
| 138 | + | } | |
| 139 | + | if owned["C-d"] { | |
| 140 | + | t.Fatal("conflicting raw key C-d must not be marked owned when Insert fails") | |
| 141 | + | } | |
| 142 | + | ||
| 143 | + | seq, err := vimkeys.ParseBinding("<C-d>") | |
| 144 | + | if err != nil { | |
| 145 | + | t.Fatalf("ParseBinding(<C-d>) error = %v", err) | |
| 146 | + | } | |
| 147 | + | node, ok := trie.Walk(seq) | |
| 148 | + | if !ok || !node.Exact || node.Action != "aaa-first" { | |
| 149 | + | t.Fatalf("Walk(<C-d>) = (%#v, %v), want exact aaa-first", node, ok) | |
| 150 | + | } | |
| 151 | + | } | |
| 152 | + | ||
| 124 | 153 | func pageModeSequencesOwnershipFixture(t *testing.T) *ShortcutSet { | |
| 125 | 154 | t.Helper() | |
| 126 | 155 | ||
| @@ -121,6 +121,35 @@ func TestBuildPageModeTrie(t *testing.T) { | |||||
| 121 | } | 121 | } | ||
| 122 | } | 122 | } | ||
| 123 | 123 | ||||
| 124 | + | func TestBuildPageModeTrie_ConflictKeepsFirstSortedWinner(t *testing.T) { | |||
| 125 | + | // Same sequence under two raw aliases; first sorted action wins Insert. | |||
| 126 | + | // Losing raw key must not be marked owned (insert failed). | |||
| 127 | + | cfg := &entity.PageModeConfig{ | |||
| 128 | + | Actions: map[string]entity.ActionBinding{ | |||
| 129 | + | "zzz-later": {Keys: []string{"C-d"}}, | |||
| 130 | + | "aaa-first": {Keys: []string{"<C-d>"}}, | |||
| 131 | + | "page-scroll-down": {Keys: []string{"j"}}, | |||
| 132 | + | }, | |||
| 133 | + | } | |||
| 134 | + | ||||
| 135 | + | trie, owned := buildPageModeTrie(cfg) | |||
| 136 | + | if !owned["<C-d>"] { | |||
| 137 | + | t.Fatal("winning raw key <C-d> should be owned after successful Insert") | |||
| 138 | + | } | |||
| 139 | + | if owned["C-d"] { | |||
| 140 | + | t.Fatal("conflicting raw key C-d must not be marked owned when Insert fails") | |||
| 141 | + | } | |||
| 142 | + | ||||
| 143 | + | seq, err := vimkeys.ParseBinding("<C-d>") | |||
| 144 | + | if err != nil { | |||
| 145 | + | t.Fatalf("ParseBinding(<C-d>) error = %v", err) | |||
| 146 | + | } | |||
| 147 | + | node, ok := trie.Walk(seq) | |||
| 148 | + | if !ok || !node.Exact || node.Action != "aaa-first" { | |||
| 149 | + | t.Fatalf("Walk(<C-d>) = (%#v, %v), want exact aaa-first", node, ok) | |||
| 150 | + | } | |||
| 151 | + | } | |||
| 152 | + | ||||
| 124 | func pageModeSequencesOwnershipFixture(t *testing.T) *ShortcutSet { | 153 | func pageModeSequencesOwnershipFixture(t *testing.T) *ShortcutSet { | ||
| 125 | t.Helper() | 154 | t.Helper() | ||
| 126 | 155 | ||||