Skip to content

Commit bfc3bfb

Browse files
authored
Merge pull request #55 from specsnl/feat/GH-19-config-load-normalise
Add config file resolution, YAML load, and normalisation (GH-19)
2 parents a866d9f + 73bfb99 commit bfc3bfb

19 files changed

Lines changed: 1221 additions & 15 deletions

File tree

‎AGENTS.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ Run `task --list` for the full set. The ones used most:
2424
| `task lint` | `golangci-lint run` |
2525
| `task lint:fix` | `golangci-lint run --fix` |
2626
| `task test` | `go test -race -tags=integration ./...` |
27+
| `task test:update` | Rewrite the golden files from the current output |
2728
| `task md:check` | markdownlint over every Markdown file |
2829
| `task md:fix` | Align Markdown tables, then apply autofixable rules |
2930
| `task build` | Build the binary into the working directory |

‎Taskfile.dist.yml‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,29 @@ tasks:
103103
vars:
104104
SUB_CMD: "go test -race -tags=integration ./..."
105105

106+
test:update:
107+
desc: Rewrite the golden files from what the tests produce now
108+
summary: |
109+
Runs the tests with -update in every package that defines the flag, which
110+
rewrites that package's testdata/*.golden files from the current output.
111+
112+
Then read the diff. A golden that changed is either the change you meant
113+
to make or a regression, and the file is the only thing that says which.
114+
115+
The package list is discovered rather than hardcoded: any test file that
116+
declares an -update flag puts its package in. Nothing to remember when a
117+
new golden suite lands.
118+
cmds:
119+
- task: dc:run:go-builder
120+
vars:
121+
# Single-quoted so the command substitution runs in the container
122+
# rather than on the host. -F makes the pattern a literal string, so
123+
# the ( and the . in flag.Bool("update" need no regex escaping; the
124+
# \" survive because bash leaves a backslash alone inside "..." unless
125+
# it precedes $ ` " \ or a newline.
126+
SUB_CMD: >-
127+
bash -c 'go test $(grep -rlF --include="*_test.go" "flag.Bool(\"update\"" . | xargs -n1 dirname | sort -u) -update'
128+
106129
checkall:
107130
desc: Run every check — the tidy check, golangci-lint, the Go tests, and markdownlint
108131
summary: |

‎docs/content/docs/architecture/_index.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ lands, its behaviour moves from the design plan into this section.
1818
| [Output & Exit Codes](./output.md) | `output.Writer`, pretty vs NDJSON, TTY detection, the `slog` boundary, exit codes |
1919
| [Versioning](./versioning.md) | The linker-injected `Version`, and what each build produces |
2020
| [Colour Palette](./palette.md) | The deterministic HSL candidate grid, its legibility bounds, and determinism |
21+
| [Configuration](./configuration.md) | Config file resolution, YAML parsing, and the normalisation rules |
2122

