Skip to content

Commit 0914121

Browse files
committed
Fixing number color issue
1 parent 4d620bf commit 0914121

File tree

3 files changed

+124
-3
lines changed

3 files changed

+124
-3
lines changed

agents.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
- run ./scripts/format.sh then ./scripts/check.sh to format, then validate linting and spelling
55
- Add comprehensive tests to cover the changes
66
- Run test suite to ensure there is no regression
7-
7+
- Use UK english spelling (e.g. Colorisation not Colorization)
88

99
**DON'T:**
1010
- Git add or commit

pkg/yqlib/encoder_toml.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -611,8 +611,16 @@ func (te *tomlEncoder) colorizeToml(input []byte) []byte {
611611
if ch == '-' {
612612
end++
613613
}
614-
for end < len(toml) && ((toml[end] >= '0' && toml[end] <= '9') || toml[end] == '.' || toml[end] == 'e' || toml[end] == 'E' || toml[end] == '+' || toml[end] == '-') {
615-
end++
614+
for end < len(toml) {
615+
c := toml[end]
616+
if (c >= '0' && c <= '9') || c == '.' || c == 'e' || c == 'E' {
617+
end++
618+
} else if (c == '+' || c == '-') && end > 0 && (toml[end-1] == 'e' || toml[end-1] == 'E') {
619+
// Only allow + or - immediately after 'e' or 'E' for scientific notation
620+
end++
621+
} else {
622+
break
623+
}
616624
}
617625
result.WriteString(numberColor(toml[i:end]))
618626
i = end

pkg/yqlib/toml_test.go

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ package yqlib
33
import (
44
"bufio"
55
"fmt"
6+
"strings"
67
"testing"
78

9+
"github.com/fatih/color"
810
"github.com/mikefarah/yq/v4/test"
911
)
1012

@@ -625,3 +627,114 @@ func TestTomlScenarios(t *testing.T) {
625627
}
626628
documentScenarios(t, "usage", "toml", genericScenarios, documentTomlScenario)
627629
}
630+
631+
func TestTomlColorisationNumberBug(t *testing.T) {
632+
// Save and restore color state
633+
oldNoColor := color.NoColor
634+
color.NoColor = false
635+
defer func() { color.NoColor = oldNoColor }()
636+
637+
encoder := NewTomlEncoder()
638+
tomlEncoder := encoder.(*tomlEncoder)
639+
640+
// Test case that exposes the bug: "123-+-45" should NOT be colorized as a single number
641+
input := "A = 123-+-45\n"
642+
result := string(tomlEncoder.colorizeToml([]byte(input)))
643+
644+
// The bug causes "123-+-45" to be colorized as one token
645+
// It should stop at "123" because the next character '-' is not valid in this position
646+
if strings.Contains(result, "123-+-45") {
647+
// Check if it's colorized as a single token (no color codes in the middle)
648+
idx := strings.Index(result, "123-+-45")
649+
// Look backwards for color code
650+
beforeIdx := idx - 1
651+
for beforeIdx >= 0 && result[beforeIdx] != '\x1b' {
652+
beforeIdx--
653+
}
654+
// Look forward for reset code
655+
afterIdx := idx + 8 // length of "123-+-45"
656+
hasResetAfter := false
657+
for afterIdx < len(result) && afterIdx < idx+20 {
658+
if result[afterIdx] == '\x1b' {
659+
hasResetAfter = true
660+
break
661+
}
662+
afterIdx++
663+
}
664+
665+
if beforeIdx >= 0 && hasResetAfter {
666+
// The entire "123-+-45" is wrapped in color codes - this is the bug!
667+
t.Errorf("BUG DETECTED: '123-+-45' is incorrectly colorized as a single number")
668+
t.Errorf("Expected only '123' to be colorized as a number, but got the entire '123-+-45'")
669+
t.Logf("Full output: %q", result)
670+
t.Fail()
671+
}
672+
}
673+
674+
// Additional test cases for the bug
675+
bugTests := []struct {
676+
name string
677+
input string
678+
invalidSequence string
679+
description string
680+
}{
681+
{
682+
name: "consecutive minuses",
683+
input: "A = 123--45\n",
684+
invalidSequence: "123--45",
685+
description: "'123--45' should not be colorized as a single number",
686+
},
687+
{
688+
name: "plus in middle",
689+
input: "A = 123+45\n",
690+
invalidSequence: "123+45",
691+
description: "'123+45' should not be colorized as a single number",
692+
},
693+
}
694+
695+
for _, tt := range bugTests {
696+
t.Run(tt.name, func(t *testing.T) {
697+
result := string(tomlEncoder.colorizeToml([]byte(tt.input)))
698+
if strings.Contains(result, tt.invalidSequence) {
699+
idx := strings.Index(result, tt.invalidSequence)
700+
beforeIdx := idx - 1
701+
for beforeIdx >= 0 && result[beforeIdx] != '\x1b' {
702+
beforeIdx--
703+
}
704+
afterIdx := idx + len(tt.invalidSequence)
705+
hasResetAfter := false
706+
for afterIdx < len(result) && afterIdx < idx+20 {
707+
if result[afterIdx] == '\x1b' {
708+
hasResetAfter = true
709+
break
710+
}
711+
afterIdx++
712+
}
713+
714+
if beforeIdx >= 0 && hasResetAfter {
715+
t.Errorf("BUG: %s", tt.description)
716+
t.Logf("Full output: %q", result)
717+
}
718+
}
719+
})
720+
}
721+
722+
// Test that valid scientific notation still works
723+
validTests := []struct {
724+
name string
725+
input string
726+
}{
727+
{"scientific positive", "A = 1.23e+45\n"},
728+
{"scientific negative", "A = 6.626e-34\n"},
729+
{"scientific uppercase", "A = 1.23E+10\n"},
730+
}
731+
732+
for _, tt := range validTests {
733+
t.Run(tt.name, func(t *testing.T) {
734+
result := tomlEncoder.colorizeToml([]byte(tt.input))
735+
if len(result) == 0 {
736+
t.Error("Expected non-empty colorized output")
737+
}
738+
})
739+
}
740+
}

0 commit comments

Comments
 (0)