Skip to content

Commit 13c008a

Browse files
author
Darren.Zeng
authored
fix(config): support Chinese comma separator in allow_from environment variables (sipeed#1301)
Add UnmarshalText method to FlexibleStringSlice to support both English (,) and Chinese (,) comma separators in environment variables. Includes comprehensive unit tests covering: - English commas, Chinese commas, mixed commas - Single values, whitespace trimming - Empty strings, edge cases Fixes sipeed#1280
1 parent 52ef2df commit 13c008a

2 files changed

Lines changed: 142 additions & 0 deletions

File tree

pkg/config/config.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ var rrCounter atomic.Uint64
1717

1818
// FlexibleStringSlice is a []string that also accepts JSON numbers,
1919
// so allow_from can contain both "123" and 123.
20+
// It also supports parsing comma-separated strings from environment variables,
21+
// including both English (,) and Chinese (,) commas.
2022
type FlexibleStringSlice []string
2123

2224
func (f *FlexibleStringSlice) UnmarshalJSON(data []byte) error {
@@ -48,6 +50,30 @@ func (f *FlexibleStringSlice) UnmarshalJSON(data []byte) error {
4850
return nil
4951
}
5052

53+
// UnmarshalText implements encoding.TextUnmarshaler to support env variable parsing.
54+
// It handles comma-separated values with both English (,) and Chinese (,) commas.
55+
func (f *FlexibleStringSlice) UnmarshalText(text []byte) error {
56+
if len(text) == 0 {
57+
*f = nil
58+
return nil
59+
}
60+
61+
s := string(text)
62+
// Replace Chinese comma with English comma, then split
63+
s = strings.ReplaceAll(s, ",", ",")
64+
parts := strings.Split(s, ",")
65+
66+
result := make([]string, 0, len(parts))
67+
for _, part := range parts {
68+
part = strings.TrimSpace(part)
69+
if part != "" {
70+
result = append(result, part)
71+
}
72+
}
73+
*f = result
74+
return nil
75+
}
76+
5177
type Config struct {
5278
Agents AgentsConfig `json:"agents"`
5379
Bindings []AgentBinding `json:"bindings,omitempty"`

pkg/config/config_test.go

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -505,3 +505,119 @@ func TestDefaultConfig_WorkspacePath_WithPicoclawHome(t *testing.T) {
505505
t.Errorf("Workspace path with PICOCLAW_HOME = %q, want %q", cfg.Agents.Defaults.Workspace, want)
506506
}
507507
}
508+
509+
// TestFlexibleStringSlice_UnmarshalText tests UnmarshalText with various comma separators
510+
func TestFlexibleStringSlice_UnmarshalText(t *testing.T) {
511+
tests := []struct {
512+
name string
513+
input string
514+
expected []string
515+
}{
516+
{
517+
name: "English commas only",
518+
input: "123,456,789",
519+
expected: []string{"123", "456", "789"},
520+
},
521+
{
522+
name: "Chinese commas only",
523+
input: "123,456,789",
524+
expected: []string{"123", "456", "789"},
525+
},
526+
{
527+
name: "Mixed English and Chinese commas",
528+
input: "123,456,789",
529+
expected: []string{"123", "456", "789"},
530+
},
531+
{
532+
name: "Single value",
533+
input: "123",
534+
expected: []string{"123"},
535+
},
536+
{
537+
name: "Values with whitespace",
538+
input: " 123 , 456 , 789 ",
539+
expected: []string{"123", "456", "789"},
540+
},
541+
{
542+
name: "Empty string",
543+
input: "",
544+
expected: nil,
545+
},
546+
{
547+
name: "Only commas - English",
548+
input: ",,",
549+
expected: []string{},
550+
},
551+
{
552+
name: "Only commas - Chinese",
553+
input: ",,",
554+
expected: []string{},
555+
},
556+
{
557+
name: "Mixed commas with empty parts",
558+
input: "123,,456,,789",
559+
expected: []string{"123", "456", "789"},
560+
},
561+
{
562+
name: "Complex mixed values",
563+
564+
565+
},
566+
}
567+
568+
for _, tt := range tests {
569+
t.Run(tt.name, func(t *testing.T) {
570+
var f FlexibleStringSlice
571+
err := f.UnmarshalText([]byte(tt.input))
572+
if err != nil {
573+
t.Fatalf("UnmarshalText(%q) error = %v", tt.input, err)
574+
}
575+
576+
if tt.expected == nil {
577+
if f != nil {
578+
t.Errorf("UnmarshalText(%q) = %v, want nil", tt.input, f)
579+
}
580+
return
581+
}
582+
583+
if len(f) != len(tt.expected) {
584+
t.Errorf("UnmarshalText(%q) length = %d, want %d", tt.input, len(f), len(tt.expected))
585+
return
586+
}
587+
588+
for i, v := range tt.expected {
589+
if f[i] != v {
590+
t.Errorf("UnmarshalText(%q)[%d] = %q, want %q", tt.input, i, f[i], v)
591+
}
592+
}
593+
})
594+
}
595+
}
596+
597+
// TestFlexibleStringSlice_UnmarshalText_EmptySliceConsistency tests nil vs empty slice behavior
598+
func TestFlexibleStringSlice_UnmarshalText_EmptySliceConsistency(t *testing.T) {
599+
t.Run("Empty string returns nil", func(t *testing.T) {
600+
var f FlexibleStringSlice
601+
err := f.UnmarshalText([]byte(""))
602+
if err != nil {
603+
t.Fatalf("UnmarshalText error = %v", err)
604+
}
605+
if f != nil {
606+
t.Errorf("Empty string should return nil, got %v", f)
607+
}
608+
})
609+
610+
t.Run("Commas only returns empty slice", func(t *testing.T) {
611+
var f FlexibleStringSlice
612+
err := f.UnmarshalText([]byte(",,,"))
613+
if err != nil {
614+
t.Fatalf("UnmarshalText error = %v", err)
615+
}
616+
if f == nil {
617+
t.Error("Commas only should return empty slice, not nil")
618+
}
619+
if len(f) != 0 {
620+
t.Errorf("Expected empty slice, got %v", f)
621+
}
622+
})
623+
}

0 commit comments

Comments
 (0)