fix(config): report the first config problem in line order

Load parses and decodes the config leniently, then fails on the earliest
problem, so a bad value is no longer hidden behind a later duplicate or
syntax error, as before kvconf. Problems are classified by their source
and the known keys instead of kvconf's message text, the defaults are
written once, and Settings is built with keyed fields.

Tests cover mixed problems, a '#' after a value, quoted values and
invalid UTF-8.

commit: 6243ac06136d15bd368fda9df099506b282b78bf
author: brice · 12d ago · 2026-10-10 08:35 UTC
parent: aa29a4d
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

2 files · +56 -18

M internal/adapters/config/config.go +45 -18


@@ -5,6 +5,7 @@ import (
5	5		"errors"
6	6		"fmt"
7	7		"os"
	8	+	"slices"
8	9		"strings"
9	10
10	11		"github.com/bnema/kvconf"

@@ -36,11 +37,17 @@ type Settings struct {
36	37		Output ScreenshotOutput
37	38		}
38	39
39		-	const knownKeys = "selector.grid, selector.freeze, screenshot.dir, screenshot.output, video.dir"
	40	+	var keys = [...]string{"selector.grid", "selector.freeze", "screenshot.dir", "screenshot.output", "video.dir"}
40	41
41		-	// Strict: a line without "=", an unknown key or a duplicate is fatal. A "#"
42		-	// only starts a comment at the start of a line, and values are never unquoted.
43		-	var options = kvconf.Options{Strict: true, Comments: kvconf.CommentLine, RawValues: true, MaxBytes: maxBytes}
	42	+	var knownKeys = strings.Join(keys[:], ", ")
	43	+
	44	+	// A "#" only starts a comment at the start of a line, and values are never
	45	+	// unquoted. Every problem is collected, so Load can fail on the first one in
	46	+	// line order: a line without "=", an unknown key, a duplicate or a bad value.
	47	+	var options = kvconf.Options{Comments: kvconf.CommentLine, RawValues: true, MaxBytes: maxBytes}
	48	+
	49	+	// defaults are the settings of an absent file or key.
	50	+	var defaults = Settings{Grid: true, Freeze: true, Output: OutputFile}
44	51
45	52		// file is the config file as kvconf decodes it, before it becomes Settings.
46	53		type file struct {

@@ -71,7 +78,7 @@ type dirPath string
71	78
72	79		func (d *dirPath) UnmarshalText(text []byte) error {
73	80		if len(text) > maxPath {
74		-	return kvconf.ValueError("must be a non-empty path")
	81	+	return kvconf.ValueError(fmt.Sprintf("must be at most %d bytes", maxPath))
75	82		}
76	83		if string(text) == "~" {
77	84		return kvconf.ValueError("must be an absolute path or start with ~/")

@@ -89,15 +96,14 @@ func (d *dirPath) UnmarshalText(text []byte) error {
89	96		// screenshot selector. Invalid settings fail explicitly rather than silently
90	97		// changing the requested appearance.
91	98		func Load() (Settings, error) {
92		-	settings := Settings{Grid: true, Freeze: true, Output: OutputFile}
93	99		path, err := kvconf.DefaultPath("nefercap", "config")
94	100		if err != nil {
95		-	return settings, nil
	101	+	return defaults, nil
96	102		}
97	103		data, err := kvconf.ReadFile(path, options)
98	104		switch {
99	105		case errors.Is(err, os.ErrNotExist):
100		-	return settings, nil
	106	+	return defaults, nil
101	107		case errors.Is(err, kvconf.ErrTooLarge):
102	108		return Settings{}, fmt.Errorf("config %s exceeds %d bytes", path, maxBytes)
103	109		case errors.Is(err, kvconf.ErrNotRegular):

@@ -105,17 +111,38 @@ func Load() (Settings, error) {
105	111		case err != nil:
106	112		return Settings{}, fmt.Errorf("read config: %w", err)
107	113		}
108		-	f := file{Grid: true, Freeze: true, Output: OutputFile}
109		-	if _, err := kvconf.Unmarshal(data, &f, options); err != nil {
110		-	var w kvconf.Warning
111		-	if !errors.As(err, &w) {
112		-	return Settings{}, fmt.Errorf("config %s: %w", path, err)
113		-	}
114		-	msg := w.Key + " " + w.Msg
115		-	if w.Key == "" \|\| w.Msg == "unknown key" \|\| strings.HasPrefix(w.Msg, "overrides line") {
	114	+	entries, syntax, err := kvconf.ParseBytes(data, options)
	115	+	if err != nil {
	116	+	return Settings{}, fmt.Errorf("config %s: %w", path, err)
	117	+	}
	118	+	f := file{Grid: onOff(defaults.Grid), Freeze: onOff(defaults.Freeze), Output: defaults.Output}
	119	+	values, err := kvconf.Decode(entries, &f, options)
	120	+	if err != nil {
	121	+	return Settings{}, fmt.Errorf("config %s: %w", path, err)
	122	+	}
	123	+	// Fail on the first problem in line order. A line without "=" or a
	124	+	// duplicate (from ParseBytes) wins a tie, as a repeated key is wrong
	125	+	// whatever its value.
	126	+	var first *kvconf.Warning
	127	+	generic := false
	128	+	if len(syntax) > 0 {
	129	+	first, generic = &syntax[0], true
	130	+	}
	131	+	if len(values) > 0 && (first == nil \|\| values[0].Line < first.Line) {
	132	+	first, generic = &values[0], !slices.Contains(keys[:], values[0].Key)
	133	+	}
	134	+	if first != nil {
	135	+	msg := first.Key + " " + first.Msg
	136	+	if generic {
116	137		msg = "expected each of " + knownKeys + " at most once as key = value"
117	138		}
118		-	return Settings{}, fmt.Errorf("config %s line %d: %s", path, w.Line, msg)
	139	+	return Settings{}, fmt.Errorf("config %s line %d: %s", path, first.Line, msg)
119	140		}
120		-	return Settings{bool(f.Grid), bool(f.Freeze), string(f.ShotDir), string(f.VideoDir), f.Output}, nil
	141	+	return Settings{
	142	+	Grid: bool(f.Grid),
	143	+	Freeze: bool(f.Freeze),
	144	+	ScreenshotDir: string(f.ShotDir),
	145	+	VideoDir: string(f.VideoDir),
	146	+	Output: f.Output,
	147	+	}, nil
121	148		}


@@ -5,6 +5,7 @@ import (
5		"errors"	5		"errors"
6		"fmt"	6		"fmt"
7		"os"	7		"os"
			8	+	"slices"
8		"strings"	9		"strings"
9			10
10		"github.com/bnema/kvconf"	11		"github.com/bnema/kvconf"

@@ -36,11 +37,17 @@ type Settings struct {
36		Output ScreenshotOutput	37		Output ScreenshotOutput
37		}	38		}
38			39
39	-	const knownKeys = "selector.grid, selector.freeze, screenshot.dir, screenshot.output, video.dir"	40	+	var keys = [...]string{"selector.grid", "selector.freeze", "screenshot.dir", "screenshot.output", "video.dir"}
40			41
41	-	// Strict: a line without "=", an unknown key or a duplicate is fatal. A "#"	42	+	var knownKeys = strings.Join(keys[:], ", ")
42	-	// only starts a comment at the start of a line, and values are never unquoted.	43	+
43	-	var options = kvconf.Options{Strict: true, Comments: kvconf.CommentLine, RawValues: true, MaxBytes: maxBytes}	44	+	// A "#" only starts a comment at the start of a line, and values are never
			45	+	// unquoted. Every problem is collected, so Load can fail on the first one in
			46	+	// line order: a line without "=", an unknown key, a duplicate or a bad value.
			47	+	var options = kvconf.Options{Comments: kvconf.CommentLine, RawValues: true, MaxBytes: maxBytes}
			48	+
			49	+	// defaults are the settings of an absent file or key.
			50	+	var defaults = Settings{Grid: true, Freeze: true, Output: OutputFile}
44			51
45		// file is the config file as kvconf decodes it, before it becomes Settings.	52		// file is the config file as kvconf decodes it, before it becomes Settings.
46		type file struct {	53		type file struct {

@@ -71,7 +78,7 @@ type dirPath string
71			78
72		func (d *dirPath) UnmarshalText(text []byte) error {	79		func (d *dirPath) UnmarshalText(text []byte) error {
73		if len(text) > maxPath {	80		if len(text) > maxPath {
74	-	return kvconf.ValueError("must be a non-empty path")	81	+	return kvconf.ValueError(fmt.Sprintf("must be at most %d bytes", maxPath))
75		}	82		}
76		if string(text) == "~" {	83		if string(text) == "~" {
77		return kvconf.ValueError("must be an absolute path or start with ~/")	84		return kvconf.ValueError("must be an absolute path or start with ~/")

@@ -89,15 +96,14 @@ func (d *dirPath) UnmarshalText(text []byte) error {
89		// screenshot selector. Invalid settings fail explicitly rather than silently	96		// screenshot selector. Invalid settings fail explicitly rather than silently
90		// changing the requested appearance.	97		// changing the requested appearance.
91		func Load() (Settings, error) {	98		func Load() (Settings, error) {
92	-	settings := Settings{Grid: true, Freeze: true, Output: OutputFile}
93		path, err := kvconf.DefaultPath("nefercap", "config")	99		path, err := kvconf.DefaultPath("nefercap", "config")
94		if err != nil {	100		if err != nil {
95	-	return settings, nil	101	+	return defaults, nil
96		}	102		}
97		data, err := kvconf.ReadFile(path, options)	103		data, err := kvconf.ReadFile(path, options)
98		switch {	104		switch {
99		case errors.Is(err, os.ErrNotExist):	105		case errors.Is(err, os.ErrNotExist):
100	-	return settings, nil	106	+	return defaults, nil
101		case errors.Is(err, kvconf.ErrTooLarge):	107		case errors.Is(err, kvconf.ErrTooLarge):
102		return Settings{}, fmt.Errorf("config %s exceeds %d bytes", path, maxBytes)	108		return Settings{}, fmt.Errorf("config %s exceeds %d bytes", path, maxBytes)
103		case errors.Is(err, kvconf.ErrNotRegular):	109		case errors.Is(err, kvconf.ErrNotRegular):

@@ -105,17 +111,38 @@ func Load() (Settings, error) {
105		case err != nil:	111		case err != nil:
106		return Settings{}, fmt.Errorf("read config: %w", err)	112		return Settings{}, fmt.Errorf("read config: %w", err)
107		}	113		}
108	-	f := file{Grid: true, Freeze: true, Output: OutputFile}	114	+	entries, syntax, err := kvconf.ParseBytes(data, options)
109	-	if _, err := kvconf.Unmarshal(data, &f, options); err != nil {	115	+	if err != nil {
110	-	var w kvconf.Warning	116	+	return Settings{}, fmt.Errorf("config %s: %w", path, err)
111	-	if !errors.As(err, &w) {	117	+	}
112	-	return Settings{}, fmt.Errorf("config %s: %w", path, err)	118	+	f := file{Grid: onOff(defaults.Grid), Freeze: onOff(defaults.Freeze), Output: defaults.Output}
113	-	}	119	+	values, err := kvconf.Decode(entries, &f, options)
114	-	msg := w.Key + " " + w.Msg	120	+	if err != nil {
115	-	if w.Key == "" \|\| w.Msg == "unknown key" \|\| strings.HasPrefix(w.Msg, "overrides line") {	121	+	return Settings{}, fmt.Errorf("config %s: %w", path, err)
			122	+	}
			123	+	// Fail on the first problem in line order. A line without "=" or a
			124	+	// duplicate (from ParseBytes) wins a tie, as a repeated key is wrong
			125	+	// whatever its value.
			126	+	var first *kvconf.Warning
			127	+	generic := false
			128	+	if len(syntax) > 0 {
			129	+	first, generic = &syntax[0], true
			130	+	}
			131	+	if len(values) > 0 && (first == nil \|\| values[0].Line < first.Line) {
			132	+	first, generic = &values[0], !slices.Contains(keys[:], values[0].Key)
			133	+	}
			134	+	if first != nil {
			135	+	msg := first.Key + " " + first.Msg
			136	+	if generic {
116		msg = "expected each of " + knownKeys + " at most once as key = value"	137		msg = "expected each of " + knownKeys + " at most once as key = value"
117		}	138		}
118	-	return Settings{}, fmt.Errorf("config %s line %d: %s", path, w.Line, msg)	139	+	return Settings{}, fmt.Errorf("config %s line %d: %s", path, first.Line, msg)
119		}	140		}
120	-	return Settings{bool(f.Grid), bool(f.Freeze), string(f.ShotDir), string(f.VideoDir), f.Output}, nil	141	+	return Settings{
			142	+	Grid: bool(f.Grid),
			143	+	Freeze: bool(f.Freeze),
			144	+	ScreenshotDir: string(f.ShotDir),
			145	+	VideoDir: string(f.VideoDir),
			146	+	Output: f.Output,
			147	+	}, nil
121		}	148		}

M internal/adapters/config/config_test.go +11 -0


@@ -182,6 +182,17 @@ func TestLoadRejectsInvalidNewKeys(t *testing.T) {
182	182		{"screenshot.output = file\n\nscreenshot.output = file", "line 3: expected each of"},
183	183		{"video.dir = /a\nvideo.dir = /a", "line 2: expected each of"},
184	184		{"video.output = file", "line 1: expected each of"},
	185	+	// The first problem in line order is reported.
	186	+	{"selector.grid = maybe\nselector.grid = on", "line 1: selector.grid must be on or off"},
	187	+	{"selector.grid = maybe\nnoequals", "line 1: selector.grid must be on or off"},
	188	+	{"selector.grid = maybe\n= x", "line 1: selector.grid must be on or off"},
	189	+	{"selector.grid = maybe\nscreenshot.dir = /a\nscreenshot.dir = /b", "line 1: selector.grid must be on or off"},
	190	+	{"foo = 1\nbar", "line 1: expected each of"},
	191	+	{"selector.grid = on\nselector.grid = maybe", "line 2: expected each of"},
	192	+	// '#' only starts a comment at the start of a line; values stay raw.
	193	+	{"selector.grid = on # c", "line 1: selector.grid must be on or off"},
	194	+	{"screenshot.output = \"file\"", "line 1: screenshot.output must be"},
	195	+	{"# caf\xe9\nselector.grid = on", "not valid UTF-8"},
185	196		} {
186	197		t.Run(tc.text, func(t *testing.T) {
187	198		t.Setenv("HOME", t.TempDir())


@@ -182,6 +182,17 @@ func TestLoadRejectsInvalidNewKeys(t *testing.T) {
182	{"screenshot.output = file\n\nscreenshot.output = file", "line 3: expected each of"},	182		{"screenshot.output = file\n\nscreenshot.output = file", "line 3: expected each of"},
183	{"video.dir = /a\nvideo.dir = /a", "line 2: expected each of"},	183		{"video.dir = /a\nvideo.dir = /a", "line 2: expected each of"},
184	{"video.output = file", "line 1: expected each of"},	184		{"video.output = file", "line 1: expected each of"},
		185	+	// The first problem in line order is reported.
		186	+	{"selector.grid = maybe\nselector.grid = on", "line 1: selector.grid must be on or off"},
		187	+	{"selector.grid = maybe\nnoequals", "line 1: selector.grid must be on or off"},
		188	+	{"selector.grid = maybe\n= x", "line 1: selector.grid must be on or off"},
		189	+	{"selector.grid = maybe\nscreenshot.dir = /a\nscreenshot.dir = /b", "line 1: selector.grid must be on or off"},
		190	+	{"foo = 1\nbar", "line 1: expected each of"},
		191	+	{"selector.grid = on\nselector.grid = maybe", "line 2: expected each of"},
		192	+	// '#' only starts a comment at the start of a line; values stay raw.
		193	+	{"selector.grid = on # c", "line 1: selector.grid must be on or off"},
		194	+	{"screenshot.output = \"file\"", "line 1: screenshot.output must be"},
		195	+	{"# caf\xe9\nselector.grid = on", "not valid UTF-8"},
185	} {	196		} {
186	t.Run(tc.text, func(t *testing.T) {	197		t.Run(tc.text, func(t *testing.T) {
187	t.Setenv("HOME", t.TempDir())	198		t.Setenv("HOME", t.TempDir())