2223
Pages are added here as the subsystems they document are implemented — the planner and the GitHub
2324
client with its rate limiting each get their own page.
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
---
2+
title: Configuration
3+
weight: 6
4+
---
5+
6+
`internal/config` turns a YAML file into the structs every later stage reads. It is deliberately
7+
three separate concerns in three files, and only the first has landed:
8+
9+
| File | Concern | Status |
10+
|---------------|---------------------------------------------------|---------|
11+
| `config.go` | Find the file, parse it, normalise it | landed |
12+
| `validate.go` | The rules in the design's validation table | planned |
13+
| `resolve.go` | Groups → repository sets, `include_groups` cycles | planned |
14+
15+
Nothing in the package touches the network, and nothing in `config.go` rejects a config: an
16+
invalid colour parses into the struct exactly as written and is `validate.go`'s answer to give.
17+
Splitting it that way keeps the error a user sees attached to the rule it broke rather than to a
18+
decoder.
19+
20+
## Finding the file
21+
22+
```go
23+
func Find(explicit string) (string, error) // path only
24+
func Load(explicit string) (*Config, error) // Find + LoadFile
25+
func LoadFile(path string) (*Config, error) // read + Parse, records Config.Path
26+
func Parse(data []byte) (*Config, error) // decode + normalise, no I/O
27+
```
28+
29+
`Find` resolves in this order:
30+
31+
1. `explicit` — the `--config` value, when one was given
32+
2. `labels.yml` or `labels.yaml` in the working directory
33+
3. `labels.yml` or `labels.yaml` under `ConfigDir()` — `$XDG_CONFIG_HOME/labelsync`
34+
35+
A `--config` value naming a **directory** is searched the same way as the other two, so
36+
`--config ~/.config/labelsync` and `--config ~/.config/labelsync/labels.yml` both work.
37+
38+
Two outcomes are sentinels, wrapped with `%w` so `KindOf` can name them in JSON output:
39+
40+
| Situation | Sentinel |
41+
|-----------------------------------------------|--------------------------|
42+
| Both spellings in the same directory | `ErrAmbiguousConfigFile` |
43+
| No config file at the path, or in any of them | `ErrConfigNotFound` |
44+
45+
Ambiguity is checked **per directory, while searching it**. A working directory holding both
46+
spellings is an error even though the XDG directory holds a perfectly good file — a run whose
47+
config depends on which name the tool happened to look at first is worse than a run that stops.
48+
This is the rule specs-cli applies to `project.yml` / `project.yaml`.
49+
50+
A YAML syntax error is not one of the run's failure modes and carries no sentinel. It is wrapped
51+
with the file it came from, so `KindOf` returns `""` and the message still says where to look.
52+
53+
## Normalisation
54+
55+
`Parse` normalises before it returns, so no later stage has to ask whether a colour still has its
56+
`#` or whether a label's groups have been filled in.
57+
58+
| Input | Normalised to |
59+
|---------------------------------|-------------------------------------------------------|
60+
| `color: "#D73A4A"` | `d73a4a` — one leading `#` stripped, then lowercased |
61+
| `name: " type: bug "` | `type: bug` — trimmed |
62+
| `from` / `to` in a rename | Trimmed, because they are matched against label names |
63+
| A label with no `groups` | A copy of `defaults.groups` |
64+
| A group with no `skip_archived` | `true` |
65+
| A group with no `skip_forks` | `true` |
66+
| A group with no `visibility` | `all` |
67+
68+
Only **one** leading `#` is stripped: `##abc` normalises to `#abc`, which is invalid, which is the
69+
point. Normalisation tidies, it never repairs — a repaired colour would silently apply a colour
70+
the file does not name.
71+
72+
Inherited groups are **cloned per label** rather than aliased to the one `defaults.groups` slice.
73+
Sharing a backing array across every defaulted label is the kind of thing that works until the
74+
first stage that filters a label's groups in place.
75+
76+
### Why group defaults are applied while decoding
77+
78+
`skip_archived` and `skip_forks` default to `true`, which cannot be done after decoding: once the
79+
struct is filled in, an explicit `skip_archived: false` and an omitted `skip_archived` are both
80+
`false` and nothing can tell them apart. So `Group.UnmarshalYAML` decodes over a defaulted value
81+
instead of over a zero one.
82+
83+
That alone is not quite enough. A group written as a bare key —
84+
85+
```yaml
86+
groups:
87+
self:
88+
```
89+
90+
— is a YAML null, and `yaml.v3` resolves a null to the zero value *without* consulting the
91+
target's own `UnmarshalYAML`. The defaults would be zeroed straight back out. `Groups` therefore
92+
has a decoder of its own that walks the mapping entry by entry and hands a null entry the defaults
93+
directly. Both paths are pinned by tests.
94+
95+
### Descriptions
96+
97+
`Label.Description` is a plain `string`, not a `*string`, and that is a decision rather than an
98+
omission. The design defines descriptions as **authoritative**: omitting one *clears* the remote
99+
description. An omitted description and an explicitly empty one therefore describe the same
100+
desired state, and nothing downstream would do anything different with the distinction.
101+
102+
If a future mode ever means "leave descriptions alone", that is the change that introduces the
103+
pointer — not this one.
104+
105+
## Testing
106+
107+
- **Resolution** is table-driven over every branch of the order, including both ambiguity cases
108+
and both not-found cases, with the working directory and `XDG_CONFIG_HOME` pointed at temporary
109+
directories. Every error case also asserts `KindOf` still names the sentinel through the
110+
wrapping.
111+
- **Normalisation** is pinned by golden files: `testdata/*.yml` in, the normalised struct
112+
marshalled back to YAML out. A change to any defaulting or tidying rule shows up as a diff in
113+
the golden rather than as a subtly different plan several stages later.
114+
115+
```sh
116+
task test:update # rewrite the goldens, here and in every other golden package
117+
```

‎docs/content/docs/architecture/output.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,7 @@ decide.
497497
Golden files under `internal/util/output/testdata/`, regenerated with:
498498

499499
```sh
500-
task dc:run:go-builder -- go test ./internal/util/output/ -update
500+
task test:update
501501
```
502502

503503
The pretty rendering has **two** goldens per stream: one produced with an empty environment, where

‎docs/content/docs/architecture/overview.md‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,15 @@ labelsync/
3636

3737
### Implemented so far
3838

