diff options
Diffstat (limited to 'internal/config')
| -rw-r--r-- | internal/config/enum_test.go | 52 | ||||
| -rw-r--r-- | internal/config/load.go | 45 | ||||
| -rw-r--r-- | internal/config/load_test.go | 34 | ||||
| -rw-r--r-- | internal/config/print.go | 12 | ||||
| -rw-r--r-- | internal/config/print_test.go | 24 |
5 files changed, 157 insertions, 10 deletions
diff --git a/internal/config/enum_test.go b/internal/config/enum_test.go new file mode 100644 index 0000000..2e5b718 --- /dev/null +++ b/internal/config/enum_test.go @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package config + +import ( + "strings" + "testing" + + "krino/internal/enumtest" +) + +// TestEverySettingValuePrintsAsItselfInAConfig: PrintRule writes settings +// with their String(), so every Conflict and CaseMode value must print as +// the word a configuration uses, and parse back as that same value. A value +// added later without its branch would otherwise print as "Conflict(3)", +// which krino check refuses (plan 13 review F5). +func TestEverySettingValuePrintsAsItselfInAConfig(t *testing.T) { + for _, c := range []struct { + typ, form string + printed func(int) string + parsed func(*Rule) string + }{ + {"Conflict", "on-conflict", func(i int) string { return Conflict(i).String() }, + func(r *Rule) string { return r.Settings.OnConflict.String() }}, + {"CaseMode", "case", func(i int) string { return CaseMode(i).String() }, + func(r *Rule) string { return r.Settings.Case.String() }}, + } { + names, err := enumtest.Names("settings.go", c.typ) + if err != nil { + t.Fatal(err) + } + if len(names) == 0 { + t.Fatalf("no %s values found", c.typ) + } + for i, name := range names { + word := c.printed(i) + if strings.ContainsAny(word, "()0123456789") { + t.Errorf("%s.String() = %q; give it a word a configuration uses", name, word) + continue + } + src := "(path \"/tmp\")\n(rule \"r\" (" + c.form + " " + word + ") (move \"Out\"))\n" + dir, errs := ParseDir("dl", "dl.conf", []byte(src)) + if len(errs) > 0 { + t.Errorf("%s prints as %q, which krino check refuses: %v", name, word, errs) + continue + } + if got := c.parsed(dir.Rules[0]); got != word { + t.Errorf("%s prints as %q but parses back as %q", name, word, got) + } + } + } +} diff --git a/internal/config/load.go b/internal/config/load.go index 3811802..14ce635 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -8,6 +8,7 @@ import ( "io/fs" "os" "path/filepath" + "sort" "krino/internal/sexp" "krino/internal/xdg" @@ -20,15 +21,45 @@ type Config struct { Dirs []*Dir } +// unusedOverrides reports every override LoadWith never read: text meant +// for a file this load does not touch, which would otherwise pass as +// checked while the file on disk was read instead (plan 13 review F4). +func unusedOverrides(over map[string][]byte, used map[string]bool, mainFile string) []*Diag { + var out []*Diag + for file := range over { + if !used[file] { + out = append(out, &Diag{File: mainFile, Msg: fmt.Sprintf("unsaved text for %s, which this configuration does not read", file)}) + } + } + sort.Slice(out, func(i, j int) bool { return out[i].Msg < out[j].Msg }) + return out +} + // readSource is the text of file: the caller's override when it has one, -// else the file's own bytes. -func readSource(overrides map[string][]byte, file string) ([]byte, error) { - if src, ok := overrides[file]; ok { +// else the file's own bytes. It records which overrides were used, so +// LoadWith can report one it never reached. +func readSource(overrides map[string][]byte, used map[string]bool, file string) ([]byte, error) { + key := filepath.Clean(file) + if src, ok := overrides[key]; ok { + used[key] = true return src, nil } return os.ReadFile(file) } +// cleanOverrides keys the caller's overrides by cleaned path, so a file +// named with a "." or ".." segment is still recognised as itself. +func cleanOverrides(overrides map[string][]byte) map[string][]byte { + if len(overrides) == 0 { + return nil + } + out := make(map[string][]byte, len(overrides)) + for file, src := range overrides { + out[filepath.Clean(file)] = src + } + return out +} + // DefaultFile is krino.conf in the XDG config directory. func DefaultFile() string { return filepath.Join(xdg.ConfigHome(), "krino", "krino.conf") @@ -52,7 +83,9 @@ func Load(mainFile string, names ...string) (*Config, []*Diag) { // checked exactly as a run would read it (GUI design ยง1.3). A nil map is // Load. func LoadWith(mainFile string, overrides map[string][]byte, names ...string) (*Config, []*Diag) { - src, err := readSource(overrides, mainFile) + over := cleanOverrides(overrides) + used := map[string]bool{} + src, err := readSource(over, used, mainFile) if errors.Is(err, fs.ErrNotExist) { return nil, []*Diag{{File: mainFile, Msg: "not found; create it with: krino init"}} } @@ -80,7 +113,7 @@ func LoadWith(mainFile string, overrides map[string][]byte, names ...string) (*C } for _, name := range want { file := DirFile(mainFile, name) - src, err := readSource(overrides, file) + src, err := readSource(over, used, file) if err != nil { msg := err.Error() if errors.Is(err, fs.ErrNotExist) { @@ -93,7 +126,7 @@ func LoadWith(mainFile string, overrides map[string][]byte, names ...string) (*C errs = append(errs, derrs...) cfg.Dirs = append(cfg.Dirs, dir) } - return cfg, errs + return cfg, append(errs, unusedOverrides(over, used, mainFile)...) } // Resolved is the settings that apply in dir: built-in, then the main diff --git a/internal/config/load_test.go b/internal/config/load_test.go index 13ef9d8..d522316 100644 --- a/internal/config/load_test.go +++ b/internal/config/load_test.go @@ -142,3 +142,37 @@ func TestLoadWithOverriddenText(t *testing.T) { t.Errorf("the files on disk were read differently: %v %+v", errs, cfg.Dirs) } } + +// TestLoadWithReportsAnUnusedOverride: an override whose path does not name +// a file the load reads - a different spelling of it, or a directory not in +// include - is reported, instead of the file on disk being read as though +// the unsaved text were fine (plan 13 review F4). +func TestLoadWithReportsAnUnusedOverride(t *testing.T) { + h := t.TempDir() + t.Setenv("HOME", h) + main := filepath.Join(h, "krino.conf") + os.MkdirAll(filepath.Join(h, "dirs"), 0o755) + os.WriteFile(main, []byte("(include \"dl\")\n"), 0o644) + os.WriteFile(filepath.Join(h, "dirs", "dl.conf"), []byte("(path \"/tmp\")\n"), 0o644) + + broken := []byte("(path \"/tmp\")\n(rule \"BROKEN\")\n") + // The same file, spelled with a "." segment: the text must still be used. + uncleaned := filepath.Join(h, "dirs", ".", "dl.conf") + if _, errs := LoadWith(main, map[string][]byte{uncleaned: broken}); len(errs) == 0 { + t.Error("an override keyed by an uncleaned path was ignored") + } + // A file this load never reads: say so rather than pass silently. + other := filepath.Join(h, "dirs", "other.conf") + errs := diagText(func() []*Diag { _, e := LoadWith(main, map[string][]byte{other: broken}); return e }()) + if !strings.Contains(errs, "other.conf") { + t.Errorf("an override for a file that is not read went unreported: %s", errs) + } +} + +func diagText(ds []*Diag) string { + var b strings.Builder + for _, d := range ds { + b.WriteString(d.Error() + "\n") + } + return b.String() +} diff --git a/internal/config/print.go b/internal/config/print.go index 5c3d467..a2244d0 100644 --- a/internal/config/print.go +++ b/internal/config/print.go @@ -3,6 +3,7 @@ package config import ( + "fmt" "strings" "krino/internal/sexp" @@ -59,12 +60,17 @@ func PrintExclude(x *Exclude) string { // Splice replaces the bytes of the form between start and end - a form's // Pos and End - with text, and returns the new file contents. Every other -// byte of src, comments and layout included, is kept exactly. -func Splice(src []byte, start, end sexp.Pos, text string) []byte { +// byte of src, comments and layout included, is kept exactly. Offsets that +// do not belong to src - a form parsed from text that has since changed - +// are an error, not a panic (plan 13 review F6). +func Splice(src []byte, start, end sexp.Pos, text string) ([]byte, error) { + if start.Offset < 0 || end.Offset < start.Offset || end.Offset > len(src) { + return nil, fmt.Errorf("config: splice %d:%d is not inside %d bytes", start.Offset, end.Offset, len(src)) + } out := make([]byte, 0, len(src)-(end.Offset-start.Offset)+len(text)) out = append(out, src[:start.Offset]...) out = append(out, text...) - return append(out, src[end.Offset:]...) + return append(out, src[end.Offset:]...), nil } // printAction renders one action form. diff --git a/internal/config/print_test.go b/internal/config/print_test.go index 9b8d479..510fdd9 100644 --- a/internal/config/print_test.go +++ b/internal/config/print_test.go @@ -5,6 +5,8 @@ package config import ( "strings" "testing" + + "krino/internal/sexp" ) func parseOne(t *testing.T, body string) *Dir { @@ -70,7 +72,11 @@ func TestSpliceLeavesTheRestAlone(t *testing.T) { if len(errs) > 0 { t.Fatal(errs) } - got := string(Splice(src, dir.Rules[0].Pos, dir.Rules[0].End, "(rule \"a\"\n (move \"In\"))")) + spliced, err := Splice(src, dir.Rules[0].Pos, dir.Rules[0].End, "(rule \"a\"\n (move \"In\"))") + if err != nil { + t.Fatal(err) + } + got := string(spliced) want := "; keep me\n(path \"/tmp\")\n(rule \"a\"\n (move \"In\"))\n; and me\n" if got != want { t.Errorf("got\n%q\nwant\n%q", got, want) @@ -79,3 +85,19 @@ func TestSpliceLeavesTheRestAlone(t *testing.T) { t.Error("Splice changed the source it was given") } } + +// TestSpliceRefusesOffsetsOutsideTheSource: a stale end offset - the file +// changed since the form was parsed - is an error, not a panic (plan 13 +// review F6). +func TestSpliceRefusesOffsetsOutsideTheSource(t *testing.T) { + src := []byte("(path \"/tmp\")") + for _, c := range []struct{ start, end int }{{0, len(src) + 2}, {8, 4}, {-1, 3}} { + if _, err := Splice(src, sexp.Pos{Offset: c.start}, sexp.Pos{Offset: c.end}, "x"); err == nil { + t.Errorf("Splice(%d, %d) did not refuse", c.start, c.end) + } + } + out, err := Splice(src, sexp.Pos{Offset: 0}, sexp.Pos{Offset: len(src)}, "(path \"/x\")") + if err != nil || string(out) != "(path \"/x\")" { + t.Errorf("Splice over the whole source = %q, %v", out, err) + } +} |