diff --git a/CHANGELOG.md b/CHANGELOG.md index 6dde6998..fadf5d17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -175,6 +175,28 @@ because it turns other people's test suites red. ### Added +- **A recipe can build on a preset.** Two keys the recipe reader used to + refuse as not built yet now work: `extends: preset:` names the preset + and `with:` fills its parameters, written the way the flags take them + (`limit: 5mb`, `spread: 1B,1kb,1mb`, `format: png`). The preset's files + come first and the recipe's own `targets` are added after them, so the + file is the same run as `tfg preset eject` with the extra targets typed + under it, byte for byte, and shorter. A target whose `id` the preset + already uses is refused rather than replaced, `with` without `extends` is + refused, and `extends` names a preset and nothing else yet - not another + file. A recipe with `extends` and no `targets` of its own is legal, which + is how a preset run is committed to a repository. The manifest records the + preset under `run.preset` with the parameters left out listed as + `defaulted`, as a `--preset` run does, and `run.recipe_hash` is the hash + of the file as written. `tfg validate --json` carries the same `preset` + block. On the desktop window, the batch screen has a section "Build on a + preset": a switch, the preset, and its parameters drawn under it. A preset + flag beside a recipe file (`tfg generate r.yaml --limit 5mb`) is refused + with a sentence saying the value goes under `with`. +- **The batch screen's "Label in each file" switch starts on.** It started + off, while the single batch screen and a recipe file with no `defaults` + section both have the label on - so the same recipe run from that screen + gave different bytes. All three now agree. - **Every control shows where the keyboard is and answers the pointer.** Every place the keyboard can land - a box, a menu, a switch, a button, the segmented switch, a word on the tab strip - draws the same 2 px ring when a diff --git a/README.md b/README.md index bb4a238b..d574c255 100644 --- a/README.md +++ b/README.md @@ -428,6 +428,8 @@ tfg generate fixtures.yaml | `seed` | the number that makes a run repeatable. Same seed, same bytes | | `defaults.label` | write the self describing label inside each file. Default `true` | | `targets` | the list of things to produce. See below | +| `extends` | a preset to build on, as `preset:size-boundaries`. Its files come first and your `targets` are added after them. See [Building on a preset](#building-on-a-preset) | +| `with` | the preset's parameters, written the way the flags take them: `limit: 5mb`, `spread: 1B,1kb,1mb`, `format: png`. One left out stands in from its default, and the manifest says so | | `output.dir` | where the files and the manifest go. A relative path is read from the directory you run in, not from the one the recipe sits in | | `output.manifest` | manifest file name. Default `manifest.json` | @@ -483,11 +485,39 @@ A reason names **the rule in play**, not the verdict. That is why the same reason can sit under either outcome - a file one byte under a limit is `accept`, and the rule it is about is still `size_limit`. +### Building on a preset + +A recipe can start from a preset's set and add its own files: + +```yaml +version: 1 +seed: 7 +extends: preset:size-boundaries +with: + limit: 5mb + format: png +targets: + - id: our-legacy-format + format: tiff + count: 1 + size: 3mb +``` + +This is the same run as `tfg preset eject size-boundaries --limit 5mb --format png` +with the extra target typed under it, byte for byte. The file is shorter, it +says which question the set answers, and the manifest records the preset under +`run.preset` with the parameters you left out listed as `defaulted`. A target +whose `id` the preset already uses is refused, never silently replaced. A +recipe with `extends` and no `targets` of its own is legal, and it is how a +preset run is committed to a repository. The `--limit` and other preset flags +do not apply beside a recipe file, and the recipe's `with` is where they go. + ### Not built yet These keys are recognised and **refused with a message saying so**, never -ignored quietly: `extends`, `with`, `policy`, `engine`, `defaults.fill`, -`fill` on a target, `mutations`, `output.split_threshold`. +ignored quietly: `policy`, `engine`, `defaults.fill`, `fill` on a target, +`output.split_threshold`. `extends` names a preset and nothing else yet, so a +recipe cannot build on another file. ## 📁 Formats in detail diff --git a/internal/cli/generate.go b/internal/cli/generate.go index 94fa1d16..fd3d674d 100644 --- a/internal/cli/generate.go +++ b/internal/cli/generate.go @@ -186,7 +186,7 @@ func generate(ctx context.Context, args []string, out, errOut io.Writer) int { // targetsFromRecipe reads the recipe and settles what the flags override. func targetsFromRecipe(path string, g *generateOpts, given map[string]bool, opt *engine.Options, errOut io.Writer) ([]engine.Target, int) { - rec, hash, code := loadRecipe(path, errOut) + read, hash, code := loadRecipe(path, errOut) if code != ExitOK { return nil, code } @@ -200,7 +200,11 @@ func targetsFromRecipe(path string, g *generateOpts, given map[string]bool, opt return nil, ExitUsage } - return targetsFromParsedRecipe(rec, hash, g, given, opt), ExitOK + // A file that builds on a preset is recorded and heard the way a --preset + // run is. Nil for a file that stands alone, and the field stays absent. + sayNotes(read.Notes(), errOut) + opt.Preset = record(read.Expansion) + return targetsFromParsedRecipe(read.Recipe, hash, g, given, opt), ExitOK } // targetsFromParsedRecipe settles what the flags take away from a recipe that diff --git a/internal/cli/preset.go b/internal/cli/preset.go index d2a9b369..f2cdf5a8 100644 --- a/internal/cli/preset.go +++ b/internal/cli/preset.go @@ -28,7 +28,13 @@ import ( // which is the output contract and no business of an input concept. The drift // this invites is watched behaviourally instead - a guard runs the same preset // from both surfaces and compares the records they produce. +// +// Nil in, nil out: a recipe file that stands alone has no preset to record, +// and the manifest field is absent rather than empty for it. func record(e *preset.Expansion) *manifest.Preset { + if e == nil { + return nil + } return &manifest.Preset{ ID: e.Preset.ID, Parameters: map[string]string(e.Settled), @@ -36,6 +42,15 @@ func record(e *preset.Expansion) *manifest.Preset { } } +// sayNotes tells a person what a preset invented, on standard error, where +// a run's other asides go. One place for the three roads that say it, so +// the prefix cannot drift between them. +func sayNotes(notes []string, errOut io.Writer) { + for _, note := range notes { + fmt.Fprintf(errOut, "note: %s\n", note) + } +} + // budget is what a preset would produce, counted by the planner. // // Not a declared number beside the code. The one that used to sit in @@ -270,9 +285,12 @@ func explainUndefinedFlag(fs *flag.FlagSet, args []string, errOut io.Writer) boo if name == "" { return false } + // The second sentence names both roads, because since 2026-09-22 a + // recipe file can build on the preset too - and beside a file the flag + // does not exist either, the file's with section is where the value goes. fmt.Fprintf(errOut, - "tfg: --%s is a parameter of the preset %s, so it only exists beside it. Add --preset %s, or drop --%s.\n", - name, owner, owner, name) + "tfg: --%s is a parameter of the preset %s, so it only exists beside it. Add --preset %s, put %s under with: in a recipe that extends it, or drop --%s.\n", + name, owner, owner, name, name) return true } @@ -331,9 +349,7 @@ func targetsFromPreset(fs *flag.FlagSet, g *generateOpts, given map[string]bool, return nil, classify(err) } - for _, note := range expanded.Notes() { - fmt.Fprintf(errOut, "note: %s\n", note) - } + sayNotes(expanded.Notes(), errOut) opt.Preset = record(expanded) return targetsFromParsedRecipe(rec, hash, g, given, opt), ExitOK } diff --git a/internal/cli/presetcmd.go b/internal/cli/presetcmd.go index 7bea1bdb..6dd1699c 100644 --- a/internal/cli/presetcmd.go +++ b/internal/cli/presetcmd.go @@ -241,8 +241,11 @@ func describePreset(e *preset.Expansion, b budget, out io.Writer) { fmt.Fprintf(out, " - %s\n", c) } } - fmt.Fprintf(out, "\nRun \"tfg preset eject %s\" for the recipe, or \"tfg generate --preset %s\" to produce the files.\n", - p.ID, p.ID) + // Three roads to the same set, and all three are named: the third + // arrived on 2026-09-22 and a setting nobody knows they can reach is a + // setting that is not there. + fmt.Fprintf(out, "\nRun \"tfg preset eject %s\" for the recipe, \"tfg generate --preset %s\" to produce the files, or write \"extends: preset:%s\" in a recipe of your own.\n", + p.ID, p.ID, p.ID) } func presetEject(args []string, out, errOut io.Writer) int { @@ -268,9 +271,7 @@ Usage: // The note goes to the error channel. The recipe is the data here, and a // sentence about a number we chose has no business inside a file somebody // is about to commit. - for _, note := range expanded.Notes() { - fmt.Fprintf(errOut, "note: %s\n", note) - } + sayNotes(expanded.Notes(), errOut) if _, err := out.Write(expanded.Source); err != nil { fmt.Fprintf(errOut, "tfg: cannot write the recipe: %s\n", describeError(err)) return ExitIO diff --git a/internal/cli/recipecmd.go b/internal/cli/recipecmd.go index d3a11ff3..8562413e 100644 --- a/internal/cli/recipecmd.go +++ b/internal/cli/recipecmd.go @@ -12,17 +12,28 @@ import ( "github.com/donislawdev/TestingFilesGenerator/internal/core" "github.com/donislawdev/TestingFilesGenerator/internal/engine" + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" + "github.com/donislawdev/TestingFilesGenerator/internal/preset" "github.com/donislawdev/TestingFilesGenerator/internal/recipe" ) -func loadRecipe(path string, errOut io.Writer) (*recipe.Recipe, string, int) { +// loadRecipe reads a recipe file for a run, through the door that knows +// presets: a file that builds on one comes back with the preset it built on, +// so the run can record which numbers were the preset's own. +// +// The hash is of the file as written, whether or not it builds on a preset. +// It answers the question a pipeline asks - was this manifest made from the +// recipe committed here - and the manifest's preset record, with the tool's +// version beside it, says the rest. The owner's decision of 2026-09-22, in +// docs/EXTENDS-WITH-2026-09-22.md section 2.4. +func loadRecipe(path string, errOut io.Writer) (*preset.Read, string, int) { src, err := readRecipe(path) if err != nil { said, code := recipeReadFailure(path, err) fmt.Fprintf(errOut, "tfg: %s\n", said) return nil, "", code } - rec, err := recipe.Parse(src, path) + read, err := preset.ReadRecipe(src, path) if err != nil { fmt.Fprintf(errOut, "tfg: %s\n", describeError(err)) return nil, "", classify(err) @@ -32,7 +43,7 @@ func loadRecipe(path string, errOut io.Writer) (*recipe.Recipe, string, int) { fmt.Fprintf(errOut, "tfg: %s\n", describeError(err)) return nil, "", classify(err) } - return rec, hash, ExitOK + return read, hash, ExitOK } // validate runs the checks a run would run and writes nothing at all, so it @@ -66,10 +77,11 @@ func validate(ctx context.Context, args []string, out, errOut io.Writer) int { return ExitUsage } - rec, hash, code := loadRecipeReporting(path, *asJSON, errOut) + read, hash, code := loadRecipeReporting(path, *asJSON, errOut) if code != ExitOK { return code } + rec := read.Recipe // The schema and the semantics both passed. Planning is what proves the // rest: a size below the minimum of its format, a format nobody @@ -85,11 +97,17 @@ func validate(ctx context.Context, args []string, out, errOut io.Writer) int { return planningRefusal(err, path, *asJSON, errOut) } + // A file that builds on a preset says so here the way the manifest will + // say it, and its notes go where they go on a run: to a person, on + // standard error, before the files exist as well as after. + sayNotes(read.Notes(), errOut) + if *asJSON { return writeJSON(out, errOut, validateReport{ Recipe: path, Valid: true, RecipeHash: hash, Targets: len(rec.Targets), Files: len(planned), TotalBytes: engine.TotalBytes(planned), + Preset: record(read.Expansion), Problems: []validateProblem{}, }, ExitOK) } @@ -97,6 +115,9 @@ func validate(ctx context.Context, args []string, out, errOut io.Writer) int { fmt.Fprintf(out, "%s is valid: %s, %s, %s total\n%s\n", path, core.Count(len(rec.Targets), "target", "targets"), core.Count(len(planned), "file", "files"), core.ExactBytes(engine.TotalBytes(planned)), hash) + if read.Expansion != nil { + fmt.Fprintf(out, "built on preset %s\n", read.Expansion.Preset.ID) + } return ExitOK } @@ -138,13 +159,17 @@ func planningOptions(rec *recipe.Recipe) engine.Options { // rather than as one blob of prose, because RC7 already reports them all at // once and a script should not have to split the message back apart. type validateReport struct { - Recipe string `json:"recipe"` - Valid bool `json:"valid"` - RecipeHash string `json:"recipe_hash,omitempty"` - Targets int `json:"targets,omitempty"` - Files int `json:"files,omitempty"` - TotalBytes int64 `json:"total_bytes,omitempty"` - Problems []validateProblem `json:"problems"` + Recipe string `json:"recipe"` + Valid bool `json:"valid"` + RecipeHash string `json:"recipe_hash,omitempty"` + Targets int `json:"targets,omitempty"` + Files int `json:"files,omitempty"` + TotalBytes int64 `json:"total_bytes,omitempty"` + // Preset is the preset the recipe builds on, in the shape the manifest + // records it - id, settled parameters, and which of them stood in from + // their defaults. Absent when the recipe stands alone. + Preset *manifest.Preset `json:"preset,omitempty"` + Problems []validateProblem `json:"problems"` } // validateProblem carries the three parts every refusal in this tool has: what @@ -212,7 +237,7 @@ func addressOf(err error) string { // loadRecipeReporting is loadRecipe with the option of a machine readable // refusal. A recipe with five problems has to arrive as five entries, not as // one string a script would have to take apart. -func loadRecipeReporting(path string, asJSON bool, errOut io.Writer) (*recipe.Recipe, string, int) { +func loadRecipeReporting(path string, asJSON bool, errOut io.Writer) (*preset.Read, string, int) { if !asJSON { return loadRecipe(path, errOut) } @@ -222,7 +247,7 @@ func loadRecipeReporting(path string, asJSON bool, errOut io.Writer) (*recipe.Re return nil, "", writeJSON(errOut, errOut, validateReport{Recipe: path, Valid: false, Problems: []validateProblem{{What: said}}}, code) } - rec, err := recipe.Parse(src, path) + read, err := preset.ReadRecipe(src, path) if err != nil { report := validateReport{Recipe: path, Valid: false, Problems: []validateProblem{}} var invalid *recipe.ValidationError @@ -240,7 +265,7 @@ func loadRecipeReporting(path string, asJSON bool, errOut io.Writer) (*recipe.Re return nil, "", writeJSON(errOut, errOut, validateReport{Recipe: path, Valid: false, Problems: []validateProblem{{What: err.Error()}}}, classify(err)) } - return rec, hash, ExitOK + return read, hash, ExitOK } // recipeCmd groups the operations that work on a recipe file itself rather diff --git a/internal/guard/extends_test.go b/internal/guard/extends_test.go new file mode 100644 index 00000000..c383bfb2 --- /dev/null +++ b/internal/guard/extends_test.go @@ -0,0 +1,419 @@ +package guard + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/cli" + _ "github.com/donislawdev/TestingFilesGenerator/internal/format/all" + "github.com/donislawdev/TestingFilesGenerator/internal/gui/text" + "github.com/donislawdev/TestingFilesGenerator/internal/gui/window" + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" + "github.com/donislawdev/TestingFilesGenerator/internal/preset" + "github.com/donislawdev/TestingFilesGenerator/internal/recipe" +) + +// A recipe that builds on a preset: extends and with, docs/RECIPE.md section +// 6, built on 2026-09-22 after docs/EXTENDS-WITH-2026-09-22.md. +// +// The whole of the promise is that such a file is the same run as "tfg preset +// eject" followed by editing - PR5 in a second form. So the guards here run +// both roads and compare the bytes, hold every preset to contributing targets +// and nothing else, hold every door a file comes through to the one that +// expands a preset, and read every refusal about the preset side by the line +// it names. + +// A preset's expansion is a version and a list of targets, and nothing else. +// +// This is the rule that makes the merge simple: the file owns the seed, the +// defaults and the output section, and the preset never contests them. It is +// asked of every registered preset with its declared defaults, so the day a +// preset expands into a seed of its own, this goes red before any recipe has +// built on it. ParseExtending asks the same question of the one expansion it +// is handed, which is what makes the rule a fact for a preset this list has +// not met. +func TestAPresetExpandsToTargetsAndNothingElse(t *testing.T) { + presets := preset.All() + if len(presets) == 0 { + t.Fatal("no preset is registered - this guard would pass without checking anything") + } + for _, p := range presets { + t.Run(p.ID, func(t *testing.T) { + settled, err := p.Settle(nil) + if err != nil { + t.Fatal(err) + } + src, err := p.Expand(settled) + if err != nil { + t.Fatal(err) + } + if err := recipe.CheckBase(src, p.ID); err != nil { + t.Errorf("%v\nA recipe that builds on this preset takes its targets and owns "+ + "everything else, so the preset may not carry anything else.", err) + } + }) + } + + // The predicate itself, on a base the tree does not contain. Every + // registered preset passes today, so a rule that stopped looking would + // find nothing to refuse and stay green. + for _, c := range []struct { + src string + bad bool + why string + }{ + {"version: 1\ntargets:\n - id: a\n format: txt\n count: 1\n size: 1\n", false, "a version and targets"}, + {"version: 1\nseed: 3\ntargets:\n - id: a\n format: txt\n count: 1\n size: 1\n", true, "a seed"}, + {"version: 1\ndefaults:\n label: false\ntargets:\n - id: a\n format: txt\n count: 1\n size: 1\n", true, "a defaults section"}, + {"version: 1\noutput:\n dir: x\ntargets:\n - id: a\n format: txt\n count: 1\n size: 1\n", true, "an output section"}, + } { + err := recipe.CheckBase([]byte(c.src), "base") + if (err != nil) != c.bad { + t.Errorf("CheckBase on a base carrying %s: got %v, and the rule says refused=%v", c.why, err, c.bad) + } + } +} + +// A recipe built on a preset and the ejected preset with the same targets +// appended produce the same bytes, and only the first records the preset. +// +// Both roads go through the command line, as a person would take them: one +// file says extends and with, the other is what "tfg preset eject" printed +// with the same batch typed under it and the same seed and defaults above. +// The files are compared byte for byte and the two manifests' records read: +// run.preset is present on the first road, with exactly the parameters the +// file left out listed as defaulted, and absent on the second, because that +// recipe stands alone. The recipe hashes differ, and that is asserted rather +// than avoided - they are two files, and the owner's decision is that the +// hash names the file that was run. +func TestARecipeBuildingOnAPresetGivesTheBytesOfTheEjectedOneWithItsTargetsAppended(t *testing.T) { + root := t.TempDir() + fromExtends := filepath.Join(root, "extends") + fromEject := filepath.Join(root, "eject") + + own := " - id: mine\n format: txt\n count: 2\n size: 100\n" + extending := filepath.Join(root, "extending.yaml") + if err := os.WriteFile(extending, []byte( + "version: 1\nseed: 7\ndefaults:\n label: false\n"+ + "extends: preset:size-boundaries\nwith:\n limit: 4mb\n format: txt\n"+ + "targets:\n"+own), 0o644); err != nil { + t.Fatal(err) + } + + var source, notes bytes.Buffer + if code := cli.Run(context.Background(), []string{ + "preset", "eject", "size-boundaries", "--limit", "4mb", "--format", "txt", + }, &source, ¬es); code != cli.ExitOK { + t.Fatalf("eject ended with %d: %s", code, notes.String()) + } + ejected := filepath.Join(root, "ejected.yaml") + if err := os.WriteFile(ejected, append(source.Bytes(), []byte(own+"seed: 7\ndefaults:\n label: false\n")...), 0o644); err != nil { + t.Fatal(err) + } + + a := runAndReadManifest(t, fromExtends, []string{"generate", extending, "--out", fromExtends}) + b := runAndReadManifest(t, fromEject, []string{"generate", ejected, "--out", fromEject}) + + if a.Run.Preset == nil { + t.Fatal("the run built on a preset recorded no run.preset, so the manifest does not say where the set came from") + } + if got, want := a.Run.Preset.ID, "size-boundaries"; got != want { + t.Errorf("run.preset.id is %q, want %q", got, want) + } + // Two parameters were given and one was not, so exactly one stood in. + if got := strings.Join(a.Run.Preset.Defaulted, ","); got != "spread" { + t.Errorf("run.preset.defaulted is %q, and the file gave every parameter but spread", got) + } + if b.Run.Preset != nil { + t.Errorf("the run from the ejected recipe recorded run.preset %+v, and that recipe stands alone", b.Run.Preset) + } + if a.Run.RecipeHash == b.Run.RecipeHash { + t.Errorf("both runs recorded the hash %s, and they were two different files", a.Run.RecipeHash) + } + + // The preset's targets first and the file's own after them, in the + // manifest as in the recipe - the order is part of what a consumer + // reads, and swapping it would leave every byte in place. + if first, last := a.Files[0].TargetID, a.Files[len(a.Files)-1].TargetID; first != "under_1mb" || last != "mine" { + t.Errorf("the manifest lists %s first and %s last, and the preset's targets come before the file's own", first, last) + } + + // The bytes themselves. Nine files, seven from the preset and two typed. + compared := 0 + for _, f := range a.Files { + x, err := os.ReadFile(filepath.Join(fromExtends, f.Path)) + if err != nil { + t.Fatal(err) + } + y, err := os.ReadFile(filepath.Join(fromEject, f.Path)) + if err != nil { + t.Errorf("%s came from the recipe building on the preset and not from the ejected one", f.Path) + continue + } + if !bytes.Equal(x, y) { + t.Errorf("%s differs between the two roads, so building on a preset is not the same run as ejecting it", f.Path) + } + compared++ + } + if compared != 9 { + t.Fatalf("%d files were compared and the two recipes describe nine", compared) + } +} + +// The batch screen, with the switch on, produces the bytes the file produces. +// +// The window is the third road to the same run, and it is pressed rather +// than looked at: the switch is turned on, the limit typed, the format +// chosen, one batch filled in, Generate pressed, and the manifest read back +// and compared with the one the command line wrote from the equivalent file. +// A screen that drew the section and dropped the choice on the way to the +// engine would produce seven files fewer and a different record. +func TestARecipeBuiltOnAPresetFromTheWindowGivesTheBytesTheFileGives(t *testing.T) { + root := t.TempDir() + fromWindow := filepath.Join(root, "window") + fromFile := filepath.Join(root, "file") + + host := newFakeHost(t) + screen := window.NewRecipe(host) + content := screen.Object() + t.Cleanup(func() { join(host) }) + + // Turned on through the control itself, which fires the same change a + // press does and rebuilds the form with the preset's fields on it. + switchOn := checkNamed(content, text.FieldBuildOnPreset()) + if switchOn == nil { + t.Fatal("there is no switch to build on a preset on the batch screen") + } + switchOn.SetChecked(true) + fields := screen.Fields() + setBox(t, fields, recipe.KeyWith+".limit", "4mb") + chooserIn(t, fields, recipe.KeyWith+".format").SetSelected("txt") + setBox(t, fields, recipe.TargetAddress(1, recipe.KeyID), "mine") + chooserIn(t, fields, recipe.TargetAddress(1, recipe.KeyFormat)).SetSelected("txt") + setBox(t, fields, recipe.TargetAddress(1, recipe.KeySize), "100") + setBox(t, fields, recipe.KeySeed, "7") + entryUnder(t, content, text.FieldOutputDir()).SetText(fromWindow) + press(t, content, text.ButtonGenerate()) + join(host) + + // No defaults section: the label switch on this screen starts on, as a + // recipe file with no such section has it. It started off until + // 2026-09-22, and this guard was what showed it - every byte differed + // between the two roads for a reason that was not this section's (O231). + file := filepath.Join(root, "same.yaml") + if err := os.WriteFile(file, []byte( + "version: 1\nseed: 7\nextends: preset:size-boundaries\nwith:\n limit: 4mb\n format: txt\n"+ + "targets:\n - id: mine\n format: txt\n count: 1\n size: 100\n"), 0o644); err != nil { + t.Fatal(err) + } + b := runAndReadManifest(t, fromFile, []string{"generate", file, "--out", fromFile}) + a := wholeManifest(t, filepath.Join(fromWindow, "manifest.json")) + + if a.Run.Preset == nil || a.Run.Preset.ID != "size-boundaries" { + t.Fatalf("the window recorded run.preset %+v, so the switch never reached the recipe", a.Run.Preset) + } + if got := strings.Join(a.Run.Preset.Defaulted, ","); got != "spread" { + t.Errorf("the window recorded defaulted %q, and only spread was left alone", got) + } + if len(a.Files) != len(b.Files) || len(a.Files) != 8 { + t.Fatalf("the window wrote %d files and the file %d, and both describe eight", len(a.Files), len(b.Files)) + } + for i := range a.Files { + if a.Files[i].Path != b.Files[i].Path || a.Files[i].Hashes.SHA256 != b.Files[i].Hashes.SHA256 { + t.Errorf("file %d: the window wrote %s %s and the file %s %s", i, + a.Files[i].Path, a.Files[i].Hashes.SHA256, b.Files[i].Path, b.Files[i].Hashes.SHA256) + } + } +} + +// The batch screen can run a preset's set alone: the switch on, the only +// batch removed, and Generate pressed. +// +// A recipe of extends and nothing else is legal - it is a preset run kept in +// a repository - and the screen could not produce one until an outside +// review of #119 said so: the last batch had no Remove button. Pressed +// rather than looked at, and the manifest read back: seven files, all the +// preset's, and the preset recorded. Then the switch goes off again and a +// batch is back, because a form with nothing on it can produce nothing. +func TestTheBatchScreenCanRunAPresetsSetAlone(t *testing.T) { + dir := t.TempDir() + host := newFakeHost(t) + screen := window.NewRecipe(host) + content := screen.Object() + t.Cleanup(func() { join(host) }) + + if buttonNamed(content, text.ButtonRemoveBatch()) != nil { + t.Fatal("the only batch offers Remove before the switch is on, and a screen with nothing on it can produce nothing") + } + switchOn := checkNamed(content, text.FieldBuildOnPreset()) + switchOn.SetChecked(true) + press(t, content, text.ButtonRemoveBatch()) + if screen.FirstField() != switchOn { + t.Error("with no batch left, the keyboard does not start at the switch") + } + + fields := screen.Fields() + setBox(t, fields, recipe.KeyWith+".limit", "4mb") + chooserIn(t, fields, recipe.KeyWith+".format").SetSelected("txt") + entryUnder(t, content, text.FieldOutputDir()).SetText(dir) + press(t, content, text.ButtonGenerate()) + join(host) + + m := wholeManifest(t, filepath.Join(dir, "manifest.json")) + if len(m.Files) != 7 { + t.Fatalf("the window wrote %d files, and the preset's set alone is seven", len(m.Files)) + } + if m.Run.Preset == nil || m.Run.Preset.ID != "size-boundaries" { + t.Errorf("the manifest records run.preset %+v", m.Run.Preset) + } + + switchOn.SetChecked(false) + if screen.FirstField() == switchOn { + t.Error("with the switch off and no batch, the screen has nothing to produce and no batch came back") + } +} + +// Every refusal about the preset side of a recipe names the line it is about. +// +// A screen marks a box by the address a refusal carries and the command line +// lists it, so a refusal about with.limit that arrived addressed to nothing +// would fall to the foot of the form. Each case is one thing a person can +// write wrong, with the address and a phrase the refusal has to carry. +func TestEveryRefusalAboutThePresetSideOfARecipeNamesItsLine(t *testing.T) { + for _, c := range []struct { + name string + src string + at string + says string + }{ + {"an unknown preset", "version: 1\nextends: preset:nope\n", "extends", "this build does not have"}, + {"a scheme this build lacks", "version: 1\nextends: other.yaml\n", "extends", "does not name a preset"}, + {"with and no extends", "version: 1\nwith:\n limit: 1kb\ntargets:\n - id: a\n format: txt\n count: 1\n size: 1\n", "with", "no preset is named"}, + {"an unknown parameter", "version: 1\nextends: preset:size-boundaries\nwith:\n spreed: 1kb\n", "with.spreed", "does not take"}, + {"a value the parameter refuses", "version: 1\nextends: preset:size-boundaries\nwith:\n limit: banana\n", "with.limit", "cannot be \"banana\""}, + {"a set the preset cannot build", "version: 1\nextends: preset:size-boundaries\nwith:\n limit: 1kb\n", "with.limit", "cannot build this set"}, + {"a parameter written as a list", "version: 1\nextends: preset:size-boundaries\nwith:\n spread: [1kb, 2kb]\n", "with.spread", "written as a list"}, + {"a format this build lacks", "version: 1\nextends: preset:size-boundaries\nwith:\n format: nope\n", "with.format", "does not have"}, + {"an id the preset already uses", "version: 1\nextends: preset:size-boundaries\ntargets:\n - id: at_limit\n format: txt\n count: 1\n size: 1\n", "targets[1].id", "already builds"}, + {"a preset's targets handed to a plain reader", "version: 1\nextends: preset:size-boundaries\n", "extends", "were not supplied"}, + } { + t.Run(c.name, func(t *testing.T) { + var err error + if strings.HasPrefix(c.name, "a preset's targets handed") { + _, err = recipe.Parse([]byte(c.src), c.name) + } else { + _, err = preset.ReadRecipe([]byte(c.src), c.name) + } + var invalid *recipe.ValidationError + if !errors.As(err, &invalid) { + t.Fatalf("got %v, and this recipe has to be refused as invalid", err) + } + found := false + for _, p := range invalid.Problems { + if p.At == c.at && strings.Contains(p.String(), c.says) { + found = true + } + } + if !found { + t.Errorf("no problem is addressed to %q saying %q. The problems are:\n%v", c.at, c.says, err) + } + for _, p := range invalid.Problems { + if p.Why == "" || p.Fix == "" { + t.Errorf("the problem %q arrives without all four parts (D6): why=%q fix=%q", p.What, p.Why, p.Fix) + } + } + // One sentence, not two. A file of extends alone handed to the + // plain reader used to collect a second refusal about asking for + // no files, which contradicts the README - such a recipe is legal. + // Pointed out by an outside review of #119. + if len(invalid.Problems) != 1 { + t.Errorf("%d problems, and this case has exactly one thing wrong:\n%v", len(invalid.Problems), err) + } + }) + } +} + +// A recipe file is read through the door that knows presets, on both surfaces. +// +// recipe.Parse cannot expand a preset and refuses a file that names one, so +// a surface reading a file through it would turn away every recipe built on +// a preset with a sentence about a reader. The three places that read +// expansions rather than files - the budget of a preset, a --preset run, and +// the preset screen - are named by file, and everything else in cli and gui +// has to go through preset.ReadRecipe. Read from the source, the way the +// layer guard reads imports. +func TestEveryRecipeFileIsReadThroughTheDoorThatKnowsPresets(t *testing.T) { + root := repoRoot(t) + // The files that parse an EXPANSION, which has no extends by the rule + // above, and so may use Parse. + allowed := map[string]bool{ + filepath.Join("internal", "cli", "preset.go"): true, + filepath.Join("internal", "gui", "window", "preset.go"): true, + } + seen := 0 + for _, dir := range []string{"internal/cli", "internal/gui"} { + err := filepath.Walk(filepath.Join(root, dir), func(path string, info os.FileInfo, err error) error { + if err != nil || info.IsDir() || !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") { + return err + } + body, err := os.ReadFile(path) + if err != nil { + return err + } + rel, _ := filepath.Rel(root, path) + for i, line := range strings.Split(string(body), "\n") { + if !strings.Contains(line, "recipe.Parse(") || strings.HasPrefix(strings.TrimSpace(line), "//") { + continue + } + seen++ + if allowed[rel] { + continue + } + t.Errorf("%s:%d reads a recipe through recipe.Parse, which cannot expand the preset a "+ + "file may build on. Read it through preset.ReadRecipe, which can.", rel, i+1) + } + return nil + }) + if err != nil { + t.Fatal(err) + } + } + // The three allowed call sites are read today. A walk that found none + // would have stopped seeing the call rather than proved the rule. + if seen < 3 { + t.Errorf("%d calls of recipe.Parse were found under cli and gui, and there are three that parse expansions - "+ + "either they moved or the way this reads them stopped working", seen) + } +} + +// runAndReadManifest runs the command line and reads back what it wrote. +func runAndReadManifest(t *testing.T, dir string, args []string) manifest.Manifest { + t.Helper() + var out, errOut bytes.Buffer + if code := cli.Run(context.Background(), args, &out, &errOut); code != cli.ExitOK { + t.Fatalf("%v ended with %d: %s", args, code, errOut.String()) + } + return wholeManifest(t, filepath.Join(dir, "manifest.json")) +} + +// wholeManifest reads a manifest as the manifest package spells it, the whole +// of it rather than the narrow view the recipe guards share - run.preset is +// what these guards ask about and that view does not carry it. +func wholeManifest(t *testing.T, path string) manifest.Manifest { + t.Helper() + raw, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + var m manifest.Manifest + if err := json.Unmarshal(raw, &m); err != nil { + t.Fatal(err) + } + return m +} diff --git a/internal/guard/guitext_test.go b/internal/guard/guitext_test.go index d0578257..2be0293f 100644 --- a/internal/guard/guitext_test.go +++ b/internal/guard/guitext_test.go @@ -78,7 +78,8 @@ var notWords = map[string]string{ `": "`: "what joins a file name to what the system said about it, in an error nobody reads as prose", `"GetSystemDirectoryW"`: "the Windows entry point that says where the system keeps its own " + "libraries, asked for by name because that is how the loader takes it", - `"preset"`: "the key the preset field is registered under, not a label", + `"preset"`: "the key the preset field is registered under, not a label", + `"start_from_preset"`: "the key the batch screen's switch to build on a preset is registered under, not a label", `"outputDirectory"`: "the name the window files the last output directory under, never shown. " + "Translating a storage key would lose what was kept the day somebody changed language", `"windowWidth"`: "the name the window files its width under, never shown", diff --git a/internal/guard/parity_test.go b/internal/guard/parity_test.go index 6de453d4..acddc7eb 100644 --- a/internal/guard/parity_test.go +++ b/internal/guard/parity_test.go @@ -65,6 +65,16 @@ var reachableFromTheWindow = []string{ // menu. "preset:size-boundaries.format", + // A recipe that builds on a preset, since 2026-09-22: the switch and the + // menu on the batch screen are the extends key, and the chosen preset's + // parameters under it are the with section, drawn from the declaration + // the way the preset screen draws them. Pressed rather than looked at: + // TestARecipeBuiltOnAPresetFromTheWindowGivesTheBytesTheFileGives runs + // the same recipe from the batch screen and from a file and compares + // the manifests. + "recipe:extends", + "recipe:with", + // What to break about the files, drawn from the damage registry rather // than listed in the window, with the parameters of whatever is chosen // drawn by the same call that draws a format's settings. @@ -229,15 +239,16 @@ var reachableFromTheWindow = []string{ // parity, written down rather than estimated. // // Some entries are here for a second reason - the engine refuses them too, so -// neither surface has them. extends, with, policy, engine, targets.fill, -// defaults.fill and output.split_threshold are all answered today with "not in -// this build yet". +// neither surface has them. policy, engine, targets.fill, defaults.fill and +// output.split_threshold are all answered today with "not in this build yet". // -// That is seven, and it was eight until 2026-09-09: targets.mutations was -// refused with a message pointing at a module that will never exist, and it is -// now targets.damage, which both surfaces reach. It left this list rather than -// moving down it, which is what this list is for - the distance to parity is -// only allowed to shrink. +// That is five. It was seven until 2026-09-22, when extends and with were +// built - engine, command line and the batch screen's base section in one +// change - and both left this list. It was eight until 2026-09-09: +// targets.mutations was refused with a message pointing at a module that +// will never exist, and it is now targets.damage, which both surfaces reach. +// Each left this list rather than moving down it, which is what this list is +// for - the distance to parity is only allowed to shrink. // // The sentence said seven once before, until 2026-08-18, and was wrong: it had // left out output.split_threshold, which recipe.go has refused all along. @@ -252,13 +263,11 @@ var notYetReachable = []string{ "recipe:allow_nondeterministic", "recipe:defaults.fill", "recipe:engine", - "recipe:extends", "recipe:locale", "recipe:output.split_threshold", "recipe:policy", "recipe:targets.fill", "recipe:version", - "recipe:with", } // capabilities is everything the engine can be asked for, gathered from the diff --git a/internal/guard/screenpixels_test.go b/internal/guard/screenpixels_test.go index 7d22a222..3ba9be39 100644 --- a/internal/guard/screenpixels_test.go +++ b/internal/guard/screenpixels_test.go @@ -485,6 +485,12 @@ func screenScenes() []screenScene { chooseFormat(t, s.tab, "zip") pressNamed(t, s.tab, text.ButtonAddContents()) }}, + // A recipe built on a preset, since 2026-09-22: the switch on, and + // the preset's menu and parameters drawn under it. The state the + // section exists for, and the one a fresh screen never shows. + {name: "recipe-on-a-preset", tab: text.TabRecipe(), set: func(t *testing.T, s scene) { + flipSwitch(t, s.canvas, s.tab, text.FieldBuildOnPreset()) + }}, } } diff --git a/internal/guard/settingslot_test.go b/internal/guard/settingslot_test.go index 876b2808..c61fa8fc 100644 --- a/internal/guard/settingslot_test.go +++ b/internal/guard/settingslot_test.go @@ -186,6 +186,10 @@ func TestEveryNameARefusalCanBeGivenTakesTheArticleThisRuleGivesIt(t *testing.T) "paragraphs": "a", "rows": "a", "columns": "a", "slides": "a", "depth": "a", "colours": "a", "records": "a", "lines": "a", "damage": "a", "bytes": "a", + // The batch screen's base section: the switch, and the two recipe + // keys behind it. The parameters under with. arrive as the + // preset's own names, which are above. + "start_from_preset": "a", "extends": "an", "with": "a", // Labels, which is what a window shows. "Batch name": "a", "How many files": "a", "File names": "a", "Size": "a", "Damage": "a", @@ -194,7 +198,8 @@ func TestEveryNameARefusalCanBeGivenTakesTheArticleThisRuleGivesIt(t *testing.T) "Limit to test": "a", "One size": "a", "A range": "a", "Rule being tested": "a", "Manifest file name": "a", "Preset": "a", "Limit": "a", "Spread": "a", "Width": "a", "Height": "a", "Quality": "a", - "Label in each file": "a", + "Label in each file": "a", + "Start from a preset": "a", "Preset to build on": "a", } check := func(name, source string) { diff --git a/internal/guard/testdata/screens/recipe-contents.png b/internal/guard/testdata/screens/recipe-contents.png index dde01231..78573b5b 100644 Binary files a/internal/guard/testdata/screens/recipe-contents.png and b/internal/guard/testdata/screens/recipe-contents.png differ diff --git a/internal/guard/testdata/screens/recipe-contents.xml b/internal/guard/testdata/screens/recipe-contents.xml index 1cf271c8..880648f6 100644 --- a/internal/guard/testdata/screens/recipe-contents.xml +++ b/internal/guard/testdata/screens/recipe-contents.xml @@ -32,8 +32,8 @@ - - + + @@ -56,7 +56,44 @@ - + + + + + + Build on a preset + + + + + With the switch on, the chosen preset's files come first and the batches below are added after them. With it off, the batches are the whole + recipe. + + + + + + + + Start from a preset + + + + + + + + + + + + + + + + + + @@ -412,7 +449,7 @@ - + @@ -529,7 +566,8 @@ - + + @@ -543,8 +581,8 @@ - - + + diff --git a/internal/guard/testdata/screens/recipe-on-a-preset.png b/internal/guard/testdata/screens/recipe-on-a-preset.png new file mode 100644 index 00000000..77498223 Binary files /dev/null and b/internal/guard/testdata/screens/recipe-on-a-preset.png differ diff --git a/internal/guard/testdata/screens/recipe-on-a-preset.xml b/internal/guard/testdata/screens/recipe-on-a-preset.xml new file mode 100644 index 00000000..a643567d --- /dev/null +++ b/internal/guard/testdata/screens/recipe-on-a-preset.xml @@ -0,0 +1,694 @@ + + + + + + + + + + + Single batch + + + + + Presets + + + + + Several batches + + + + + About + + + + + + + + + + + + + + + + + Several batches + + + + + + + + + Batches of different formats and sizes, generated together in one run. + + + + + + + + + + + + Build on a preset + + + + + With the switch on, the chosen preset's files come first and the batches below are added after them. With it off, the batches are the whole + recipe. + + + + + + + + Start from a preset + + + + + + + + + + + + + + + + + + Preset to build on + + + + + + + + + + + + + size-boundaries + + + + + + + + + + + + + + Limit + + + + + + + + + + + + + + + 10mb + + + + + + + + + + + + + + + + + + + + + + + + + Spread + + + + + + + + + + + + + + + 1B,1kb,1mb + + + + + + + + + + + + + + + + Format + + + + + + + + + + + + + pdf + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Batch 1 + + + + + + + + + + + + + + + Duplicate + + + + + Remove + + + + + + + + + Format + + + + + + + + + + + + + avif + + + + + + + + + + + + + + Batch name + + * + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + How many files + + + + + + + + + 1 + + + + + + + + + + + + + + + + + + + + One size + A range + Around a limit + + + + + + Size + + * + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + File names + + + + + + + + + + + + + + + files_0001 + + + + + + + + + + + + + + + + + + + + + + + Settings for avif + + + + + + + + + + + + + + + + + + + + + + + + + + Notes for the manifest + + + + + + + + + + + + + + + + + + + + + + + + + + + Output + + + + Output directory + + * + + + + + + + + + + + + + + + + /tfg/out + + + + + + + + Choose... + + + + + + + + + + Manifest file name + + + + + + + + + + + + + + + manifest.json + + + + + + + + + + + + + + + + Seed + + + + + + + + + + + + + + + 0 + + + + + + + + + + + + + + + + Label in each file + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Preview + + + + + Generate + + + + + + + + + + + Files will go to /tfg/out + + + + + + + + + + + + + + + + Donate + + + + + + + Add a batch + + + + + + + + + + + + + diff --git a/internal/guard/testdata/screens/recipe-refused-with-one-batch-filled.png b/internal/guard/testdata/screens/recipe-refused-with-one-batch-filled.png index a0dc7afa..39ad9c8b 100644 Binary files a/internal/guard/testdata/screens/recipe-refused-with-one-batch-filled.png and b/internal/guard/testdata/screens/recipe-refused-with-one-batch-filled.png differ diff --git a/internal/guard/testdata/screens/recipe-refused-with-one-batch-filled.xml b/internal/guard/testdata/screens/recipe-refused-with-one-batch-filled.xml index 90b4e8ef..830d0855 100644 --- a/internal/guard/testdata/screens/recipe-refused-with-one-batch-filled.xml +++ b/internal/guard/testdata/screens/recipe-refused-with-one-batch-filled.xml @@ -32,8 +32,8 @@ - - + + @@ -56,7 +56,44 @@ - + + + + + + Build on a preset + + + + + With the switch on, the chosen preset's files come first and the batches below are added after them. With it off, the batches are the whole + recipe. + + + + + + + + Start from a preset + + + + + + + + + + + + + + + + + + @@ -615,7 +652,7 @@ - + @@ -732,7 +769,8 @@ - + + @@ -742,12 +780,15 @@ + + + - - + + diff --git a/internal/guard/testdata/screens/recipe-refused.png b/internal/guard/testdata/screens/recipe-refused.png index a3c1b564..3cfe564e 100644 Binary files a/internal/guard/testdata/screens/recipe-refused.png and b/internal/guard/testdata/screens/recipe-refused.png differ diff --git a/internal/guard/testdata/screens/recipe-refused.xml b/internal/guard/testdata/screens/recipe-refused.xml index bcb8fc91..22874c4b 100644 --- a/internal/guard/testdata/screens/recipe-refused.xml +++ b/internal/guard/testdata/screens/recipe-refused.xml @@ -32,8 +32,8 @@ - - + + @@ -56,7 +56,44 @@ - + + + + + + Build on a preset + + + + + With the switch on, the chosen preset's files come first and the batches below are added after them. With it off, the batches are the whole + recipe. + + + + + + + + Start from a preset + + + + + + + + + + + + + + + + + + @@ -349,7 +386,7 @@ - + @@ -466,7 +503,8 @@ - + + @@ -476,12 +514,15 @@ + + + - - + + diff --git a/internal/guard/testdata/screens/recipe-two-batches.png b/internal/guard/testdata/screens/recipe-two-batches.png index 28b6f55f..831af43c 100644 Binary files a/internal/guard/testdata/screens/recipe-two-batches.png and b/internal/guard/testdata/screens/recipe-two-batches.png differ diff --git a/internal/guard/testdata/screens/recipe-two-batches.xml b/internal/guard/testdata/screens/recipe-two-batches.xml index 0aaf64c6..504ca7af 100644 --- a/internal/guard/testdata/screens/recipe-two-batches.xml +++ b/internal/guard/testdata/screens/recipe-two-batches.xml @@ -32,8 +32,8 @@ - - + + @@ -56,7 +56,44 @@ - + + + + + + Build on a preset + + + + + With the switch on, the chosen preset's files come first and the batches below are added after them. With it off, the batches are the whole + recipe. + + + + + + + + Start from a preset + + + + + + + + + + + + + + + + + + @@ -594,7 +631,7 @@ - + @@ -711,7 +748,8 @@ - + + @@ -725,8 +763,8 @@ - - + + diff --git a/internal/guard/testdata/screens/recipe.png b/internal/guard/testdata/screens/recipe.png index 5dcf93c9..92299f9e 100644 Binary files a/internal/guard/testdata/screens/recipe.png and b/internal/guard/testdata/screens/recipe.png differ diff --git a/internal/guard/testdata/screens/recipe.xml b/internal/guard/testdata/screens/recipe.xml index 55c666c4..79eab4fe 100644 --- a/internal/guard/testdata/screens/recipe.xml +++ b/internal/guard/testdata/screens/recipe.xml @@ -32,8 +32,8 @@ - - + + @@ -56,7 +56,44 @@ - + + + + + + Build on a preset + + + + + With the switch on, the chosen preset's files come first and the batches below are added after them. With it off, the batches are the whole + recipe. + + + + + + + + Start from a preset + + + + + + + + + + + + + + + + + + @@ -322,7 +359,7 @@ - + @@ -439,7 +476,8 @@ - + + @@ -449,6 +487,14 @@ + + + + + + + + diff --git a/internal/gui/text/locale/en.json b/internal/gui/text/locale/en.json index 267c51d1..bf8eff1f 100644 --- a/internal/gui/text/locale/en.json +++ b/internal/gui/text/locale/en.json @@ -63,10 +63,18 @@ "description": "Shown in the window. Carries one value, {{.Damage}}, which has to stay spelled exactly that way.", "other": "Settings for {{.Damage}}" }, + "DetailBasePreset": { + "description": "The longer explanation behind the button beside a field name.", + "other": "The settings under it are the preset's own. One left empty takes its default, and the manifest records which ones did." + }, "DetailBoundary": { "description": "The longer explanation behind the button beside a field name.", "other": "Give the limit your system declares, as 10mb. Units count in 1024s, and the run prints the number it used." }, + "DetailBuildOnPreset": { + "description": "The longer explanation behind the button beside a field name.", + "other": "Off, the batches below are the whole recipe. On, the chosen preset's files come first and the batches are added after them." + }, "DetailDamage": { "description": "The longer explanation behind the button beside a field name.", "other": "The files come out the size you asked for and no reader will accept them, which is what a validator has to reject. The manifest records what was broken and says the file is expected to be rejected." @@ -127,10 +135,18 @@ "description": "Shown in the window. Carries these values, each of which has to stay spelled exactly that way: {{.Directory}}, {{.Free}}.", "other": "{{.Directory}} ({{.Free}} free)" }, + "FieldBasePreset": { + "description": "The name above a box somebody fills in.", + "other": "Preset to build on" + }, "FieldBoundary": { "description": "The name above a box somebody fills in.", "other": "Limit to test" }, + "FieldBuildOnPreset": { + "description": "The name above a box somebody fills in.", + "other": "Start from a preset" + }, "FieldCount": { "description": "The name above a box somebody fills in.", "other": "How many files" @@ -196,6 +212,10 @@ "one": "1 file", "other": "{{.Count}} files" }, + "HintBasePreset": { + "description": "The line under a field name, saying what the field does.", + "other": "Its files come first." + }, "HintBoundary": { "description": "The line under a field name, saying what the field does.", "other": "Three files: one byte under the limit, one on it, one over." @@ -268,6 +288,10 @@ "description": "Shown in the window. Carries these values, each of which has to stay spelled exactly that way: {{.Field}}, {{.Value}}.", "other": "{{.Field}} is {{.Value}}, which is not a whole number. Write the digits out, such as 1 or 500" }, + "NoteBase": { + "description": "Shown in the window.", + "other": "With the switch on, the chosen preset's files come first and the batches below are added after them. With it off, the batches are the whole recipe." + }, "NoteManifestOnly": { "description": "Shown in the window.", "other": "These describe the case. They go into the manifest and change nothing in the files." @@ -332,6 +356,10 @@ "description": "About the software renderer shipped beside the program on Windows: in the system dialog and on standard error when the window could not be opened even so, on standard error when a window starts drawing with it, and on the About screen while it does.", "other": "The first attempt to open a window did not succeed - usually a graphics driver without OpenGL 2.1. Starting again with the software renderer shipped beside the program." }, + "SectionBase": { + "description": "The heading over a group of fields.", + "other": "Build on a preset" + }, "SectionCarriedBeside": { "description": "The heading over a group of fields.", "other": "Shipped beside the program on Windows" diff --git a/internal/gui/text/screens.go b/internal/gui/text/screens.go index b293cb70..60b37318 100644 --- a/internal/gui/text/screens.go +++ b/internal/gui/text/screens.go @@ -489,6 +489,41 @@ func NoteManifestOnly() string { "These describe the case. They go into the manifest and change nothing in the files.") } +// SectionBase heads the part of the recipe screen that builds on a preset. +// +// A recipe may start from a preset's set and add its own batches after it - +// the recipe key is extends, and this is the same thing on the screen. It +// stands above the batches because that is the order the run takes them in: +// the preset's targets first, then the batches. +func SectionBase() string { return say("SectionBase", "Build on a preset") } + +// NoteBase is the sentence inside that section. It describes the switch, +// because the switch is the state: a preset is always chosen once the menu +// is there, and "unchosen" - which the first wording said - was a state the +// screen does not have. Pointed out by an outside review of #119. +func NoteBase() string { + return say("NoteBase", + "With the switch on, the chosen preset's files come first and the batches below are added after them. With it off, the batches are the whole recipe.") +} + +// FieldBuildOnPreset names the switch that turns the section on. Off is the +// ordinary recipe, on is one that carries extends. +func FieldBuildOnPreset() string { return say("FieldBuildOnPreset", "Start from a preset") } +func DetailBuildOnPreset() string { + return say("DetailBuildOnPreset", + "Off, the batches below are the whole recipe. On, the chosen preset's files come first and the batches are added after them.") +} + +// FieldBasePreset names the box choosing the preset to build on. A different +// name from the preset screen's, because that one asks which set to make and +// this one asks what to make a set on top of. +func FieldBasePreset() string { return say("FieldBasePreset", "Preset to build on") } +func HintBasePreset() string { return say("HintBasePreset", "Its files come first.") } +func DetailBasePreset() string { + return say("DetailBasePreset", + "The settings under it are the preset's own. One left empty takes its default, and the manifest records which ones did.") +} + // Buttons on the recipe screen. func ButtonAddBatch() string { return say("ButtonAddBatch", "Add a batch") } func ButtonRemoveBatch() string { return say("ButtonRemoveBatch", "Remove") } diff --git a/internal/gui/window/recipe.go b/internal/gui/window/recipe.go index 2690266e..f26409d2 100644 --- a/internal/gui/window/recipe.go +++ b/internal/gui/window/recipe.go @@ -10,6 +10,8 @@ import ( "github.com/donislawdev/TestingFilesGenerator/internal/format" "github.com/donislawdev/TestingFilesGenerator/internal/gui/parts" "github.com/donislawdev/TestingFilesGenerator/internal/gui/text" + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" + "github.com/donislawdev/TestingFilesGenerator/internal/preset" "github.com/donislawdev/TestingFilesGenerator/internal/recipe" ) @@ -56,13 +58,18 @@ type Recipe struct { host Host + // base is the preset the recipe builds on, if it builds on one. Drawn + // above the batches because the run takes the preset's targets first. + base *base + batches []*batch - // batchBox and outBox are refilled together by rebuild, and they have to be: + // baseBox, batchBox and outBox are refilled together by rebuild, and they have to be: // the address of a setting carries the position of its batch, so the whole // registry is built again whenever the list changes. A section built once and // left alone would hold controls the registry had forgotten, which is a // control whose refusal has nowhere to go. + baseBox *fyne.Container batchBox *fyne.Container outBox *fyne.Container @@ -176,9 +183,16 @@ func NewRecipe(host Host, links ...fyne.CanvasObject) *Recipe { // must fill this in" looked the same. See the note on newBatch. r.seed.SetPlaceHolder(text.PlaceholderLeftEmpty(strconv.Itoa(recipe.DefaultSeed))) r.label = parts.NewToggle(nil) + // On, as on the single batch screen and as in a recipe file with no + // defaults section. It started off until 2026-09-22, so the three + // surfaces had two defaults and the same recipe gave different bytes + // from this screen - found by the guard that compares them, O231. + r.label.SetChecked(true) + r.baseBox = parts.FieldColumn() r.batchBox = parts.FieldColumn() r.outBox = parts.FieldColumn() + r.base = newBase(r) r.batches = []*batch{r.newBatch()} // In the bar rather than in the list, so the one control that makes this @@ -192,7 +206,7 @@ func NewRecipe(host Host, links ...fyne.CanvasObject) *Recipe { nil, r.footer(rail(append([]fyne.CanvasObject{donateButton(host), parts.Divider(), r.addBtn}, links...)...)), nil, nil, - (r.keepScroll(container.NewVScroll(parts.Screen(parts.Titled(text.TabRecipe(), text.SubtitleRecipe()), r.batchBox, r.outBox)))), + (r.keepScroll(container.NewVScroll(parts.Screen(parts.Titled(text.TabRecipe(), text.SubtitleRecipe()), r.baseBox, r.batchBox, r.outBox)))), )) // The format of the first batch has to be chosen for its declared settings @@ -215,9 +229,15 @@ func NewRecipe(host Host, links ...fyne.CanvasObject) *Recipe { // Object is the screen, to put in the window. func (r *Recipe) Object() fyne.CanvasObject { return r.body } -// FirstField is where the keyboard starts: the format of the first batch. There -// is always a first batch - the last one cannot be removed. -func (r *Recipe) FirstField() fyne.Focusable { return r.batches[0].formatPick } +// FirstField is where the keyboard starts: the format of the first batch, +// or the switch above it on a screen that has removed its last batch to run +// a preset's set alone - see base.carriesTheRun. +func (r *Recipe) FirstField() fyne.Focusable { + if len(r.batches) == 0 { + return r.base.on + } + return r.batches[0].formatPick +} // OutDir is where this screen would write, for the screen somebody moves to. func (r *Recipe) OutDir() string { return r.outDir.Text } @@ -299,9 +319,14 @@ func (r *Recipe) newBatch() *batch { // without a second place remembering which index a widget belongs to. func (r *Recipe) rebuild() { r.fields.KeepFirst(0) + r.baseBox.RemoveAll() r.batchBox.RemoveAll() r.outBox.RemoveAll() + // Before the batches, so that Tab walks the screen in the order it is + // read and the order the run takes the targets in. + r.baseBox.Add(r.base.section(r.fields, r.tips)) + panels := make([]fyne.CanvasObject, 0, len(r.batches)+1) for i, b := range r.batches { panels = append(panels, r.batchBlock(i, b)) @@ -320,6 +345,7 @@ func (r *Recipe) rebuild() { // After the batches, so that Tab walks the screen in the order it is read. r.outBox.Add(r.outputSection()) + r.baseBox.Refresh() r.batchBox.Refresh() r.outBox.Refresh() // A batch added, copied or taken away changes what the form comes to, @@ -421,7 +447,7 @@ func (r *Recipe) batchBlock(index int, b *batch) fyne.CanvasObject { head := []fyne.CanvasObject{ parts.NewButton(parts.Secondary, text.ButtonDuplicateBatch(), func() { r.duplicateBatch(index) }), } - if len(r.batches) > 1 { + if len(r.batches) > 1 || r.base.carriesTheRun() { head = append(head, parts.NewButton(parts.Secondary, text.ButtonRemoveBatch(), func() { r.removeBatch(index) })) } b.fold = parts.NewFolding(text.BatchHeading(index+1), head, rows...) @@ -554,9 +580,10 @@ func (r *Recipe) addBatch() { r.rebuild() } -// removeBatch drops one batch. The last cannot go: a screen with no batches can -// produce nothing, and would answer a press with a refusal about a document -// rather than about anything anybody did. +// removeBatch drops one batch. The last cannot go, unless the screen builds +// on a preset (base.carriesTheRun): a screen with no batches and no preset +// can produce nothing, and would answer a press with a refusal about a +// document rather than about anything anybody did. // duplicateBatch copies one batch and puts the copy under it. // // Batches usually differ from each other in one setting - a size, a format, a @@ -595,7 +622,10 @@ func (r *Recipe) duplicateBatch(index int) { } func (r *Recipe) removeBatch(index int) { - if len(r.batches) <= 1 || index < 0 || index >= len(r.batches) { + if index < 0 || index >= len(r.batches) { + return + } + if len(r.batches) <= 1 && !r.base.carriesTheRun() { return } r.batches = append(r.batches[:index], r.batches[index+1:]...) @@ -644,34 +674,57 @@ func (r *Recipe) settle() ([]engine.Target, engine.Options, error) { Manifest: r.manifest.Text, Label: &r.label.Checked, } + doc.Extends, doc.With = r.base.draft() for _, b := range r.batches { doc.Targets = append(doc.Targets, b.draft()) } + // Cleared before the reading rather than after it, for the reason the + // preset screen gives: a form that does not settle carries no notes. + r.notes = nil + src, err := recipe.Compose(doc) if err != nil { return nil, none, err } - rec, err := recipe.Parse(src, text.TabRecipe()) + // Through the door that knows presets, the same one the command line + // reads a file through. A document with no extends comes back with no + // expansion and nothing below changes for it. + read, err := preset.ReadRecipe(src, text.TabRecipe()) if err != nil { return nil, none, err } + rec := read.Recipe hash, err := recipe.Hash(src) if err != nil { return nil, none, err } + // What the run has to say out loud about a preset's value nobody gave. + r.notes = read.Notes() + targets := make([]engine.Target, 0, len(rec.Targets)) for _, t := range rec.Targets { targets = append(targets, engineTarget(t)) } - return targets, engine.Options{ + opt := engine.Options{ OutDir: statedDirectory(r.outDir.Text, rec.Output.Dir), Seed: rec.Seed, Command: "tfg-gui", ManifestName: manifestName(rec.Output.Manifest), RecipeHash: hash, - }, nil + } + // Which numbers were the preset's own rather than only which preset the + // recipe built on - the same record the preset screen writes, because it + // is the same fact. + if e := read.Expansion; e != nil { + opt.Preset = &manifest.Preset{ + ID: e.Preset.ID, + Parameters: map[string]string(e.Settled), + Defaulted: e.Defaulted, + } + } + return targets, opt, nil } // statedDirectory is where this screen would write, with an emptied box left diff --git a/internal/gui/window/recipebase.go b/internal/gui/window/recipebase.go new file mode 100644 index 00000000..c5bf6a32 --- /dev/null +++ b/internal/gui/window/recipebase.go @@ -0,0 +1,167 @@ +package window + +import ( + "fyne.io/fyne/v2" + + "github.com/donislawdev/TestingFilesGenerator/internal/format" + "github.com/donislawdev/TestingFilesGenerator/internal/gui/parts" + "github.com/donislawdev/TestingFilesGenerator/internal/gui/text" + "github.com/donislawdev/TestingFilesGenerator/internal/preset" + "github.com/donislawdev/TestingFilesGenerator/internal/recipe" +) + +// The part of the batch screen that builds on a preset. +// +// A recipe may name a preset and add its own targets after the preset's - +// the extends and with keys, docs/RECIPE.md section 6 - and this is the same +// thing on the screen: a switch, a menu of presets, and the chosen preset's +// parameters drawn from its declaration the way the preset screen draws +// them. The batches below are then the recipe's own targets. +// +// A switch rather than a menu with a blank position, and the reason is what +// the screen has to be able to say. A menu that has been chosen from cannot +// be un-chosen, so a screen with only the menu could turn a preset on and +// never off again - and "off" is the ordinary state of this screen. The +// switch is that state, and the menu appears only while it is on. +// +// Its own type with its own methods rather than more of the screen's: the +// screen stood at its ceiling of methods, and what is here answers one +// question - what the recipe builds on - which the rest of the screen only +// asks. + +// base is the preset the batch screen builds on, as the screen holds it. +type base struct { + on *parts.Toggle + pick *parts.Chooser + + // declared is what the chosen preset takes - its parameters and the + // globals it reads - and params are the controls drawn from it. Both are + // replaced when the preset changes and kept across a rebuild, for the + // reason the batch's own props are: a rebuilt box has forgotten what was + // typed into it. + declared []format.Property + params []parts.PropertyField +} + +// newBase builds the controls, without placing them. The switch starts off, +// which is the recipe with no extends key. +func newBase(r *Recipe) *base { + b := &base{} + b.on = parts.NewToggle(func(on bool) { + // Off with no batch left is a form that can produce nothing, so a + // batch comes back - the one the screen opened with. + if !on && len(r.batches) == 0 { + r.addBatch() + return + } + r.rebuild() + }) + ids := preset.IDs() + b.pick = parts.NewChooser(ids, func(id string) { + if err := b.choose(id); err != nil { + // The registry filled the list, so a press cannot land here. A + // build where the two have come apart can, and saying so beats a + // section with no settings and no reason given. + r.refuse(err) + return + } + // Chosen while the screen is still being built, before the screen + // holds the base and before rebuild has anything to lay out. + if r.base != nil { + r.rebuild() + } + }) + // Chosen here rather than left empty, so a switch turned on shows a + // preset with its parameters at once rather than a menu asking to be + // opened first. + if len(ids) > 0 { + b.pick.SetSelected(ids[0]) + } + return b +} + +// choose replaces the chosen preset's settings. +func (b *base) choose(id string) error { + chosen, err := preset.Get(id) + if err != nil { + return err + } + // What the preset declares, then the globals it supplies a value for - + // the same order "tfg preset show" prints and the preset screen draws. + settings := make([]format.Property, 0, len(chosen.Parameters)+len(chosen.Reads)) + settings = append(settings, chosen.Parameters...) + settings = append(settings, chosen.Globals()...) + b.declared = settings + b.params = make([]parts.PropertyField, 0, len(settings)) + for _, p := range settings { + b.params = append(b.params, parts.FromProperty(p)) + } + return nil +} + +// carriesTheRun says whether the preset's files are part of the run, which +// is when the switch is on. While it is, the screen may stand with no batch +// at all - a recipe of extends alone is legal, a preset run kept in a +// repository - and an outside review of #119 pointed out that the screen +// could not produce one, because the last batch had no Remove button. +func (b *base) carriesTheRun() bool { return b.on.Checked } + +// section is the part as it appears on the screen, registered under the +// keys a refusal about it arrives with: extends for the preset itself, and +// with. for each parameter, which is the line in the recipe the value +// would be written on. +func (b *base) section(fields *parts.Fields, tips *parts.Tips) fyne.CanvasObject { + rows := []fyne.CanvasObject{ + parts.Note(text.NoteBase()), + fields.AddToggle(settingBuildOnPreset, text.FieldBuildOnPreset(), "", + tips.Say(text.DetailBuildOnPreset()), b.on), + } + if b.on.Checked { + rows = append(rows, fields.Add(recipe.KeyExtends, text.FieldBasePreset(), text.HintBasePreset(), + tips.Say(text.DetailBasePreset()), parts.Menu(b.pick))) + rows = append(rows, b.parameterRows(fields, tips)...) + } + return parts.Section(text.SectionBase(), rows...) +} + +// parameterRows draws the chosen preset's parameters. +func (b *base) parameterRows(fields *parts.Fields, tips *parts.Tips) []fyne.CanvasObject { + rows := make([]fyne.CanvasObject, 0, len(b.params)) + for i, f := range b.params { + d := b.declared[i] + if d.Kind == format.PropertySize { + fields.InBytes(recipe.KeyWith + "." + f.Name) + } + rows = append(rows, fields.Add(recipe.KeyWith+"."+f.Name, text.SettingLabel(f.Name), + parts.PropertyDetail(d), tips.Say(text.SettingKey(f.Name)), parts.ShapedFor(d, f.Control))) + } + return rows +} + +// settingBuildOnPreset is the key the switch goes under. +// +// Not a recipe key, because a recipe has no such setting - a recipe either +// carries extends or it does not. It is here so that the switch has a key +// like every other control rather than being the one exception, the same +// reason the preset screen gives its own menu one. +const settingBuildOnPreset = "start_from_preset" + +// draft is what the section says, ready to be composed: the preset's id when +// the switch is on, and every parameter somebody typed. A field left empty +// is left out, so the declared default stands in and the manifest records +// that it did. +func (b *base) draft() (extends string, with map[string]string) { + if !b.on.Checked { + return "", nil + } + with = map[string]string{} + for _, f := range b.params { + if v := f.Value(); v != "" { + with[f.Name] = v + } + } + if len(with) == 0 { + with = nil + } + return b.pick.Selected, with +} diff --git a/internal/preset/preset.go b/internal/preset/preset.go index 65b7de98..ac6c3054 100644 --- a/internal/preset/preset.go +++ b/internal/preset/preset.go @@ -122,8 +122,12 @@ func (p Preset) Check(args Args) error { } if raw := args[name]; raw != "" { if bad := param.Allows(raw); bad != "" { + // The remedy comes from the declaration, as it does for a + // format's property. Error leaves it out, so the command + // line reads as it always did, and a form gets the fourth + // part of the refusal under the box. return &format.PropertyValueError{ - Format: p.ID, Key: name, Value: raw, Reason: bad, + Format: p.ID, Key: name, Value: raw, Reason: bad, Remedy: param.Instead(), } } } diff --git a/internal/preset/read.go b/internal/preset/read.go new file mode 100644 index 00000000..f8fee07f --- /dev/null +++ b/internal/preset/read.go @@ -0,0 +1,145 @@ +package preset + +import ( + "errors" + "fmt" + "strings" + + "github.com/donislawdev/TestingFilesGenerator/internal/format" + "github.com/donislawdev/TestingFilesGenerator/internal/recipe" +) + +// Read is a recipe file as a run sees it: the recipe, and the preset it was +// built on when it was built on one. +// +// This is the door every surface reads a file through. The recipe package +// cannot expand a preset - the layer rule lets this package import that one +// and not the other way round - so a file that says extends: preset: is +// read in two steps with the expansion between them, and this is where the +// steps meet. A guard holds the command line and the window to reading files +// here rather than through recipe.Parse, because Parse on such a file refuses +// it, loudly, and the refusal is not one a person can act on. +type Read struct { + Recipe *recipe.Recipe + // Expansion is the preset the file built on, settled on what the file's + // with section gave it. Nil when the file stands alone - and that is how + // a caller tells the two apart, because the manifest records a preset + // only when there was one. + Expansion *Expansion +} + +// Notes is what the run has to say out loud about the preset's parameters +// nobody gave - see Expansion.Notes. Nothing, for a file that stands alone. +func (r *Read) Notes() []string { + if r.Expansion == nil { + return nil + } + return r.Expansion.Notes() +} + +// ReadRecipe reads a file, expanding the preset it builds on if it builds +// on one. +// +// A problem with the preset side - a preset this build does not have, a +// parameter it does not declare, a value it refuses, a set it cannot build - +// is reported the way a problem with the file is reported, with the address +// of the line it came from: the extends key, or with.. It is the file +// that is wrong in every one of those cases, and a screen has a box for each. +func ReadRecipe(src []byte, name string) (*Read, error) { + ext, err := recipe.ExtensionOf(src, name) + if err != nil { + return nil, err + } + if ext == nil { + rec, err := recipe.Parse(src, name) + if err != nil { + return nil, err + } + return &Read{Recipe: rec}, nil + } + + expanded, err := Expand(ext.Preset, Args(ext.With)) + if err != nil { + return nil, aboutTheFile(name, ext.Preset, err) + } + // On the extension rather than on src, so the file is decoded once. + rec, err := ext.Parse(expanded.Source) + if err != nil { + return nil, err + } + return &Read{Recipe: rec, Expansion: expanded}, nil +} + +// aboutTheFile turns a refusal from the preset side into a refusal about the +// recipe, addressed to the line it came from. +// +// The preset package words its refusals for the command line's --preset +// flags, where "the preset size-boundaries does not have a parameter called +// spreed" stands on its own. In a file the same mistake is a line under with, +// and the refusal has to say so and carry that address - the window marks a +// box by it, and the command line's list of problems names it. +func aboutTheFile(name, id string, err error) error { + problem := recipe.Problem{At: recipe.KeyExtends, What: err.Error()} + + var unknownPreset *UnknownPresetError + var unknownParameter *UnknownParameterError + var unknownFormat *format.UnknownFormatError + var value *format.PropertyValueError + var about interface{ AboutSetting() string } + var three interface { + What() string + Why() string + Instead() string + } + switch { + case errors.As(err, &unknownPreset): + problem.What = fmt.Sprintf("extends names preset:%s, which this build does not have", unknownPreset.ID) + problem.Why = "a recipe can build only on a preset the tool it runs on knows" + problem.Fix = "run \"tfg preset list\" for the ids this build has" + if len(unknownPreset.Known) > 0 { + problem.Fix = fmt.Sprintf("this build has: %s", strings.Join(unknownPreset.Known, ", ")) + } + case errors.As(err, &unknownParameter): + problem.At = recipe.KeyWith + "." + unknownParameter.Name + problem.What = fmt.Sprintf("with names %s, which the preset %s does not take", unknownParameter.Name, unknownParameter.Preset) + problem.Why = "with fills the parameters the preset declares, and this is not one of them" + problem.Fix = "remove the line" + if len(unknownParameter.Known) > 0 { + problem.Fix = fmt.Sprintf("the preset takes: %s", strings.Join(unknownParameter.Known, ", ")) + } + case errors.As(err, &value): + // A value the parameter refuses. Named by its line in the file + // rather than by the preset, which is how the command line's + // --preset path words the same refusal - there the flag is the line. + problem.At = recipe.KeyWith + "." + value.Key + problem.What = fmt.Sprintf("with.%s cannot be %q", value.Key, value.Value) + problem.Why = value.Why() + problem.Fix = value.Instead() + case errors.As(err, &unknownFormat): + // The one global a preset reads. The format registry refuses it in + // its own words, which name no line, so the line is named here. + problem.At = recipe.KeyWith + ".format" + problem.What = fmt.Sprintf("with.format names %q, which this build does not have", unknownFormat.ID) + problem.Why = "the preset gives every file it builds this format, so it has to be one the build can write" + problem.Fix = "run \"tfg formats\" for the list" + if len(unknownFormat.Known) > 0 { + problem.Fix = fmt.Sprintf("this build has: %s", strings.Join(unknownFormat.Known, ", ")) + } + case errors.As(err, &three) && errors.As(err, &about) && about.AboutSetting() != "": + // A set the preset cannot build from these values. It comes in three + // parts already and names the parameter, so the file's address is + // the parameter's line. The hint ends in a full stop of its own and + // the problem adds one, so the stop comes off here. + problem.At = recipe.KeyWith + "." + about.AboutSetting() + problem.What = three.What() + problem.Why = strings.TrimSuffix(three.Why(), ".") + problem.Fix = strings.TrimSuffix(three.Instead(), ".") + default: + // A refusal of a shape this does not know still arrives whole, at + // the extends line, with the two parts it lacks filled in rather than + // printed as a bare dash and a bare stop. + problem.Why = "the preset refused what the recipe gave it" + problem.Fix = "run \"tfg preset show " + id + "\" for what it takes" + } + return &recipe.ValidationError{Name: name, Problems: []recipe.Problem{problem}} +} diff --git a/internal/recipe/compose.go b/internal/recipe/compose.go index 4f2aa2e5..e5afb54b 100644 --- a/internal/recipe/compose.go +++ b/internal/recipe/compose.go @@ -53,7 +53,14 @@ type Document struct { Manifest string // Label is defaults.label, and a pointer because a switch has no third // position for silence. Nil leaves the defaults section out. - Label *bool + Label *bool + // Extends is the id of the preset the recipe builds on, and empty when + // it stands alone. With is what the screen typed into that preset's + // parameters, by name, with the ones left empty absent - so that a + // parameter nobody stated is written nowhere and stands in from its + // default, which is what the manifest then records as defaulted. + Extends string + With map[string]string Targets []TargetDraft } @@ -105,6 +112,14 @@ func Compose(d Document) ([]byte, error) { if d.Seed != "" { doc = append(doc, yaml.MapItem{Key: "seed", Value: d.Seed}) } + // The preset before the targets, because that is the order the run + // takes them in. + if d.Extends != "" { + doc = append(doc, yaml.MapItem{Key: KeyExtends, Value: presetScheme + d.Extends}) + } + if with := withSection(d); len(with) > 0 { + doc = append(doc, yaml.MapItem{Key: KeyWith, Value: with}) + } if d.Label != nil { doc = append(doc, yaml.MapItem{Key: "defaults", Value: yaml.MapSlice{{Key: "label", Value: *d.Label}}}) @@ -126,6 +141,18 @@ func Compose(d Document) ([]byte, error) { return yaml.Marshal(doc) } +// withSection is the preset's parameters, sorted for the reason properties +// are: this text is hashed into the manifest. Written whenever the map holds +// anything, extends or no extends - a with section without extends is a +// refusal Parse words, and dropping it here would turn that into silence. +func withSection(d Document) yaml.MapSlice { + var with yaml.MapSlice + for _, name := range sortedKeys(d.With) { + with = append(with, yaml.MapItem{Key: name, Value: d.With[name]}) + } + return with +} + func outputSection(d Document) yaml.MapSlice { var out yaml.MapSlice if d.OutDir != "" { @@ -244,6 +271,10 @@ func refuseUnwritable(d Document) error { check("seed", d.Seed) check("output.dir", d.OutDir) check("output.manifest", d.Manifest) + check(KeyExtends, d.Extends) + for _, name := range sortedKeys(d.With) { + check(KeyWith+"."+name, d.With[name]) + } for i, t := range d.Targets { where := targetSpot(i, t.ID) diff --git a/internal/recipe/errors.go b/internal/recipe/errors.go index 00b894b1..f0069396 100644 --- a/internal/recipe/errors.go +++ b/internal/recipe/errors.go @@ -161,6 +161,11 @@ func (p *problems) notYetIn(where spot, setting, why, fix string) { type spot struct { says string key string + // whole means the address is the whole of this spot whatever setting is + // named under it. A preset's target has no boxes of its own on any + // screen, so every refusal about it lands on the one box that is about + // the preset - see presetTargetSpot. + whole bool } func (s spot) String() string { return s.says } @@ -169,6 +174,9 @@ func (s spot) String() string { return s.says } // targets[2].size. A dotted setting is passed through, for the settings that // have a part of their own such as expected.reason. func (s spot) of(setting string) string { + if s.whole { + return s.key + } if s.key == "" { return setting } @@ -178,12 +186,33 @@ func (s spot) of(setting string) string { // entry names one item of a list inside this spot, counted from one the way the // prose counts, so the two halves agree about which entry is meant. func (s spot) entry(list string, index int) spot { + if s.whole { + return spot{says: fmt.Sprintf("%s: %s entry %d", s.says, list, index+1), key: s.key, whole: true} + } return spot{ says: fmt.Sprintf("%s: %s entry %d", s.says, list, index+1), key: fmt.Sprintf("%s.%s[%d]", s.key, list, index+1), } } +// presetTargetSpot is where one target the preset contributed is. +// +// Its prose says whose the target is, because a person looking at the file +// will not find it there, and its address is the extends key - the one box a +// screen has that is about the preset. The position counts inside the +// preset's own list, which is what "tfg preset eject" prints. +func presetTargetSpot(index int, id string) spot { + s := spot{ + says: fmt.Sprintf("the preset's target %d", index+1), + key: KeyExtends, + whole: true, + } + if id != "" { + s.says = fmt.Sprintf("the preset's target %q", id) + } + return s +} + // targetSpot is where one entry of the targets list is. func targetSpot(index int, id string) spot { s := spot{ diff --git a/internal/recipe/extends.go b/internal/recipe/extends.go new file mode 100644 index 00000000..096c7930 --- /dev/null +++ b/internal/recipe/extends.go @@ -0,0 +1,234 @@ +package recipe + +import ( + "fmt" + "sort" + "strings" +) + +// A recipe that builds on a preset. +// +// The recipe names the preset and fills its parameters, and its own targets +// are added after the preset's - so the file is the same run as "tfg preset +// eject" followed by editing, and shorter, and it says which test question +// the set came from. docs/RECIPE.md section 6, and the analysis in +// docs/EXTENDS-WITH-2026-09-22.md. +// +// This package cannot expand the preset, because it cannot import the preset +// package - the layer rule runs the other way. So reading such a file is two +// steps with the expansion between them, and both steps are here: ExtensionOf +// says what the file builds on, and ParseExtending reads the file with the +// preset's targets in front of its own. Whoever knows presets - internal/preset +// - joins the two, and every surface reads a file through that door. + +// presetScheme is what an extends value starts with. The only scheme this +// build knows: a recipe cannot build on another file yet, and saying so is +// better than guessing which of the two a bare name meant. +const presetScheme = "preset:" + +// Extension is what a recipe says it builds on. +type Extension struct { + // Preset is the id named after "preset:". + Preset string + // With is the parameters the recipe fills, as the text somebody would + // type into the flag of the same name. Absent when the recipe fills + // none, so every parameter stands in from its declared default. + With map[string]string + + // raw is the file as decoded, kept so that Parse does not decode it a + // second time. The batch screen reads its document on every change of + // a box, and the decoder is where the cost of reading sits - measured + // on the largest recipe allowed, 107 ms of an 841 ms validate. + raw rawRecipe + name string +} + +// ExtensionOf reads what a recipe builds on, and nil when it stands alone. +// +// It refuses what Parse would refuse about the document itself, so a file +// that is not a recipe is turned away here rather than expanded first. What +// it refuses about the two keys - with and no extends, a scheme this build +// does not know, a parameter written as a list - is worded the same way +// ParseExtending words it, because both call extension. +func ExtensionOf(src []byte, name string) (*Extension, error) { + raw, err := decode(src, name) + if err != nil { + return nil, err + } + p := &problems{name: name} + ext := raw.extension(p) + if err := p.err(); err != nil { + return nil, err + } + if ext != nil { + ext.raw, ext.name = raw, name + } + return ext, nil +} + +// Parse reads the recipe this extension came from, given the expansion of +// the preset it names - ParseExtending on the file already decoded. +func (e *Extension) Parse(base []byte) (*Recipe, error) { + return e.raw.parseExtending(e.name, base) +} + +// extension reads extends and with, refusing what cannot be read. +func (raw rawRecipe) extension(p *problems) *Extension { + if raw.Extends == nil { + if raw.With != nil { + p.add(KeyWith, "with fills the parameters of a preset, and no preset is named", + "with belongs beside extends: it says what to fill in, and extends says what to fill it into", + "add extends: preset: above it, or remove with") + } + return nil + } + value, ok := oneValue(p, KeyExtends, KeyExtends, "extends: preset:size-boundaries", raw.Extends) + if !ok { + return nil + } + id := strings.TrimPrefix(value, presetScheme) + if id == value || id == "" { + p.add(KeyExtends, fmt.Sprintf("extends %q does not name a preset", value), + "a recipe can build on a preset, and on nothing else in this build - not on another file", + "write extends: preset:, and run \"tfg preset list\" for the ids this build has") + return nil + } + ext := &Extension{Preset: id} + if len(raw.With) == 0 { + return ext + } + ext.With = make(map[string]string, len(raw.With)) + // In name order, so two runs over one file word their refusals in one + // order - a map is walked in whatever order it likes. + names := make([]string, 0, len(raw.With)) + for n := range raw.With { + names = append(names, n) + } + sort.Strings(names) + for _, n := range names { + s := raw.With[n] + text, ok := oneValue(p, KeyWith+"."+n, KeyWith+"."+n, + fmt.Sprintf("%s: %s", n, "1B,1kb,1mb"), &s) + if !ok { + continue + } + ext.With[n] = text + } + return ext +} + +// ParseExtending reads a recipe that builds on a preset, given the preset's +// expansion. +// +// The preset's targets come first and the file's own after them, and then the +// whole is judged once, by the same rules and in the same place as a recipe +// that stands alone - so an id used by both, a size under a format's floor, +// a name that cannot be written, are all refused the way they always were. +// Everything that is not a target comes from the file: the seed, the +// defaults, the output section. The preset may carry nothing else, and +// CheckBase is what holds a preset to that. +// +// Refusals about the file's own targets count from the file's first target, +// not the merged list's, so the position a screen registered a box under is +// the position the refusal names. +func ParseExtending(src []byte, name string, base []byte) (*Recipe, error) { + own, err := decode(src, name) + if err != nil { + return nil, err + } + return own.parseExtending(name, base) +} + +// parseExtending is ParseExtending on a file already decoded. +func (own rawRecipe) parseExtending(name string, base []byte) (*Recipe, error) { + // The expansion is decoded strictly too. A preset that expanded into a + // key this build does not know would be refused on the command line's + // --preset path, and this path refuses it the same way rather than + // merging the half it understands. + from, err := decode(base, name) + if err != nil { + return nil, err + } + p := &problems{name: name} + if own.extension(p) == nil { + // A file that does not extend anything was handed a base. That is a + // caller's mistake rather than the file's, and reading the file as + // if it did would run targets the file never asked for. + if p.err() == nil { + p.add(KeyExtends, "this recipe does not build on a preset", + "a preset's targets were supplied to the reader, and the recipe names none", + "read the file with Parse, or add extends: preset:") + } + return nil, p.err() + } + if err := from.onlyTargets(); err != nil { + p.add(KeyExtends, err.Error(), + "a preset contributes targets and nothing else, so that the seed, the defaults and the output section are always the file's own", + "this is a mistake in the preset rather than in the recipe - run \"tfg preset eject\" and edit the result instead") + return nil, p.err() + } + + merged := own + merged.Extends = nil + merged.With = nil + merged.Targets = append(append([]rawTarget{}, from.Targets...), own.Targets...) + + rec := merged.validate(p, len(from.Targets)) + if err := p.err(); err != nil { + return nil, err + } + return rec, nil +} + +// CheckBase says whether a preset's expansion is one a recipe can build on: +// a version and a list of targets, and nothing else. +// +// A guard asks this of every registered preset, so the rule that the file +// owns everything but the targets is held by every preset before any recipe +// extends it. ParseExtending asks it again of the one it was handed, because +// a layer that trusts the layer above it to have checked is a layer with a +// hole in it. +func CheckBase(src []byte, name string) error { + raw, err := decode(src, name) + if err != nil { + return err + } + return raw.onlyTargets() +} + +// onlyTargets is the rule behind CheckBase, on a decoded document. +func (raw rawRecipe) onlyTargets() error { + var carries []string + if raw.Seed != nil { + carries = append(carries, KeySeed) + } + if raw.Engine != nil { + carries = append(carries, KeyEngine) + } + if raw.Locale != nil { + carries = append(carries, KeyLocale) + } + if raw.Defaults != nil { + carries = append(carries, "defaults") + } + if raw.AllowNondeterministic != nil { + carries = append(carries, "allow_nondeterministic") + } + if raw.Policy != nil { + carries = append(carries, KeyPolicy) + } + if raw.Extends != nil { + carries = append(carries, KeyExtends) + } + if raw.With != nil { + carries = append(carries, KeyWith) + } + if raw.Output != nil { + carries = append(carries, "output") + } + if len(carries) == 0 { + return nil + } + return fmt.Errorf("the preset's recipe carries %s, which a recipe building on it cannot inherit", + strings.Join(carries, ", ")) +} diff --git a/internal/recipe/recipe.go b/internal/recipe/recipe.go index 45c6954a..16377be6 100644 --- a/internal/recipe/recipe.go +++ b/internal/recipe/recipe.go @@ -172,15 +172,32 @@ func (e *TooLargeError) Error() string { // once rather than the first one. Fixing a recipe one error per run is the // cheapest way to make someone stop using the tool. func Parse(src []byte, name string) (*Recipe, error) { + raw, err := decode(src, name) + if err != nil { + return nil, err + } + p := &problems{name: name} + rec := raw.validate(p, 0) + if err := p.err(); err != nil { + return nil, err + } + return rec, nil +} + +// decode reads a file into the raw recipe, refusing what is not a recipe +// document at all: too big, not UTF-8, nested past the limit, more than one +// document, a key no version of the schema has. Every door comes through here, +// so the two that read a file - Parse and ParseExtending - cannot come to +// accept different files. +func decode(src []byte, name string) (rawRecipe, error) { + var raw rawRecipe // Checked here as well as before the read, because this is the door every // caller comes through - including the fuzz target, which hands over bytes // that never were a file. if int64(len(src)) > MaxBytes { - return nil, &TooLargeError{Name: name, Bytes: int64(len(src))} + return raw, &TooLargeError{Name: name, Bytes: int64(len(src))} } - var raw rawRecipe - // A recipe is UTF-8, and anything else is refused rather than read as best // it can be. // @@ -196,7 +213,7 @@ func Parse(src []byte, name string) (*Recipe, error) { // one step earlier: what somebody typed is what they get, or they are told // why not. if !utf8.Valid(src) { - return nil, &SyntaxError{Name: name, Detail: "this file is not valid UTF-8. Every character that could not be read would come back as a replacement mark, so a name written with accents would produce a file called something else. Save the file as UTF-8 and try again"} + return raw, &SyntaxError{Name: name, Detail: "this file is not valid UTF-8. Every character that could not be read would come back as a replacement mark, so a name written with accents would produce a file called something else. Save the file as UTF-8 and try again"} } // An editor that writes a byte order mark would otherwise hand the decoder @@ -209,7 +226,7 @@ func Parse(src []byte, name string) (*Recipe, error) { // for the two shapes this and the budget below answer, and why one number // cannot answer both. if depth := nestingDepth(src); depth > MaxNestingDepth { - return nil, &TooDeepError{Name: name, Depth: depth} + return raw, &TooDeepError{Name: name, Depth: depth} } // One file is one recipe. Everything after a document separator would be @@ -217,22 +234,16 @@ func Parse(src []byte, name string) (*Recipe, error) { // asked for and a run that says it went fine. doc, err := oneDocument(src, name) if err != nil { - return nil, err + return raw, err } // Strict decoding turns an unknown key into an error. A typo in // "siez: 10mb" accepted in silence gives a file of the default size and an // hour spent wondering why the test passes when it should not. if err := decodeStrict(doc, &raw); err != nil { - return nil, &SyntaxError{Name: name, Detail: strings.TrimRight(err.Error(), "\n")} - } - - p := &problems{name: name} - rec := raw.validate(p) - if err := p.err(); err != nil { - return nil, err + return raw, &SyntaxError{Name: name, Detail: strings.TrimRight(err.Error(), "\n")} } - return rec, nil + return raw, nil } // decodeStrict runs the YAML decoder and turns a crash inside it into an error. @@ -314,9 +325,9 @@ type rawRecipe struct { AllowNondeterministic *scalar `yaml:"allow_nondeterministic"` - Policy map[string]any `yaml:"policy"` - Extends *scalar `yaml:"extends"` - With map[string]any `yaml:"with"` + Policy map[string]any `yaml:"policy"` + Extends *scalar `yaml:"extends"` + With map[string]scalar `yaml:"with"` Output *rawOutput `yaml:"output"` } @@ -332,7 +343,12 @@ type rawOutput struct { SplitThreshold *scalar `yaml:"split_threshold"` } -func (raw rawRecipe) validate(p *problems) *Recipe { +// fromPreset is how many of the targets came from the preset the recipe +// extends, and they are the first ones. It decides two things: the position a +// refusal about a target of the file carries, which counts from the file's +// own first target rather than from the merged list, and the words a refusal +// about a preset's target gets, since there is no line in the file to point at. +func (raw rawRecipe) validate(p *problems, fromPreset int) *Recipe { rec := &Recipe{ Version: SchemaVersion, Defaults: Defaults{Label: true}, @@ -361,6 +377,22 @@ func (raw rawRecipe) validate(p *problems) *Recipe { raw.refuseUnsupported(p) raw.applySettings(p, rec) + // A recipe that builds on a preset is read by ParseExtending, which is + // handed the preset's targets and clears the key before coming here. + // Reaching this with the key still set means a caller read such a file + // with Parse - which cannot expand a preset, because this package cannot + // import that one - and the honest answer is that the targets are + // missing, not that the key is unknown, and not that the recipe asks for + // no files: the sentence below about targets would contradict the + // README, which says a recipe of extends alone is legal. Until + // 2026-09-22 both keys were refused here as "not in this build yet". + if ext := raw.extension(p); ext != nil { + p.add(KeyExtends, fmt.Sprintf("this recipe builds on preset:%s, and the preset's targets were not supplied", ext.Preset), + "a recipe that builds on a preset is read by a reader that expands the preset first, and this one does not", + "read the file with \"tfg generate\" or \"tfg validate\", which do") + return rec + } + if len(raw.Targets) == 0 { p.add("targets", "the recipe asks for no files", "a recipe without targets has nothing to produce", @@ -368,22 +400,57 @@ func (raw rawRecipe) validate(p *problems) *Recipe { return rec } - seen := map[string]bool{} + // Which target first used an id, by position in the merged list. The + // position is what tells a clash with the preset's target apart from a + // clash with another target of the file - and the two get different + // words, because only one of them has a line the reader can change. + seen := map[string]int{} for i, rt := range raw.Targets { - t := rt.validate(p, i, rec.Defaults) + at := spotOfTarget(i, fromPreset) + t := rt.validate(p, at, rec.Defaults) if t.ID != "" { - if seen[t.ID] { - p.add(targetSpot(i, t.ID).of("id"), fmt.Sprintf("target {setting} %q is used twice", t.ID), - "{a} {setting} identifies a target, anchors its seed and links it to the manifest", - "give one of them a different {setting}") + if first, dup := seen[t.ID]; dup { + usedTwice(p, at(t.ID), t.ID, first < fromPreset && i >= fromPreset) + } else { + seen[t.ID] = i } - seen[t.ID] = true } rec.Targets = append(rec.Targets, t) } return rec } +// usedTwice refuses the second target carrying an id, in one of two wordings: +// a clash with the preset's target has no line in the file to point at for +// the first of the two, so it says whose the id is and where the way out +// lies. ofThePreset says which. +func usedTwice(p *problems, where spot, id string, ofThePreset bool) { + if ofThePreset { + p.add(where.of("id"), fmt.Sprintf("%s has the {setting} of a target the preset already builds", where), + "the preset's targets come first and every {setting} anchors a seed, so a second one would be a target nobody can tell from the first", + "give it a different {setting}, or leave the preset's target out by ejecting the preset and editing the recipe") + return + } + p.add(where.of("id"), fmt.Sprintf("target {setting} %q is used twice", id), + "{a} {setting} identifies a target, anchors its seed and links it to the manifest", + "give one of them a different {setting}") +} + +// spotOfTarget is where the target at position i of the merged list is, as a +// function of its id - the id is known only once the target has been read. +// +// A target of the file is at its position IN THE FILE, which is i less the +// preset's targets, so that the address a refusal carries is the box a screen +// registered and the number in the prose is the one a person counts to. A +// target of the preset has no line in the file, so its refusal is addressed to +// extends - the one box that is about it - and its prose says whose it is. +func spotOfTarget(i, fromPreset int) func(id string) spot { + if i < fromPreset { + return func(id string) spot { return presetTargetSpot(i, id) } + } + return func(id string) spot { return targetSpot(i-fromPreset, id) } +} + // refuseUnsupported names every top level key the document describes and this // build cannot honour. // @@ -408,18 +475,6 @@ func (raw rawRecipe) refuseUnsupported(p *problems) { p.notYet("policy", "unspecified expectations are left in the manifest for the consumer to settle", "remove the section - the expected field on a target already works") } - // The reason these two give used to be "presets are not in this build", - // and presets arrived on 2026-08-05 while this sentence stayed. A recipe - // still cannot name one - that is what is missing - so the key is refused - // for the same reason as before and the sentence now says which. - if raw.Extends != nil { - p.notYet("extends", "a recipe cannot build on a preset yet, though the command line can run one", - "run \"tfg preset eject > recipe.yaml\" and edit the targets, or write them out in full") - } - if raw.With != nil { - p.notYet("with", "a recipe cannot build on a preset yet, though the command line can run one", - "run \"tfg preset eject > recipe.yaml\" and edit the targets, or write them out in full") - } } // applySettings copies the seed, the defaults and the output settings onto the diff --git a/internal/recipe/target.go b/internal/recipe/target.go index 21ac53ee..bb3e38fe 100644 --- a/internal/recipe/target.go +++ b/internal/recipe/target.go @@ -65,14 +65,19 @@ type rawTarget struct { // that put one answer in tfg formats and another in the generator once already. const DefaultCount = 1 -func (rt rawTarget) validate(p *problems, index int, def Defaults) Target { +// validate reads one target and reports everything wrong with it. +// +// at is where this target is, as a function of its id - see spotOfTarget. It +// is a function rather than a position because the prose names a target by +// its id as soon as one is read, and the id is read here. +func (rt rawTarget) validate(p *problems, at func(id string) spot, def Defaults) Target { t := Target{Label: def.Label} count := DefaultCount - where := targetSpot(index, "") + where := at("") if id, ok := oneValue(p, where.of("id"), where.String()+" {setting}", "id: invoices", rt.ID); ok && id != "" { t.ID = id - where = targetSpot(index, t.ID) + where = at(t.ID) } else { p.add(where.of("id"), fmt.Sprintf("%s has no {setting}", where), "{a} {setting} anchors the seed of a target, so editing one target never moves the bytes of another",