fix: address PR review - fix token double-counting and add migration tests
- commit
- 47613bde1f4095b5c31c59d3c5b6f1072ca43bcb
- author
- brice · · 2025-12-28 07:23 UTC
- parent
- 885eb5e
- signature
- unsigned
3 files · +290 -10
M internal/infrastructure/config/migrate.go +5 -3
| @@ -318,12 +318,14 @@ func keysAreSimilar(oldKey, newKey string) bool { | |||
| 318 | 318 | oldTokens := strings.Split(oldLeaf, "_") | |
| 319 | 319 | newTokens := strings.Split(newLeaf, "_") | |
| 320 | 320 | ||
| 321 | - | // Count matching tokens | |
| 321 | + | // Count matching tokens, ensuring each new token is matched at most once | |
| 322 | 322 | matches := 0 | |
| 323 | + | matchedNew := make([]bool, len(newTokens)) | |
| 323 | 324 | for _, ot := range oldTokens { | |
| 324 | - | for _, nt := range newTokens { | |
| 325 | - | if ot == nt { | |
| 325 | + | for j, nt := range newTokens { | |
| 326 | + | if ot == nt && !matchedNew[j] { | |
| 326 | 327 | matches++ | |
| 328 | + | matchedNew[j] = true | |
| 327 | 329 | break | |
| 328 | 330 | } | |
| 329 | 331 | } | |
| @@ -318,12 +318,14 @@ func keysAreSimilar(oldKey, newKey string) bool { | |||||
| 318 | oldTokens := strings.Split(oldLeaf, "_") | 318 | oldTokens := strings.Split(oldLeaf, "_") | ||
| 319 | newTokens := strings.Split(newLeaf, "_") | 319 | newTokens := strings.Split(newLeaf, "_") | ||
| 320 | 320 | ||||
| 321 | - | // Count matching tokens | 321 | + | // Count matching tokens, ensuring each new token is matched at most once |
| 322 | matches := 0 | 322 | matches := 0 | ||
| 323 | + | matchedNew := make([]bool, len(newTokens)) | |||
| 323 | for _, ot := range oldTokens { | 324 | for _, ot := range oldTokens { | ||
| 324 | - | for _, nt := range newTokens { | 325 | + | for j, nt := range newTokens { |
| 325 | - | if ot == nt { | 326 | + | if ot == nt && !matchedNew[j] { |
| 326 | matches++ | 327 | matches++ | ||
| 328 | + | matchedNew[j] = true | |||
| 327 | break | 329 | break | ||
| 328 | } | 330 | } | ||
| 329 | } | 331 | } | ||
M internal/infrastructure/config/migrate_test.go +277 -0
| @@ -340,6 +340,283 @@ func TestMigrator_FormatValue(t *testing.T) { | |||
| 340 | 340 | } | |
| 341 | 341 | } | |
| 342 | 342 | ||
| 343 | + | func TestKeysAreSimilar(t *testing.T) { | |
| 344 | + | tests := []struct { | |
| 345 | + | name string | |
| 346 | + | oldKey string | |
| 347 | + | newKey string | |
| 348 | + | expected bool | |
| 349 | + | }{ | |
| 350 | + | { | |
| 351 | + | name: "identical keys", | |
| 352 | + | oldKey: "section.pane_mode_color", | |
| 353 | + | newKey: "section.pane_mode_color", | |
| 354 | + | expected: true, | |
| 355 | + | }, | |
| 356 | + | { | |
| 357 | + | name: "substring match - old contains new", | |
| 358 | + | oldKey: "section.pane_mode_border_color", | |
| 359 | + | newKey: "section.pane_mode_color", | |
| 360 | + | expected: true, | |
| 361 | + | }, | |
| 362 | + | { | |
| 363 | + | name: "substring match - new contains old", | |
| 364 | + | oldKey: "section.mode_color", | |
| 365 | + | newKey: "section.pane_mode_color", | |
| 366 | + | expected: true, | |
| 367 | + | }, | |
| 368 | + | { | |
| 369 | + | name: "token matching - border removed", | |
| 370 | + | oldKey: "section.tab_mode_border_color", | |
| 371 | + | newKey: "section.tab_mode_color", | |
| 372 | + | expected: true, | |
| 373 | + | }, | |
| 374 | + | { | |
| 375 | + | name: "different parent paths", | |
| 376 | + | oldKey: "section_a.pane_mode_color", | |
| 377 | + | newKey: "section_b.pane_mode_color", | |
| 378 | + | expected: false, | |
| 379 | + | }, | |
| 380 | + | { | |
| 381 | + | name: "different depth", | |
| 382 | + | oldKey: "pane_mode_color", | |
| 383 | + | newKey: "section.pane_mode_color", | |
| 384 | + | expected: false, | |
| 385 | + | }, | |
| 386 | + | { | |
| 387 | + | name: "completely different keys", | |
| 388 | + | oldKey: "section.foo_bar", | |
| 389 | + | newKey: "section.baz_qux", | |
| 390 | + | expected: false, | |
| 391 | + | }, | |
| 392 | + | { | |
| 393 | + | name: "top level keys rejected", | |
| 394 | + | oldKey: "foo", | |
| 395 | + | newKey: "bar", | |
| 396 | + | expected: false, | |
| 397 | + | }, | |
| 398 | + | { | |
| 399 | + | name: "no common tokens", | |
| 400 | + | oldKey: "section.aaa_bbb", | |
| 401 | + | newKey: "section.ccc_ddd", | |
| 402 | + | expected: false, | |
| 403 | + | }, | |
| 404 | + | { | |
| 405 | + | name: "single token difference allowed", | |
| 406 | + | oldKey: "section.pane_mode_border_width", | |
| 407 | + | newKey: "section.mode_border_width", | |
| 408 | + | expected: true, | |
| 409 | + | }, | |
| 410 | + | { | |
| 411 | + | name: "duplicate tokens in old key - no double counting", | |
| 412 | + | oldKey: "section.mode_mode_mode", | |
| 413 | + | newKey: "section.mode_other_another", | |
| 414 | + | expected: false, // oldTokens=[mode,mode,mode], newTokens=[mode,other,another], matches=1 (only one mode can match), minTokens=3, needs 1>=2 which is false | |
| 415 | + | }, | |
| 416 | + | { | |
| 417 | + | name: "enough common tokens with duplicates", | |
| 418 | + | oldKey: "section.pane_mode_border", | |
| 419 | + | newKey: "section.pane_mode_color", | |
| 420 | + | expected: true, // oldTokens=[pane,mode,border], newTokens=[pane,mode,color], matches=2, minTokens=3, 2>=2 && 2>0 = true | |
| 421 | + | }, | |
| 422 | + | } | |
| 423 | + | ||
| 424 | + | for _, tt := range tests { | |
| 425 | + | t.Run(tt.name, func(t *testing.T) { | |
| 426 | + | result := keysAreSimilar(tt.oldKey, tt.newKey) | |
| 427 | + | assert.Equal(t, tt.expected, result, "keysAreSimilar(%q, %q)", tt.oldKey, tt.newKey) | |
| 428 | + | }) | |
| 429 | + | } | |
| 430 | + | } | |
| 431 | + | ||
| 432 | + | func TestMigrator_TypesAreCompatible(t *testing.T) { | |
| 433 | + | m := NewMigrator() | |
| 434 | + | ||
| 435 | + | tests := []struct { | |
| 436 | + | name string | |
| 437 | + | oldKey string | |
| 438 | + | newKey string | |
| 439 | + | expected bool | |
| 440 | + | }{ | |
| 441 | + | { | |
| 442 | + | name: "color to color - compatible (using real key)", | |
| 443 | + | oldKey: "workspace.styling.pane_mode_border_color", | |
| 444 | + | newKey: "workspace.styling.pane_mode_color", | |
| 445 | + | expected: true, | |
| 446 | + | }, | |
| 447 | + | { | |
| 448 | + | name: "width to width - compatible (using real key)", | |
| 449 | + | oldKey: "workspace.styling.pane_mode_border_width", | |
| 450 | + | newKey: "workspace.styling.mode_border_width", | |
| 451 | + | expected: true, | |
| 452 | + | }, | |
| 453 | + | { | |
| 454 | + | name: "width suffix to color type - incompatible", | |
| 455 | + | oldKey: "workspace.styling.some_width", | |
| 456 | + | newKey: "workspace.styling.pane_mode_color", // real key with string type | |
| 457 | + | expected: false, | |
| 458 | + | }, | |
| 459 | + | { | |
| 460 | + | name: "color suffix to int type - incompatible", | |
| 461 | + | oldKey: "workspace.styling.some_color", | |
| 462 | + | newKey: "workspace.styling.mode_border_width", // real key with int type | |
| 463 | + | expected: false, | |
| 464 | + | }, | |
| 465 | + | { | |
| 466 | + | name: "generic key to string type - compatible", | |
| 467 | + | oldKey: "workspace.styling.some_setting", | |
| 468 | + | newKey: "workspace.styling.pane_mode_color", // real key with string type | |
| 469 | + | expected: true, | |
| 470 | + | }, | |
| 471 | + | { | |
| 472 | + | name: "non-existent new key - incompatible", | |
| 473 | + | oldKey: "section.old_key", | |
| 474 | + | newKey: "section.nonexistent_key_xyz", | |
| 475 | + | expected: false, | |
| 476 | + | }, | |
| 477 | + | } | |
| 478 | + | ||
| 479 | + | for _, tt := range tests { | |
| 480 | + | t.Run(tt.name, func(t *testing.T) { | |
| 481 | + | result := m.typesAreCompatible(tt.oldKey, tt.newKey) | |
| 482 | + | assert.Equal(t, tt.expected, result, "typesAreCompatible(%q, %q)", tt.oldKey, tt.newKey) | |
| 483 | + | }) | |
| 484 | + | } | |
| 485 | + | } | |
| 486 | + | ||
| 487 | + | func TestMigrator_FindDeprecatedKeys(t *testing.T) { | |
| 488 | + | m := NewMigrator() | |
| 489 | + | ||
| 490 | + | tests := []struct { | |
| 491 | + | name string | |
| 492 | + | userKeys map[string]any | |
| 493 | + | defaultKeys map[string]bool | |
| 494 | + | expected []string | |
| 495 | + | }{ | |
| 496 | + | { | |
| 497 | + | name: "no deprecated keys", | |
| 498 | + | userKeys: map[string]any{ | |
| 499 | + | "history.max_entries": 1000, | |
| 500 | + | "logging.level": "info", | |
| 501 | + | }, | |
| 502 | + | defaultKeys: map[string]bool{ | |
| 503 | + | "history.max_entries": true, | |
| 504 | + | "logging.level": true, | |
| 505 | + | }, | |
| 506 | + | expected: []string{}, | |
| 507 | + | }, | |
| 508 | + | { | |
| 509 | + | name: "one deprecated key", | |
| 510 | + | userKeys: map[string]any{ | |
| 511 | + | "history.max_entries": 1000, | |
| 512 | + | "old.deprecated_key": "value", | |
| 513 | + | }, | |
| 514 | + | defaultKeys: map[string]bool{ | |
| 515 | + | "history.max_entries": true, | |
| 516 | + | }, | |
| 517 | + | expected: []string{"old.deprecated_key"}, | |
| 518 | + | }, | |
| 519 | + | { | |
| 520 | + | name: "multiple deprecated keys", | |
| 521 | + | userKeys: map[string]any{ | |
| 522 | + | "old.key1": "a", | |
| 523 | + | "old.key2": "b", | |
| 524 | + | "old.key3": "c", | |
| 525 | + | }, | |
| 526 | + | defaultKeys: map[string]bool{ | |
| 527 | + | "new.key1": true, | |
| 528 | + | }, | |
| 529 | + | expected: []string{"old.key1", "old.key2", "old.key3"}, | |
| 530 | + | }, | |
| 531 | + | { | |
| 532 | + | name: "parent key exists in defaults", | |
| 533 | + | userKeys: map[string]any{ | |
| 534 | + | "parent.child": "value", | |
| 535 | + | }, | |
| 536 | + | defaultKeys: map[string]bool{ | |
| 537 | + | "parent.child": true, | |
| 538 | + | "parent.other_child": true, | |
| 539 | + | }, | |
| 540 | + | expected: []string{}, | |
| 541 | + | }, | |
| 542 | + | } | |
| 543 | + | ||
| 544 | + | for _, tt := range tests { | |
| 545 | + | t.Run(tt.name, func(t *testing.T) { | |
| 546 | + | result := m.findDeprecatedKeys(tt.userKeys, tt.defaultKeys) | |
| 547 | + | assert.ElementsMatch(t, tt.expected, result) | |
| 548 | + | }) | |
| 549 | + | } | |
| 550 | + | } | |
| 551 | + | ||
| 552 | + | func TestMigrator_MatchRenamedKeys(t *testing.T) { | |
| 553 | + | m := NewMigrator() | |
| 554 | + | ||
| 555 | + | tests := []struct { | |
| 556 | + | name string | |
| 557 | + | deprecated []string | |
| 558 | + | missing []string | |
| 559 | + | expectedRenames map[string]string | |
| 560 | + | expectedUnmatchedOld []string | |
| 561 | + | expectedUnmatchedNew []string | |
| 562 | + | }{ | |
| 563 | + | { | |
| 564 | + | name: "no keys to match", | |
| 565 | + | deprecated: []string{}, | |
| 566 | + | missing: []string{}, | |
| 567 | + | expectedRenames: map[string]string{}, | |
| 568 | + | expectedUnmatchedOld: nil, | |
| 569 | + | expectedUnmatchedNew: nil, | |
| 570 | + | }, | |
| 571 | + | { | |
| 572 | + | name: "simple rename match (using real keys)", | |
| 573 | + | deprecated: []string{"workspace.styling.pane_mode_border_color"}, | |
| 574 | + | missing: []string{"workspace.styling.pane_mode_color"}, | |
| 575 | + | expectedRenames: map[string]string{ | |
| 576 | + | "workspace.styling.pane_mode_border_color": "workspace.styling.pane_mode_color", | |
| 577 | + | }, | |
| 578 | + | expectedUnmatchedOld: nil, | |
| 579 | + | expectedUnmatchedNew: nil, | |
| 580 | + | }, | |
| 581 | + | { | |
| 582 | + | name: "no match possible - different types (width vs color)", | |
| 583 | + | deprecated: []string{"workspace.styling.pane_mode_border_width"}, | |
| 584 | + | missing: []string{"workspace.styling.pane_mode_color"}, | |
| 585 | + | expectedRenames: map[string]string{}, | |
| 586 | + | expectedUnmatchedOld: []string{"workspace.styling.pane_mode_border_width"}, | |
| 587 | + | expectedUnmatchedNew: []string{"workspace.styling.pane_mode_color"}, | |
| 588 | + | }, | |
| 589 | + | { | |
| 590 | + | name: "no match possible - different parents", | |
| 591 | + | deprecated: []string{"old_section.some_key"}, | |
| 592 | + | missing: []string{"new_section.some_key"}, | |
| 593 | + | expectedRenames: map[string]string{}, | |
| 594 | + | expectedUnmatchedOld: []string{"old_section.some_key"}, | |
| 595 | + | expectedUnmatchedNew: []string{"new_section.some_key"}, | |
| 596 | + | }, | |
| 597 | + | { | |
| 598 | + | name: "multiple renames (using real keys)", | |
| 599 | + | deprecated: []string{"workspace.styling.pane_mode_border_color", "workspace.styling.tab_mode_border_color"}, | |
| 600 | + | missing: []string{"workspace.styling.pane_mode_color", "workspace.styling.tab_mode_color"}, | |
| 601 | + | expectedRenames: map[string]string{ | |
| 602 | + | "workspace.styling.pane_mode_border_color": "workspace.styling.pane_mode_color", | |
| 603 | + | "workspace.styling.tab_mode_border_color": "workspace.styling.tab_mode_color", | |
| 604 | + | }, | |
| 605 | + | expectedUnmatchedOld: nil, | |
| 606 | + | expectedUnmatchedNew: nil, | |
| 607 | + | }, | |
| 608 | + | } | |
| 609 | + | ||
| 610 | + | for _, tt := range tests { | |
| 611 | + | t.Run(tt.name, func(t *testing.T) { | |
| 612 | + | renames, unmatchedOld, unmatchedNew := m.matchRenamedKeys(tt.deprecated, tt.missing) | |
| 613 | + | assert.Equal(t, tt.expectedRenames, renames, "renames mismatch") | |
| 614 | + | assert.ElementsMatch(t, tt.expectedUnmatchedOld, unmatchedOld, "unmatched old mismatch") | |
| 615 | + | assert.ElementsMatch(t, tt.expectedUnmatchedNew, unmatchedNew, "unmatched new mismatch") | |
| 616 | + | }) | |
| 617 | + | } | |
| 618 | + | } | |
| 619 | + | ||
| 343 | 620 | func TestMigrator_CheckMigration_NoConfigFile(t *testing.T) { | |
| 344 | 621 | // This test would need to temporarily modify the config path | |
| 345 | 622 | // which is difficult without refactoring. Skip for now. | |
| @@ -340,6 +340,283 @@ func TestMigrator_FormatValue(t *testing.T) { | |||||
| 340 | } | 340 | } | ||
| 341 | } | 341 | } | ||
| 342 | 342 | ||||
| 343 | + | func TestKeysAreSimilar(t *testing.T) { | |||
| 344 | + | tests := []struct { | |||
| 345 | + | name string | |||
| 346 | + | oldKey string | |||
| 347 | + | newKey string | |||
| 348 | + | expected bool | |||
| 349 | + | }{ | |||
| 350 | + | { | |||
| 351 | + | name: "identical keys", | |||
| 352 | + | oldKey: "section.pane_mode_color", | |||
| 353 | + | newKey: "section.pane_mode_color", | |||
| 354 | + | expected: true, | |||
| 355 | + | }, | |||
| 356 | + | { | |||
| 357 | + | name: "substring match - old contains new", | |||
| 358 | + | oldKey: "section.pane_mode_border_color", | |||
| 359 | + | newKey: "section.pane_mode_color", | |||
| 360 | + | expected: true, | |||
| 361 | + | }, | |||
| 362 | + | { | |||
| 363 | + | name: "substring match - new contains old", | |||
| 364 | + | oldKey: "section.mode_color", | |||
| 365 | + | newKey: "section.pane_mode_color", | |||
| 366 | + | expected: true, | |||
| 367 | + | }, | |||
| 368 | + | { | |||
| 369 | + | name: "token matching - border removed", | |||
| 370 | + | oldKey: "section.tab_mode_border_color", | |||
| 371 | + | newKey: "section.tab_mode_color", | |||
| 372 | + | expected: true, | |||
| 373 | + | }, | |||
| 374 | + | { | |||
| 375 | + | name: "different parent paths", | |||
| 376 | + | oldKey: "section_a.pane_mode_color", | |||
| 377 | + | newKey: "section_b.pane_mode_color", | |||
| 378 | + | expected: false, | |||
| 379 | + | }, | |||
| 380 | + | { | |||
| 381 | + | name: "different depth", | |||
| 382 | + | oldKey: "pane_mode_color", | |||
| 383 | + | newKey: "section.pane_mode_color", | |||
| 384 | + | expected: false, | |||
| 385 | + | }, | |||
| 386 | + | { | |||
| 387 | + | name: "completely different keys", | |||
| 388 | + | oldKey: "section.foo_bar", | |||
| 389 | + | newKey: "section.baz_qux", | |||
| 390 | + | expected: false, | |||
| 391 | + | }, | |||
| 392 | + | { | |||
| 393 | + | name: "top level keys rejected", | |||
| 394 | + | oldKey: "foo", | |||
| 395 | + | newKey: "bar", | |||
| 396 | + | expected: false, | |||
| 397 | + | }, | |||
| 398 | + | { | |||
| 399 | + | name: "no common tokens", | |||
| 400 | + | oldKey: "section.aaa_bbb", | |||
| 401 | + | newKey: "section.ccc_ddd", | |||
| 402 | + | expected: false, | |||
| 403 | + | }, | |||
| 404 | + | { | |||
| 405 | + | name: "single token difference allowed", | |||
| 406 | + | oldKey: "section.pane_mode_border_width", | |||
| 407 | + | newKey: "section.mode_border_width", | |||
| 408 | + | expected: true, | |||
| 409 | + | }, | |||
| 410 | + | { | |||
| 411 | + | name: "duplicate tokens in old key - no double counting", | |||
| 412 | + | oldKey: "section.mode_mode_mode", | |||
| 413 | + | newKey: "section.mode_other_another", | |||
| 414 | + | expected: false, // oldTokens=[mode,mode,mode], newTokens=[mode,other,another], matches=1 (only one mode can match), minTokens=3, needs 1>=2 which is false | |||
| 415 | + | }, | |||
| 416 | + | { | |||
| 417 | + | name: "enough common tokens with duplicates", | |||
| 418 | + | oldKey: "section.pane_mode_border", | |||
| 419 | + | newKey: "section.pane_mode_color", | |||
| 420 | + | expected: true, // oldTokens=[pane,mode,border], newTokens=[pane,mode,color], matches=2, minTokens=3, 2>=2 && 2>0 = true | |||
| 421 | + | }, | |||
| 422 | + | } | |||
| 423 | + | ||||
| 424 | + | for _, tt := range tests { | |||
| 425 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 426 | + | result := keysAreSimilar(tt.oldKey, tt.newKey) | |||
| 427 | + | assert.Equal(t, tt.expected, result, "keysAreSimilar(%q, %q)", tt.oldKey, tt.newKey) | |||
| 428 | + | }) | |||
| 429 | + | } | |||
| 430 | + | } | |||
| 431 | + | ||||
| 432 | + | func TestMigrator_TypesAreCompatible(t *testing.T) { | |||
| 433 | + | m := NewMigrator() | |||
| 434 | + | ||||
| 435 | + | tests := []struct { | |||
| 436 | + | name string | |||
| 437 | + | oldKey string | |||
| 438 | + | newKey string | |||
| 439 | + | expected bool | |||
| 440 | + | }{ | |||
| 441 | + | { | |||
| 442 | + | name: "color to color - compatible (using real key)", | |||
| 443 | + | oldKey: "workspace.styling.pane_mode_border_color", | |||
| 444 | + | newKey: "workspace.styling.pane_mode_color", | |||
| 445 | + | expected: true, | |||
| 446 | + | }, | |||
| 447 | + | { | |||
| 448 | + | name: "width to width - compatible (using real key)", | |||
| 449 | + | oldKey: "workspace.styling.pane_mode_border_width", | |||
| 450 | + | newKey: "workspace.styling.mode_border_width", | |||
| 451 | + | expected: true, | |||
| 452 | + | }, | |||
| 453 | + | { | |||
| 454 | + | name: "width suffix to color type - incompatible", | |||
| 455 | + | oldKey: "workspace.styling.some_width", | |||
| 456 | + | newKey: "workspace.styling.pane_mode_color", // real key with string type | |||
| 457 | + | expected: false, | |||
| 458 | + | }, | |||
| 459 | + | { | |||
| 460 | + | name: "color suffix to int type - incompatible", | |||
| 461 | + | oldKey: "workspace.styling.some_color", | |||
| 462 | + | newKey: "workspace.styling.mode_border_width", // real key with int type | |||
| 463 | + | expected: false, | |||
| 464 | + | }, | |||
| 465 | + | { | |||
| 466 | + | name: "generic key to string type - compatible", | |||
| 467 | + | oldKey: "workspace.styling.some_setting", | |||
| 468 | + | newKey: "workspace.styling.pane_mode_color", // real key with string type | |||
| 469 | + | expected: true, | |||
| 470 | + | }, | |||
| 471 | + | { | |||
| 472 | + | name: "non-existent new key - incompatible", | |||
| 473 | + | oldKey: "section.old_key", | |||
| 474 | + | newKey: "section.nonexistent_key_xyz", | |||
| 475 | + | expected: false, | |||
| 476 | + | }, | |||
| 477 | + | } | |||
| 478 | + | ||||
| 479 | + | for _, tt := range tests { | |||
| 480 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 481 | + | result := m.typesAreCompatible(tt.oldKey, tt.newKey) | |||
| 482 | + | assert.Equal(t, tt.expected, result, "typesAreCompatible(%q, %q)", tt.oldKey, tt.newKey) | |||
| 483 | + | }) | |||
| 484 | + | } | |||
| 485 | + | } | |||
| 486 | + | ||||
| 487 | + | func TestMigrator_FindDeprecatedKeys(t *testing.T) { | |||
| 488 | + | m := NewMigrator() | |||
| 489 | + | ||||
| 490 | + | tests := []struct { | |||
| 491 | + | name string | |||
| 492 | + | userKeys map[string]any | |||
| 493 | + | defaultKeys map[string]bool | |||
| 494 | + | expected []string | |||
| 495 | + | }{ | |||
| 496 | + | { | |||
| 497 | + | name: "no deprecated keys", | |||
| 498 | + | userKeys: map[string]any{ | |||
| 499 | + | "history.max_entries": 1000, | |||
| 500 | + | "logging.level": "info", | |||
| 501 | + | }, | |||
| 502 | + | defaultKeys: map[string]bool{ | |||
| 503 | + | "history.max_entries": true, | |||
| 504 | + | "logging.level": true, | |||
| 505 | + | }, | |||
| 506 | + | expected: []string{}, | |||
| 507 | + | }, | |||
| 508 | + | { | |||
| 509 | + | name: "one deprecated key", | |||
| 510 | + | userKeys: map[string]any{ | |||
| 511 | + | "history.max_entries": 1000, | |||
| 512 | + | "old.deprecated_key": "value", | |||
| 513 | + | }, | |||
| 514 | + | defaultKeys: map[string]bool{ | |||
| 515 | + | "history.max_entries": true, | |||
| 516 | + | }, | |||
| 517 | + | expected: []string{"old.deprecated_key"}, | |||
| 518 | + | }, | |||
| 519 | + | { | |||
| 520 | + | name: "multiple deprecated keys", | |||
| 521 | + | userKeys: map[string]any{ | |||
| 522 | + | "old.key1": "a", | |||
| 523 | + | "old.key2": "b", | |||
| 524 | + | "old.key3": "c", | |||
| 525 | + | }, | |||
| 526 | + | defaultKeys: map[string]bool{ | |||
| 527 | + | "new.key1": true, | |||
| 528 | + | }, | |||
| 529 | + | expected: []string{"old.key1", "old.key2", "old.key3"}, | |||
| 530 | + | }, | |||
| 531 | + | { | |||
| 532 | + | name: "parent key exists in defaults", | |||
| 533 | + | userKeys: map[string]any{ | |||
| 534 | + | "parent.child": "value", | |||
| 535 | + | }, | |||
| 536 | + | defaultKeys: map[string]bool{ | |||
| 537 | + | "parent.child": true, | |||
| 538 | + | "parent.other_child": true, | |||
| 539 | + | }, | |||
| 540 | + | expected: []string{}, | |||
| 541 | + | }, | |||
| 542 | + | } | |||
| 543 | + | ||||
| 544 | + | for _, tt := range tests { | |||
| 545 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 546 | + | result := m.findDeprecatedKeys(tt.userKeys, tt.defaultKeys) | |||
| 547 | + | assert.ElementsMatch(t, tt.expected, result) | |||
| 548 | + | }) | |||
| 549 | + | } | |||
| 550 | + | } | |||
| 551 | + | ||||
| 552 | + | func TestMigrator_MatchRenamedKeys(t *testing.T) { | |||
| 553 | + | m := NewMigrator() | |||
| 554 | + | ||||
| 555 | + | tests := []struct { | |||
| 556 | + | name string | |||
| 557 | + | deprecated []string | |||
| 558 | + | missing []string | |||
| 559 | + | expectedRenames map[string]string | |||
| 560 | + | expectedUnmatchedOld []string | |||
| 561 | + | expectedUnmatchedNew []string | |||
| 562 | + | }{ | |||
| 563 | + | { | |||
| 564 | + | name: "no keys to match", | |||
| 565 | + | deprecated: []string{}, | |||
| 566 | + | missing: []string{}, | |||
| 567 | + | expectedRenames: map[string]string{}, | |||
| 568 | + | expectedUnmatchedOld: nil, | |||
| 569 | + | expectedUnmatchedNew: nil, | |||
| 570 | + | }, | |||
| 571 | + | { | |||
| 572 | + | name: "simple rename match (using real keys)", | |||
| 573 | + | deprecated: []string{"workspace.styling.pane_mode_border_color"}, | |||
| 574 | + | missing: []string{"workspace.styling.pane_mode_color"}, | |||
| 575 | + | expectedRenames: map[string]string{ | |||
| 576 | + | "workspace.styling.pane_mode_border_color": "workspace.styling.pane_mode_color", | |||
| 577 | + | }, | |||
| 578 | + | expectedUnmatchedOld: nil, | |||
| 579 | + | expectedUnmatchedNew: nil, | |||
| 580 | + | }, | |||
| 581 | + | { | |||
| 582 | + | name: "no match possible - different types (width vs color)", | |||
| 583 | + | deprecated: []string{"workspace.styling.pane_mode_border_width"}, | |||
| 584 | + | missing: []string{"workspace.styling.pane_mode_color"}, | |||
| 585 | + | expectedRenames: map[string]string{}, | |||
| 586 | + | expectedUnmatchedOld: []string{"workspace.styling.pane_mode_border_width"}, | |||
| 587 | + | expectedUnmatchedNew: []string{"workspace.styling.pane_mode_color"}, | |||
| 588 | + | }, | |||
| 589 | + | { | |||
| 590 | + | name: "no match possible - different parents", | |||
| 591 | + | deprecated: []string{"old_section.some_key"}, | |||
| 592 | + | missing: []string{"new_section.some_key"}, | |||
| 593 | + | expectedRenames: map[string]string{}, | |||
| 594 | + | expectedUnmatchedOld: []string{"old_section.some_key"}, | |||
| 595 | + | expectedUnmatchedNew: []string{"new_section.some_key"}, | |||
| 596 | + | }, | |||
| 597 | + | { | |||
| 598 | + | name: "multiple renames (using real keys)", | |||
| 599 | + | deprecated: []string{"workspace.styling.pane_mode_border_color", "workspace.styling.tab_mode_border_color"}, | |||
| 600 | + | missing: []string{"workspace.styling.pane_mode_color", "workspace.styling.tab_mode_color"}, | |||
| 601 | + | expectedRenames: map[string]string{ | |||
| 602 | + | "workspace.styling.pane_mode_border_color": "workspace.styling.pane_mode_color", | |||
| 603 | + | "workspace.styling.tab_mode_border_color": "workspace.styling.tab_mode_color", | |||
| 604 | + | }, | |||
| 605 | + | expectedUnmatchedOld: nil, | |||
| 606 | + | expectedUnmatchedNew: nil, | |||
| 607 | + | }, | |||
| 608 | + | } | |||
| 609 | + | ||||
| 610 | + | for _, tt := range tests { | |||
| 611 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 612 | + | renames, unmatchedOld, unmatchedNew := m.matchRenamedKeys(tt.deprecated, tt.missing) | |||
| 613 | + | assert.Equal(t, tt.expectedRenames, renames, "renames mismatch") | |||
| 614 | + | assert.ElementsMatch(t, tt.expectedUnmatchedOld, unmatchedOld, "unmatched old mismatch") | |||
| 615 | + | assert.ElementsMatch(t, tt.expectedUnmatchedNew, unmatchedNew, "unmatched new mismatch") | |||
| 616 | + | }) | |||
| 617 | + | } | |||
| 618 | + | } | |||
| 619 | + | ||||
| 343 | func TestMigrator_CheckMigration_NoConfigFile(t *testing.T) { | 620 | func TestMigrator_CheckMigration_NoConfigFile(t *testing.T) { | ||
| 344 | // This test would need to temporarily modify the config path | 621 | // This test would need to temporarily modify the config path | ||
| 345 | // which is difficult without refactoring. Skip for now. | 622 | // which is difficult without refactoring. Skip for now. | ||
M internal/ui/component/toaster.go +8 -7
| @@ -137,6 +137,7 @@ type Toaster struct { | |||
| 137 | 137 | hasCustomStyle bool | |
| 138 | 138 | customBgColor string // Current custom background color | |
| 139 | 139 | customTextColor string // Current custom text color | |
| 140 | + | customModeClass string // Current mode-specific CSS class (e.g., "toast-pane-mode") | |
| 140 | 141 | ||
| 141 | 142 | mu sync.Mutex | |
| 142 | 143 | } | |
| @@ -332,12 +333,12 @@ func (t *Toaster) applyCustomStyle(opts ToastOptions) { | |||
| 332 | 333 | func (t *Toaster) clearCustomStyle() { | |
| 333 | 334 | if t.hasCustomStyle { | |
| 334 | 335 | t.container.RemoveCssClass("toast-custom") | |
| 335 | - | // Remove any mode-specific classes | |
| 336 | - | t.container.RemoveCssClass("toast-pane-mode") | |
| 337 | - | t.container.RemoveCssClass("toast-tab-mode") | |
| 338 | - | t.container.RemoveCssClass("toast-session-mode") | |
| 339 | - | t.container.RemoveCssClass("toast-resize-mode") | |
| 340 | - | // Re-add level class if it was removed | |
| 336 | + | // Remove mode-specific class if one was applied | |
| 337 | + | if t.customModeClass != "" { | |
| 338 | + | t.container.RemoveCssClass(t.customModeClass) | |
| 339 | + | t.customModeClass = "" | |
| 340 | + | } | |
| 341 | + | // Re-add level class if it was removed due to custom background | |
| 341 | 342 | if t.customBgColor != "" { | |
| 342 | 343 | t.container.AddCssClass(t.currentLevel.cssClass()) | |
| 343 | 344 | } | |
| @@ -360,7 +361,7 @@ func (t *Toaster) ApplyModeClass(modeClass string) { | |||
| 360 | 361 | t.container.RemoveCssClass(t.currentLevel.cssClass()) | |
| 361 | 362 | t.container.AddCssClass(modeClass) | |
| 362 | 363 | t.hasCustomStyle = true | |
| 363 | - | t.customBgColor = modeClass // Store class name for cleanup tracking | |
| 364 | + | t.customModeClass = modeClass // Store class name for cleanup tracking | |
| 364 | 365 | } | |
| 365 | 366 | ||
| 366 | 367 | // startDismissTimerWithDuration starts the auto-dismiss timer with the given duration. | |
| @@ -137,6 +137,7 @@ type Toaster struct { | |||||
| 137 | hasCustomStyle bool | 137 | hasCustomStyle bool | ||
| 138 | customBgColor string // Current custom background color | 138 | customBgColor string // Current custom background color | ||
| 139 | customTextColor string // Current custom text color | 139 | customTextColor string // Current custom text color | ||
| 140 | + | customModeClass string // Current mode-specific CSS class (e.g., "toast-pane-mode") | |||
| 140 | 141 | ||||
| 141 | mu sync.Mutex | 142 | mu sync.Mutex | ||
| 142 | } | 143 | } | ||
| @@ -332,12 +333,12 @@ func (t *Toaster) applyCustomStyle(opts ToastOptions) { | |||||
| 332 | func (t *Toaster) clearCustomStyle() { | 333 | func (t *Toaster) clearCustomStyle() { | ||
| 333 | if t.hasCustomStyle { | 334 | if t.hasCustomStyle { | ||
| 334 | t.container.RemoveCssClass("toast-custom") | 335 | t.container.RemoveCssClass("toast-custom") | ||
| 335 | - | // Remove any mode-specific classes | 336 | + | // Remove mode-specific class if one was applied |
| 336 | - | t.container.RemoveCssClass("toast-pane-mode") | 337 | + | if t.customModeClass != "" { |
| 337 | - | t.container.RemoveCssClass("toast-tab-mode") | 338 | + | t.container.RemoveCssClass(t.customModeClass) |
| 338 | - | t.container.RemoveCssClass("toast-session-mode") | 339 | + | t.customModeClass = "" |
| 339 | - | t.container.RemoveCssClass("toast-resize-mode") | 340 | + | } |
| 340 | - | // Re-add level class if it was removed | 341 | + | // Re-add level class if it was removed due to custom background |
| 341 | if t.customBgColor != "" { | 342 | if t.customBgColor != "" { | ||
| 342 | t.container.AddCssClass(t.currentLevel.cssClass()) | 343 | t.container.AddCssClass(t.currentLevel.cssClass()) | ||
| 343 | } | 344 | } | ||
| @@ -360,7 +361,7 @@ func (t *Toaster) ApplyModeClass(modeClass string) { | |||||
| 360 | t.container.RemoveCssClass(t.currentLevel.cssClass()) | 361 | t.container.RemoveCssClass(t.currentLevel.cssClass()) | ||
| 361 | t.container.AddCssClass(modeClass) | 362 | t.container.AddCssClass(modeClass) | ||
| 362 | t.hasCustomStyle = true | 363 | t.hasCustomStyle = true | ||
| 363 | - | t.customBgColor = modeClass // Store class name for cleanup tracking | 364 | + | t.customModeClass = modeClass // Store class name for cleanup tracking |
| 364 | } | 365 | } | ||
| 365 | 366 | ||||
| 366 | // startDismissTimerWithDuration starts the auto-dismiss timer with the given duration. | 367 | // startDismissTimerWithDuration starts the auto-dismiss timer with the given duration. | ||