6f77c42 fix: address copilot review (logger context, immutable chains) · dumber

M internal/infrastructure/config/loader.go +8 -4


@@ -11,6 +11,7 @@ import (
11	11
12	12		"github.com/bnema/dumber/internal/application/port"
13	13		"github.com/bnema/dumber/internal/infrastructure/fonts"
	14	+	"github.com/bnema/dumber/internal/logging"
14	15		"github.com/spf13/viper"
15	16		)
16	17

@@ -322,7 +323,10 @@ func (m *Manager) createDefaultConfig() error {
322	323		// detectAndSetFonts detects available system fonts and sets the best available
323	324		// fonts from the fallback chains. This runs only during first-run config creation.
324	325		func (m *Manager) detectAndSetFonts() {
325		-	ctx := context.Background()
	326	+	// Create context with logger for debugging first-run font detection.
	327	+	logger := logging.NewFromEnv()
	328	+	ctx := logging.WithContext(context.Background(), logger)
	329	+
326	330		detector := fonts.NewDetector()
327	331
328	332		if !detector.IsAvailable(ctx) {

@@ -330,9 +334,9 @@ func (m *Manager) detectAndSetFonts() {
330	334		return
331	335		}
332	336
333		-	sansFont := detector.SelectBestFont(ctx, port.FontCategorySansSerif, fonts.SansSerifFallbackChain)
334		-	serifFont := detector.SelectBestFont(ctx, port.FontCategorySerif, fonts.SerifFallbackChain)
335		-	monoFont := detector.SelectBestFont(ctx, port.FontCategoryMonospace, fonts.MonospaceFallbackChain)
	337	+	sansFont := detector.SelectBestFont(ctx, port.FontCategorySansSerif, fonts.SansSerifFallbackChain())
	338	+	serifFont := detector.SelectBestFont(ctx, port.FontCategorySerif, fonts.SerifFallbackChain())
	339	+	monoFont := detector.SelectBestFont(ctx, port.FontCategoryMonospace, fonts.MonospaceFallbackChain())
336	340
337	341		m.viper.Set("appearance.sans_font", sansFont)
338	342		m.viper.Set("appearance.serif_font", serifFont)

 	"github.com/bnema/dumber/internal/application/port"
 	"github.com/bnema/dumber/internal/infrastructure/fonts"
+	"github.com/bnema/dumber/internal/logging"
 	"github.com/spf13/viper"
 )
 // detectAndSetFonts detects available system fonts and sets the best available
 // fonts from the fallback chains. This runs only during first-run config creation.
 func (m *Manager) detectAndSetFonts() {
-	ctx := context.Background()
+	// Create context with logger for debugging first-run font detection.
+	logger := logging.NewFromEnv()
+	ctx := logging.WithContext(context.Background(), logger)
 	detector := fonts.NewDetector()
 	if !detector.IsAvailable(ctx) {
 		return
 	}
-	sansFont := detector.SelectBestFont(ctx, port.FontCategorySansSerif, fonts.SansSerifFallbackChain)
+	sansFont := detector.SelectBestFont(ctx, port.FontCategorySansSerif, fonts.SansSerifFallbackChain())
-	serifFont := detector.SelectBestFont(ctx, port.FontCategorySerif, fonts.SerifFallbackChain)
+	serifFont := detector.SelectBestFont(ctx, port.FontCategorySerif, fonts.SerifFallbackChain())
-	monoFont := detector.SelectBestFont(ctx, port.FontCategoryMonospace, fonts.MonospaceFallbackChain)
+	monoFont := detector.SelectBestFont(ctx, port.FontCategoryMonospace, fonts.MonospaceFallbackChain())
 	m.viper.Set("appearance.sans_font", sansFont)
 	m.viper.Set("appearance.serif_font", serifFont)

M internal/infrastructure/fonts/detector.go +25 -5


@@ -11,10 +11,9 @@ import (
11	11		"github.com/bnema/dumber/internal/logging"
12	12		)
13	13
14		-	// Fallback chains for each font category.
15		-	// The first available font in the chain will be selected.
	14	+	// Fallback chains for each font category (unexported to prevent modification).
16	15		var (
17		-	SansSerifFallbackChain = []string{
	16	+	sansSerifFallbackChain = []string{
18	17		"Fira Sans",
19	18		"Noto Sans",
20	19		"DejaVu Sans",

@@ -22,14 +21,14 @@ var (
22	21		"FreeSans",
23	22		}
24	23
25		-	SerifFallbackChain = []string{
	24	+	serifFallbackChain = []string{
26	25		"Noto Serif",
27	26		"DejaVu Serif",
28	27		"Liberation Serif",
29	28		"FreeSerif",
30	29		}
31	30
32		-	MonospaceFallbackChain = []string{
	31	+	monospaceFallbackChain = []string{
33	32		"Fira Code",
34	33		"JetBrains Mono",
35	34		"Noto Sans Mono",

@@ -39,6 +38,27 @@ var (
39	38		}
40	39		)
41	40
	41	+	// SansSerifFallbackChain returns the fallback chain for sans-serif fonts.
	42	+	func SansSerifFallbackChain() []string {
	43	+	result := make([]string, len(sansSerifFallbackChain))
	44	+	copy(result, sansSerifFallbackChain)
	45	+	return result
	46	+	}
	47	+
	48	+	// SerifFallbackChain returns the fallback chain for serif fonts.
	49	+	func SerifFallbackChain() []string {
	50	+	result := make([]string, len(serifFallbackChain))
	51	+	copy(result, serifFallbackChain)
	52	+	return result
	53	+	}
	54	+
	55	+	// MonospaceFallbackChain returns the fallback chain for monospace fonts.
	56	+	func MonospaceFallbackChain() []string {
	57	+	result := make([]string, len(monospaceFallbackChain))
	58	+	copy(result, monospaceFallbackChain)
	59	+	return result
	60	+	}
	61	+
42	62		// Generic CSS fallbacks when no fonts from the chain are found.
43	63		const (
44	64		genericSansSerif = "sans-serif"

 	"github.com/bnema/dumber/internal/logging"
 )
-// Fallback chains for each font category.
+// Fallback chains for each font category (unexported to prevent modification).
-// The first available font in the chain will be selected.
 var (
-	SansSerifFallbackChain = []string{
+	sansSerifFallbackChain = []string{
 		"Fira Sans",
 		"Noto Sans",
 		"DejaVu Sans",
 		"FreeSans",
 	}
-	SerifFallbackChain = []string{
+	serifFallbackChain = []string{
 		"Noto Serif",
 		"DejaVu Serif",
 		"Liberation Serif",
 		"FreeSerif",
 	}
-	MonospaceFallbackChain = []string{
+	monospaceFallbackChain = []string{
 		"Fira Code",
 		"JetBrains Mono",
 		"Noto Sans Mono",
 	}
 )
+// SansSerifFallbackChain returns the fallback chain for sans-serif fonts.
+func SansSerifFallbackChain() []string {
+	result := make([]string, len(sansSerifFallbackChain))
+	copy(result, sansSerifFallbackChain)
+	return result
+}
+// SerifFallbackChain returns the fallback chain for serif fonts.
+func SerifFallbackChain() []string {
+	result := make([]string, len(serifFallbackChain))
+	copy(result, serifFallbackChain)
+	return result
+}
+// MonospaceFallbackChain returns the fallback chain for monospace fonts.
+func MonospaceFallbackChain() []string {
+	result := make([]string, len(monospaceFallbackChain))
+	copy(result, monospaceFallbackChain)
+	return result
+}
 // Generic CSS fallbacks when no fonts from the chain are found.
 const (
 	genericSansSerif = "sans-serif"

M internal/infrastructure/fonts/detector_test.go +12 -12


@@ -67,12 +67,12 @@ func TestDetector_SelectBestFont_SansSerif(t *testing.T) {
67	67		t.Skip("fc-list not available on this system")
68	68		}
69	69
70		-	font := detector.SelectBestFont(ctx, port.FontCategorySansSerif, SansSerifFallbackChain)
	70	+	font := detector.SelectBestFont(ctx, port.FontCategorySansSerif, SansSerifFallbackChain())
71	71		assert.NotEmpty(t, font)
72	72
73	73		// Should either be a font from the chain or the generic fallback
74	74		isFromChain := false
75		-	for _, f := range SansSerifFallbackChain {
	75	+	for _, f := range SansSerifFallbackChain() {
76	76		if font == f {
77	77		isFromChain = true
78	78		break

@@ -93,11 +93,11 @@ func TestDetector_SelectBestFont_Serif(t *testing.T) {
93	93		t.Skip("fc-list not available on this system")
94	94		}
95	95
96		-	font := detector.SelectBestFont(ctx, port.FontCategorySerif, SerifFallbackChain)
	96	+	font := detector.SelectBestFont(ctx, port.FontCategorySerif, SerifFallbackChain())
97	97		assert.NotEmpty(t, font)
98	98
99	99		isFromChain := false
100		-	for _, f := range SerifFallbackChain {
	100	+	for _, f := range SerifFallbackChain() {
101	101		if font == f {
102	102		isFromChain = true
103	103		break

@@ -118,11 +118,11 @@ func TestDetector_SelectBestFont_Monospace(t *testing.T) {
118	118		t.Skip("fc-list not available on this system")
119	119		}
120	120
121		-	font := detector.SelectBestFont(ctx, port.FontCategoryMonospace, MonospaceFallbackChain)
	121	+	font := detector.SelectBestFont(ctx, port.FontCategoryMonospace, MonospaceFallbackChain())
122	122		assert.NotEmpty(t, font)
123	123
124	124		isFromChain := false
125		-	for _, f := range MonospaceFallbackChain {
	125	+	for _, f := range MonospaceFallbackChain() {
126	126		if font == f {
127	127		isFromChain = true
128	128		break

@@ -194,14 +194,14 @@ func TestDetector_genericFallback(t *testing.T) {
194	194		}
195	195
196	196		func TestFallbackChains_NotEmpty(t *testing.T) {
197		-	assert.NotEmpty(t, SansSerifFallbackChain, "sans-serif fallback chain should not be empty")
198		-	assert.NotEmpty(t, SerifFallbackChain, "serif fallback chain should not be empty")
199		-	assert.NotEmpty(t, MonospaceFallbackChain, "monospace fallback chain should not be empty")
	197	+	assert.NotEmpty(t, SansSerifFallbackChain(), "sans-serif fallback chain should not be empty")
	198	+	assert.NotEmpty(t, SerifFallbackChain(), "serif fallback chain should not be empty")
	199	+	assert.NotEmpty(t, MonospaceFallbackChain(), "monospace fallback chain should not be empty")
200	200		}
201	201
202	202		func TestFallbackChains_PreferredFontsFirst(t *testing.T) {
203	203		// Verify preferred fonts are first in chain
204		-	assert.Equal(t, "Fira Sans", SansSerifFallbackChain[0], "Fira Sans should be first in sans-serif chain")
205		-	assert.Equal(t, "Noto Serif", SerifFallbackChain[0], "Noto Serif should be first in serif chain")
206		-	assert.Equal(t, "Fira Code", MonospaceFallbackChain[0], "Fira Code should be first in monospace chain")
	204	+	assert.Equal(t, "Fira Sans", SansSerifFallbackChain()[0], "Fira Sans should be first in sans-serif chain")
	205	+	assert.Equal(t, "Noto Serif", SerifFallbackChain()[0], "Noto Serif should be first in serif chain")
	206	+	assert.Equal(t, "Fira Code", MonospaceFallbackChain()[0], "Fira Code should be first in monospace chain")
207	207		}


@@ -67,12 +67,12 @@ func TestDetector_SelectBestFont_SansSerif(t *testing.T) {
67		t.Skip("fc-list not available on this system")	67		t.Skip("fc-list not available on this system")
68		}	68		}
69			69
70	-	font := detector.SelectBestFont(ctx, port.FontCategorySansSerif, SansSerifFallbackChain)	70	+	font := detector.SelectBestFont(ctx, port.FontCategorySansSerif, SansSerifFallbackChain())
71		assert.NotEmpty(t, font)	71		assert.NotEmpty(t, font)
72			72
73		// Should either be a font from the chain or the generic fallback	73		// Should either be a font from the chain or the generic fallback
74		isFromChain := false	74		isFromChain := false
75	-	for _, f := range SansSerifFallbackChain {	75	+	for _, f := range SansSerifFallbackChain() {
76		if font == f {	76		if font == f {
77		isFromChain = true	77		isFromChain = true
78		break	78		break

@@ -93,11 +93,11 @@ func TestDetector_SelectBestFont_Serif(t *testing.T) {
93		t.Skip("fc-list not available on this system")	93		t.Skip("fc-list not available on this system")
94		}	94		}
95			95
96	-	font := detector.SelectBestFont(ctx, port.FontCategorySerif, SerifFallbackChain)	96	+	font := detector.SelectBestFont(ctx, port.FontCategorySerif, SerifFallbackChain())
97		assert.NotEmpty(t, font)	97		assert.NotEmpty(t, font)
98			98
99		isFromChain := false	99		isFromChain := false
100	-	for _, f := range SerifFallbackChain {	100	+	for _, f := range SerifFallbackChain() {
101		if font == f {	101		if font == f {
102		isFromChain = true	102		isFromChain = true
103		break	103		break

@@ -118,11 +118,11 @@ func TestDetector_SelectBestFont_Monospace(t *testing.T) {
118		t.Skip("fc-list not available on this system")	118		t.Skip("fc-list not available on this system")
119		}	119		}
120			120
121	-	font := detector.SelectBestFont(ctx, port.FontCategoryMonospace, MonospaceFallbackChain)	121	+	font := detector.SelectBestFont(ctx, port.FontCategoryMonospace, MonospaceFallbackChain())
122		assert.NotEmpty(t, font)	122		assert.NotEmpty(t, font)
123			123
124		isFromChain := false	124		isFromChain := false
125	-	for _, f := range MonospaceFallbackChain {	125	+	for _, f := range MonospaceFallbackChain() {
126		if font == f {	126		if font == f {
127		isFromChain = true	127		isFromChain = true
128		break	128		break

@@ -194,14 +194,14 @@ func TestDetector_genericFallback(t *testing.T) {
194		}	194		}
195			195
196		func TestFallbackChains_NotEmpty(t *testing.T) {	196		func TestFallbackChains_NotEmpty(t *testing.T) {
197	-	assert.NotEmpty(t, SansSerifFallbackChain, "sans-serif fallback chain should not be empty")	197	+	assert.NotEmpty(t, SansSerifFallbackChain(), "sans-serif fallback chain should not be empty")
198	-	assert.NotEmpty(t, SerifFallbackChain, "serif fallback chain should not be empty")	198	+	assert.NotEmpty(t, SerifFallbackChain(), "serif fallback chain should not be empty")
199	-	assert.NotEmpty(t, MonospaceFallbackChain, "monospace fallback chain should not be empty")	199	+	assert.NotEmpty(t, MonospaceFallbackChain(), "monospace fallback chain should not be empty")
200		}	200		}
201			201
202		func TestFallbackChains_PreferredFontsFirst(t *testing.T) {	202		func TestFallbackChains_PreferredFontsFirst(t *testing.T) {
203		// Verify preferred fonts are first in chain	203		// Verify preferred fonts are first in chain
204	-	assert.Equal(t, "Fira Sans", SansSerifFallbackChain[0], "Fira Sans should be first in sans-serif chain")	204	+	assert.Equal(t, "Fira Sans", SansSerifFallbackChain()[0], "Fira Sans should be first in sans-serif chain")
205	-	assert.Equal(t, "Noto Serif", SerifFallbackChain[0], "Noto Serif should be first in serif chain")	205	+	assert.Equal(t, "Noto Serif", SerifFallbackChain()[0], "Noto Serif should be first in serif chain")
206	-	assert.Equal(t, "Fira Code", MonospaceFallbackChain[0], "Fira Code should be first in monospace chain")	206	+	assert.Equal(t, "Fira Code", MonospaceFallbackChain()[0], "Fira Code should be first in monospace chain")
207		}	207		}