39-
| Package | Status | Notes |
40-
|------------------------|---------|----------------------------------------------------------------|
41-
| `internal/labelsync` | landed | XDG config/cache paths, config file names, sentinels, `KindOf` |
42-
| `internal/util/exit` | landed | The four exit codes — see [Output & Exit Codes](./output.md) |
43-
| `internal/util/output` | landed | `Writer`, pretty + NDJSON, TTY detection, `slog` wiring |
44-
| `internal/cmd` | partial | Root command, `App`, persistent flags, `version` |
45-
| `internal/palette` | partial | The candidate grid — see [Colour Palette](./palette.md) |
46-
| everything else | planned | See the milestone table in the design plan |
39+
| Package | Status | Notes |
40+
|------------------------|---------|--------------------------------------------------------------------------------|
41+
| `internal/labelsync` | landed | XDG config/cache paths, config file names, sentinels, `KindOf` |
42+
| `internal/util/exit` | landed | The four exit codes — see [Output & Exit Codes](./output.md) |
43+
| `internal/util/output` | landed | `Writer`, pretty + NDJSON, TTY detection, `slog` wiring |
44+
| `internal/cmd` | partial | Root command, `App`, persistent flags, `version` |
45+
| `internal/config` | partial | Resolution, YAML load, normalisation — see [Configuration](./configuration.md) |
46+
| `internal/palette` | partial | The candidate grid — see [Colour Palette](./palette.md) |
47+
| everything else | planned | See the milestone table in the design plan |
4748

4849
### Why `plan` and `palette` are isolated
4950

@@ -129,11 +130,14 @@ The config file is resolved in this order:
129130
2. `./labels.yml` or `./labels.yaml` in the working directory
130131
3. `$XDG_CONFIG_HOME/labelsync/labels.yml` (default `~/.config/labelsync/labels.yml`)
131132

132-
Both spellings in one directory is `ErrAmbiguousConfigFile`. The ETag cache lives under
133-
`$XDG_CACHE_HOME/labelsync`. Both paths come from `internal/labelsync/configuration.go`.
133+
A `--config` value naming a directory is searched the same way. Both spellings in one directory is
134+
`ErrAmbiguousConfigFile`. The ETag cache lives under `$XDG_CACHE_HOME/labelsync`. Both paths come
135+
from `internal/labelsync/configuration.go`.
134136

135-
See the [configuration reference](https://github.com/specsnl/labelsync/blob/main/docs/design.md#configuration)
136-
for the `groups`, `defaults`, `renames`, and `labels` sections.
137+
How that resolution, the YAML load, and the normalisation rules work is in
138+
[Configuration](./configuration.md). For the `groups`, `defaults`, `renames`, and `labels` sections
139+
themselves, see the
140+
[configuration reference](https://github.com/specsnl/labelsync/blob/main/docs/design.md#configuration).
137141

138142
## Data flow — `labelsync sync`
139143

‎docs/content/docs/usage/_index.md‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,34 @@ Without `--config`, the file is searched for in this order:
3939
1. `./labels.yml` or `./labels.yaml` in the working directory
4040
2. `$XDG_CONFIG_HOME/labelsync/labels.yml` (default `~/.config/labelsync/labels.yml`)
4141

42-
Both spellings in one directory is an error rather than a coin flip. The ETag cache lives under
42+
Both spellings in one directory is an error rather than a coin flip — in *either* directory, even
43+
when the other one holds a perfectly good file. The ETag cache lives under
4344
`$XDG_CACHE_HOME/labelsync` (default `~/.cache/labelsync`).
4445

46+
`--config` takes a file or a directory: `--config ./ops` searches `./ops` for both spellings, the
47+
same way the working directory is searched.
48+
49+
### What the file is tidied into
50+
51+
The config file is normalised as it is read, so a few things are accepted in more than one
52+
spelling and mean the same thing:
53+
54+
| You write | It means |
55+
|---------------------------------|------------------------------------------------------------|
56+
| `color: "#D73A4A"` | `d73a4a` — a leading `#` is optional, case does not matter |
57+
| `name: " type: bug "` | `type: bug` — surrounding whitespace is trimmed |
58+
| A label with no `groups` | The groups listed under `defaults.groups` |
59+
| A group with no `skip_archived` | `skip_archived: true` |
60+
| A group with no `skip_forks` | `skip_forks: true` |
61+
| A group with no `visibility` | `visibility: all` |
62+
63+
Omitting a label's `description` is **not** shorthand for "leave it alone" — descriptions are
64+
authoritative, so an omitted one clears whatever the repository has. Use `export` to capture the
65+
descriptions you already have before the first run.
66+
67+
The full file reference — every field of `groups`, `defaults`, `renames`, and `labels` — is in the
68+
[design plan](https://github.com/specsnl/labelsync/blob/main/docs/design.md#configuration).
69+
4570
## Commands
4671

4772
Only `version` is implemented so far. `sync`, `export`, `init`, `groups`, and `cache` are the

‎go.mod‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ require (
99
github.com/charmbracelet/x/term v0.2.2
1010
github.com/lucasb-eyer/go-colorful v1.4.0
1111
github.com/spf13/cobra v1.10.2
12+
gopkg.in/yaml.v3 v3.0.1
1213
)
1314

1415
require (

‎go.sum‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ golang.org/x/sync v0.18.0 h1:kr88TuHDroi+UVf+0hZnirlk8o8T+4MrK6mr60WkH/I=
4949
golang.org/x/sync v0.18.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI=
5050
golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw=
5151
golang.org/x/sys v0.46.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
52+
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
5253
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
5354
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
5455
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

0 commit comments

Comments
 (0)