From 16f1a7c872dd2770fd2961d36583f3b655582bce Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 22 Sep 2026 15:40:17 +0200 Subject: [PATCH 1/5] preset: let a preset say which one the window opens on, and stop a joint limit lying Two things the owner asked for in one branch. The window opened on the first preset id in order, which is alphabetical. That made what somebody sees when they open the Presets tab a property of whoever writes a preset next - it moved on 2026-09-22 when empty-and-minimal arrived, and nine window guards went red at once without one of them saying why. A preset declares it now, exactly one does, and a second is a panic at registration the way two presets of one id are. empty-and-minimal is the one, and the reason is a number rather than a preference: pressing Generate on it untouched writes 32 214 B where size-boundaries at its defaults writes 73 400 320. It also means something without being told anything, where size-boundaries says out loud that its 10mb is our placeholder and not the limit of the system under test - so an untouched run of it describes nothing. The second is O232, and the measurement found a worse case than the one recorded. A picture of 20000x2001 is 40 020 000 pixels against a limit of 40 000 000, and the refusal read "together they come to 40 megapixels and the limit is 40" - the same number twice, no unit on the second, and nothing a person could act on. The counts were divided by a million and the division truncates. Six declarations were affected, not just xlsx. The rounded form is kept where it still distinguishes the two counts, because "400 megapixels and the limit is 40 megapixels" is a sentence somebody can act on. Where it would put both on one number the exact counts are written instead, spaced every three digits the way ExactBytes has always done, and the limit carries its unit in both. Rejected: printing exactly always, which the comment above JointLimit turns down with reason. Rejected: more decimal places, which collides again at 40 000 001 against 40 000 000 at every fixed number of them. The guard reads the two numbers OUT OF THE SENTENCE rather than recomputing them, because what is being checked is what a person sees - one comparing the numbers the rule holds would agree with the rule and say nothing about the words it chose. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 19 +++++ internal/core/humanise.go | 15 +++- internal/format/avif/avif.go | 2 +- internal/format/format.go | 42 +++++++++- internal/format/gif/gif.go | 2 +- internal/format/jpg/jpg.go | 2 +- internal/format/jxl/jxl.go | 2 +- internal/format/png/png.go | 2 +- internal/format/xlsx/xlsx.go | 2 +- internal/guard/jointlimit_test.go | 126 +++++++++++++++++++++++++++++ internal/guard/landing_test.go | 106 ++++++++++++++++++++++++ internal/gui/window/preset.go | 7 +- internal/gui/window/recipebase.go | 8 +- internal/preset/emptyandminimal.go | 10 +++ internal/preset/preset.go | 46 +++++++++++ 15 files changed, 377 insertions(+), 14 deletions(-) create mode 100644 internal/guard/jointlimit_test.go create mode 100644 internal/guard/landing_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index a08766bc..0070cdba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,14 @@ because it turns other people's test suites red. ### Changed +- **The window opens on a preset that was chosen rather than sorted.** The + Presets tab, and the "Build on a preset" section of the batches screen, + used to start on whichever preset came first alphabetically - so what you + saw when you opened the tab changed whenever a preset was added. A preset + says now whether it is the one to start on, and `empty-and-minimal` is it: + pressing Generate without touching anything writes 32 214 B rather than the + 73 MB the size-boundaries defaults come to, and its set means something + without a number from you first. - **A recipe this program writes for you reads like one written by hand.** Where the tool composes a recipe - the batch screen, and `tfg preset eject` - a count and a size are now written as bare numbers (`size: 1024`) rather @@ -273,6 +281,17 @@ because it turns other people's test suites red. ### Fixed +- **A refusal about two settings that bound each other now says how far over + you are.** Asking for a picture of 20000 by 2001 pixels was turned down with + "together they come to 40 megapixels and the limit is 40" - the same number + twice, because the counts were rounded to whole megapixels and the request + is 40.02 of them. The limit had no unit after it either. Both counts are + written out exactly when rounding would put them on one number ("together + they come to 40 020 000 pixels and the limit is 40 000 000 pixels"), and the + readable form is kept where it still tells you something ("400 megapixels + and the limit is 40 megapixels"). The same sentence is used by every format + with a rule of this kind: `avif`, `gif`, `jpg`, `jxl`, `png` and `xlsx`. + - **The whole head row of a section opens and closes it.** Until now only the small arrow after a section's title was the target, so a click on "Notes for the manifest" or on "Settings for png" did nothing and the section diff --git a/internal/core/humanise.go b/internal/core/humanise.go index 8e38be1b..3c7af240 100644 --- a/internal/core/humanise.go +++ b/internal/core/humanise.go @@ -52,7 +52,20 @@ func HumanBytes(n int64) string { // Machine output is untouched on purpose. Nothing in a manifest or under --json // goes through here, because a number there is a number and not a sentence. func ExactBytes(n int64) string { - return groupedInThrees(strconv.FormatInt(n, 10)) + " B" + return Exactly(n) + " B" +} + +// Exactly is a whole number with a space every three digits, for a count that +// has to be read precisely rather than approximately. +// +// ExactBytes above it is this with a unit after it. Split out on 2026-09-22, +// when a refusal about a joint limit had to print two counts that the rounded +// form had put on one number: a picture of 20000x2001 comes to 40 020 000 +// pixels against a limit of 40 000 000, and the sentence read "they come to 40 +// megapixels and the limit is 40" - two identical numbers and a refusal nobody +// could answer. See format.JointLimit and O232. +func Exactly(n int64) string { + return groupedInThrees(strconv.FormatInt(n, 10)) } // groupedInThrees puts a space every three digits, counting from the right. diff --git a/internal/format/avif/avif.go b/internal/format/avif/avif.go index 9ad045a0..6fbe955f 100644 --- a/internal/format/avif/avif.go +++ b/internal/format/avif/avif.go @@ -138,7 +138,7 @@ func init() { }, JointLimits: []format.JointLimit{{ Of: "width", By: "height", Max: maxPixels, - Unit: "megapixels", Per: 1_000_000, + Unit: "megapixels", Per: 1_000_000, Base: "pixels", Why: "the encoder holds the whole picture in memory while it works", }}, GeneratorVersion: generatorVersion, diff --git a/internal/format/format.go b/internal/format/format.go index 9536e773..68a92caf 100644 --- a/internal/format/format.go +++ b/internal/format/format.go @@ -214,6 +214,16 @@ type JointLimit struct { // "400000000 and the limit is 40000000" is not. Per of nought means one. Unit string Per int64 + // Base is what the product itself counts, for the sentence that has to be + // exact: "pixels", "cells". + // + // It exists because the readable form above cannot always be used. Rounding + // two counts that differ can land them on one number, and then the refusal + // says "they come to 40 megapixels and the limit is 40" - measured on + // 2026-09-22 for a picture of 20000x2001, which is 40 020 000 pixels + // against a limit of 40 000 000. A person reading that has been told + // nothing. See Allows. + Base string // Why is the reason, in the words the refusal uses. Why string } @@ -228,11 +238,37 @@ type JointLimit struct { // be a branch nothing could ever reach, and an unreachable branch reads as a // protection somebody is relying on. func (j JointLimit) Allows(of, by int64) (bad string) { - if of*by <= j.Max { + got := of * by + if got <= j.Max { return "" } - return fmt.Sprintf("together they come to %d %s and the limit is %d, because %s", - of*by/j.per(), j.Unit, j.Max/j.per(), j.Why) + asked, allowed := j.readably(got) + return fmt.Sprintf("together they come to %s and the limit is %s, because %s", + asked, allowed, j.Why) +} + +// readably is the pair of counts as a person reads them, and it never puts two +// different counts on one number. +// +// The rounded form is offered first, because that is the one worth reading: +// "400 megapixels and the limit is 40 megapixels" is a sentence somebody can +// act on. It is stood down when both counts round to the same text, which is +// not a corner case - it is what a request just over the limit looks like. +// Measured on 2026-09-22 (O232): 20000x2001 is 40 020 000 pixels, the limit is +// 40 000 000, and the sentence read "they come to 40 megapixels and the limit +// is 40". Two identical numbers, and no way to tell how far over it was. +// +// Both halves carry the unit now. Only the first one did, so the limit was a +// bare number taking its noun from four words earlier. +// +// Adding decimal places was the other way out and it does not work: 40 000 001 +// against 40 000 000 collides at every fixed number of places. +func (j JointLimit) readably(got int64) (asked, allowed string) { + if j.per() > 1 && got/j.per() != j.Max/j.per() { + return fmt.Sprintf("%d %s", got/j.per(), j.Unit), + fmt.Sprintf("%d %s", j.Max/j.per(), j.Unit) + } + return core.Exactly(got) + " " + j.Base, core.Exactly(j.Max) + " " + j.Base } // Describe is the rule as one sentence, for the format list and for a window. diff --git a/internal/format/gif/gif.go b/internal/format/gif/gif.go index 187427ae..adcaf7c7 100644 --- a/internal/format/gif/gif.go +++ b/internal/format/gif/gif.go @@ -137,7 +137,7 @@ func init() { }, JointLimits: []format.JointLimit{{ Of: "width", By: "height", Max: maxPixels, - Unit: "megapixels", Per: 1_000_000, + Unit: "megapixels", Per: 1_000_000, Base: "pixels", Why: "the picture is held in memory while it is encoded", }}, GeneratorVersion: generatorVersion, diff --git a/internal/format/jpg/jpg.go b/internal/format/jpg/jpg.go index 0738c28c..20e6344e 100644 --- a/internal/format/jpg/jpg.go +++ b/internal/format/jpg/jpg.go @@ -120,7 +120,7 @@ func init() { }, JointLimits: []format.JointLimit{{ Of: "width", By: "height", Max: maxPixels, - Unit: "megapixels", Per: 1_000_000, + Unit: "megapixels", Per: 1_000_000, Base: "pixels", Why: "the picture is held in memory while it is encoded", }}, GeneratorVersion: generatorVersion, diff --git a/internal/format/jxl/jxl.go b/internal/format/jxl/jxl.go index 095d7ad0..ae84e71e 100644 --- a/internal/format/jxl/jxl.go +++ b/internal/format/jxl/jxl.go @@ -181,7 +181,7 @@ func init() { }, JointLimits: []format.JointLimit{{ Of: "width", By: "height", Max: maxPixels, - Unit: "megapixels", Per: 1_000_000, + Unit: "megapixels", Per: 1_000_000, Base: "pixels", Why: "the encoder holds the whole picture in memory while it works", }}, GeneratorVersion: generatorVersion, diff --git a/internal/format/png/png.go b/internal/format/png/png.go index c9003baa..a764bb2a 100644 --- a/internal/format/png/png.go +++ b/internal/format/png/png.go @@ -97,7 +97,7 @@ func init() { }, JointLimits: []format.JointLimit{{ Of: "width", By: "height", Max: maxPixels, - Unit: "megapixels", Per: 1_000_000, + Unit: "megapixels", Per: 1_000_000, Base: "pixels", Why: "the picture is held in memory while it is encoded", }}, GeneratorVersion: generatorVersion, diff --git a/internal/format/xlsx/xlsx.go b/internal/format/xlsx/xlsx.go index 3b0c40f7..7da19897 100644 --- a/internal/format/xlsx/xlsx.go +++ b/internal/format/xlsx/xlsx.go @@ -94,7 +94,7 @@ func init() { }, JointLimits: []format.JointLimit{{ Of: "rows", By: "columns", Max: maxCells, - Unit: "million cells", Per: 1_000_000, + Unit: "million cells", Per: 1_000_000, Base: "cells", Why: "the sheet is built in memory before it is packaged", }}, GeneratorVersion: generatorVersion, diff --git a/internal/guard/jointlimit_test.go b/internal/guard/jointlimit_test.go new file mode 100644 index 00000000..b473b572 --- /dev/null +++ b/internal/guard/jointlimit_test.go @@ -0,0 +1,126 @@ +package guard + +import ( + "strings" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/format" + _ "github.com/donislawdev/TestingFilesGenerator/internal/format/all" +) + +// A refusal about a joint limit never prints the request and the limit as one +// number. +// +// The rule binding two settings reports both counts in a unit a person reads - +// megapixels rather than pixels - and it reported them by dividing, which +// truncates. So a request just over the limit came back as the limit: a picture +// of 20000x2001 is 40 020 000 pixels against a limit of 40 000 000, and the +// sentence read "together they come to 40 megapixels and the limit is 40". +// Two identical numbers, no unit on the second one, and nothing in it a person +// could act on - which is the third part of D6 missing. Measured 2026-09-22, +// O232. +// +// The pairs below are asked of the registry's own declarations rather than of +// numbers written here, so a format arriving with a joint limit of its own is +// covered without anybody remembering to come back. +func TestNoJointLimitRefusalPrintsTheRequestAndTheLimitAsOneNumber(t *testing.T) { + limits := declaredJointLimits(t) + + for _, l := range limits { + // Three requests, and the middle one is the whole point: one unit over + // the limit is exactly where rounding used to hide the difference. + for _, over := range []int64{1, l.Max / 2, l.Max * 9} { + got := l.Max + over + bad := l.Allows(got, 1) + if bad == "" { + t.Errorf("%s: %d is past the limit of %d and the rule allowed it", + l.Of+" times "+l.By, got, l.Max) + continue + } + asked, allowed, ok := twoNumbersIn(bad) + if !ok { + t.Errorf("%s: the refusal does not read as two counts: %q", + l.Of+" times "+l.By, bad) + continue + } + if asked == allowed { + t.Errorf("%s: asked for %d against a limit of %d and the refusal says %q - "+ + "the two counts print as the same thing, so it says nothing", + l.Of+" times "+l.By, got, l.Max, bad) + } + } + } +} + +// Every joint limit says what its product counts. +// +// Base is what the exact form of the sentence is built on, so a declaration +// without one ends "together they come to 40 020 000 and the limit is +// 40 000 000 " - a sentence with a hole where its noun should be, and only on +// the path that is taken when the readable form cannot be used. That is the +// path nobody looks at, which is why it is asserted here rather than left to be +// noticed. +func TestEveryJointLimitSaysWhatItCounts(t *testing.T) { + for _, l := range declaredJointLimits(t) { + if strings.TrimSpace(l.Base) == "" { + t.Errorf("the rule binding %s and %s does not say what it counts, so its exact "+ + "refusal has no noun in it", l.Of, l.By) + } + if strings.TrimSpace(l.Unit) == "" { + t.Errorf("the rule binding %s and %s does not say what it reports in", l.Of, l.By) + } + if strings.TrimSpace(l.Why) == "" { + t.Errorf("the rule binding %s and %s gives no reason, and D6 asks for one", + l.Of, l.By) + } + } +} + +// declaredJointLimits is every rule the registry holds, and it refuses to hand +// back none - a guard walking an empty list passes against any rule ever +// written. +func declaredJointLimits(t *testing.T) []format.JointLimit { + t.Helper() + var out []format.JointLimit + for _, d := range format.All() { + out = append(out, d.JointLimits...) + } + if len(out) == 0 { + t.Fatal("no format declares a rule binding two settings, so this guard checked nothing") + } + return out +} + +// twoNumbersIn pulls the two counts out of a refusal, ignoring the spaces that +// group their digits. +// +// Read out of the sentence rather than recomputed, because what is being +// checked is what a person SEES. A guard comparing the numbers the rule holds +// would agree with the rule and say nothing about the words it chose. +func twoNumbersIn(sentence string) (first, second string, ok bool) { + found := make([]string, 0, 2) + var digits strings.Builder + flush := func() { + if digits.Len() > 0 { + found = append(found, digits.String()) + digits.Reset() + } + } + for i := 0; i < len(sentence); i++ { + c := sentence[i] + switch { + case c >= '0' && c <= '9': + digits.WriteByte(c) + case c == ' ' && digits.Len() > 0 && i+1 < len(sentence) && + sentence[i+1] >= '0' && sentence[i+1] <= '9': + // A space inside a grouped number, not the end of one. + default: + flush() + } + } + flush() + if len(found) < 2 { + return "", "", false + } + return found[0], found[1], true +} diff --git a/internal/guard/landing_test.go b/internal/guard/landing_test.go new file mode 100644 index 00000000..8a832adc --- /dev/null +++ b/internal/guard/landing_test.go @@ -0,0 +1,106 @@ +package guard + +import ( + "testing" + + "fyne.io/fyne/v2" + + _ "github.com/donislawdev/TestingFilesGenerator/internal/format/all" + "github.com/donislawdev/TestingFilesGenerator/internal/gui/parts" + "github.com/donislawdev/TestingFilesGenerator/internal/gui/text" + "github.com/donislawdev/TestingFilesGenerator/internal/gui/window" + "github.com/donislawdev/TestingFilesGenerator/internal/preset" +) + +// Exactly one preset declares itself the one a surface opens on. +// +// Before 2026-09-22 both screens opened on the first id in order, which is +// alphabetical - so what somebody saw when they opened the Presets tab was +// decided by whoever wrote a preset next. It changed on the day +// empty-and-minimal arrived, and nine window guards went red at once, none of +// them reporting anything more useful than a field being nil. The catalogue in +// docs/PRESETS.md has fourteen entries, so that was going to keep happening. +// +// Two presets claiming it is a panic at registration rather than a failure +// here, the same way two presets of one id are. What this catches is the other +// end: nobody claiming it, which is silent - preset.Landing falls back to the +// first id, and the fallback exists so that a window still opens rather than so +// that anybody relies on it. +func TestExactlyOnePresetOpensTheWindow(t *testing.T) { + all := preset.All() + if len(all) == 0 { + t.Fatal("no preset is registered, so this guard checked nothing") + } + + claimed := make([]string, 0, 1) + for _, p := range all { + if p.Landing { + claimed = append(claimed, p.ID) + } + } + switch len(claimed) { + case 1: + case 0: + t.Errorf("no preset says it is the one to open on, so the window falls back to %q - "+ + "the first id in alphabetical order, which is whichever preset gets written next", + preset.Landing()) + default: + t.Errorf("%v all say they are the one to open on, and only one can", claimed) + } + + if got := preset.Landing(); len(claimed) == 1 && got != claimed[0] { + t.Errorf("%s declares itself the one to open on and the window is told to open on %q", + claimed[0], got) + } +} + +// Both screens that offer presets open on the declared one. +// +// Asked of the SCREENS rather than of preset.Landing, which is the whole value +// of it: the declaration being right says nothing about whether a screen went +// through it, and the two screens reached for the first id separately. A guard +// calling Landing directly would agree with Landing and prove nothing. +func TestBothScreensOpenOnTheDeclaredPreset(t *testing.T) { + want := preset.Landing() + if want == "" { + t.Fatal("this build registers no preset, so neither screen has one to open on") + } + + host := newFakeHost(t) + window.Open(host) + if host.content == nil { + t.Fatal("opening the window put no screen in it") + } + t.Cleanup(func() { join(host) }) + + presets := selectTab(t, host.content, text.TabPresets()) + if got := chooserUnder(t, presets, text.FieldPreset()).Selected; got != want { + t.Errorf("the presets screen opens on %q and %q is the declared one", got, want) + } + + // The batch screen draws its preset menu only once the switch is on, so it + // is turned on the way a press turns it on. + batches := selectTab(t, host.content, text.TabRecipe()) + switchOn := checkNamed(batches, text.FieldBuildOnPreset()) + if switchOn == nil { + t.Fatal("there is no switch to build on a preset on the batch screen") + } + switchOn.SetChecked(true) + if got := basePresetOn(t, batches); got != want { + t.Errorf("the batch screen opens on %q and %q is the declared one", got, want) + } +} + +// basePresetOn is the preset chosen in the batch screen's base section. +// +// Read through the tree, because the control registered under the recipe key is +// the menu inside its width wrapper. +func basePresetOn(t *testing.T, screen fyne.CanvasObject) string { + t.Helper() + control := controlUnder(screen, text.FieldBasePreset()) + menu, ok := control.(*parts.Chooser) + if !ok { + t.Fatalf("the base preset field is %T rather than a list to choose from", control) + } + return menu.Selected +} diff --git a/internal/gui/window/preset.go b/internal/gui/window/preset.go index 6a6c4de8..b60f812a 100644 --- a/internal/gui/window/preset.go +++ b/internal/gui/window/preset.go @@ -114,8 +114,11 @@ func NewPreset(host Host, links ...fyne.CanvasObject) *Preset { // What a preset declares comes after this mark and is replaced with it. p.fixed = p.fields.Len() - if len(ids) > 0 { - p.pick.SetSelected(ids[0]) + // The preset that declares itself the one to open on, rather than the + // first in the list - the list is alphabetical, so "the first" is whoever + // is written next. See preset.Landing. + if landing := preset.Landing(); landing != "" { + p.pick.SetSelected(landing) } // Said last, once every box it reads exists. diff --git a/internal/gui/window/recipebase.go b/internal/gui/window/recipebase.go index c5bf6a32..dc73ee61 100644 --- a/internal/gui/window/recipebase.go +++ b/internal/gui/window/recipebase.go @@ -74,8 +74,12 @@ func newBase(r *Recipe) *base { // 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]) + // + // The one the preset declares, not the first in the list. The list is in + // alphabetical order, so "the first" moved the day a preset sorting + // earlier was written - see preset.Landing. + if landing := preset.Landing(); landing != "" { + b.pick.SetSelected(landing) } return b } diff --git a/internal/preset/emptyandminimal.go b/internal/preset/emptyandminimal.go index db2d11bc..d00a9ce3 100644 --- a/internal/preset/emptyandminimal.go +++ b/internal/preset/emptyandminimal.go @@ -45,6 +45,16 @@ func init() { }, }, + // The preset the window opens on, and the reason is a number rather + // than a preference. Pressing Generate on it without touching anything + // writes 32 214 B, where size-boundaries at its defaults writes + // 73 400 320 - and somebody who has just opened the program should not + // have seventy megabytes as their first result. It also means + // something without being told anything: size-boundaries says out loud + // that its 10mb is our placeholder and not the limit of the system + // under test, so an untouched run of it describes nothing. + Landing: true, + Requires: []string{"MVP"}, Catches: []string{ "a valid file turned away for being too small, where the check counts bytes instead of reading them", diff --git a/internal/preset/preset.go b/internal/preset/preset.go index 6e978f76..d63ef1da 100644 --- a/internal/preset/preset.go +++ b/internal/preset/preset.go @@ -57,6 +57,23 @@ type Preset struct { // that is not there. Reads []string + // Landing marks the preset a surface opens on before anybody has chosen + // one. Exactly one preset sets it, and Register refuses a second. + // + // Declared rather than worked out, because the only thing a surface could + // work out is "the first id in order" - and that is alphabetical, so it + // moves whenever a preset is added. It moved on 2026-09-22, when + // empty-and-minimal arrived and sorted before size-boundaries: the window + // opened on a different preset than the day before, and nine guards + // reaching for a field of the old one went red at once without one of them + // saying why. The catalogue in docs/PRESETS.md has fourteen entries, so + // that would have happened again. + // + // What it is NOT: a ranking, or an order. It answers one question - where + // does somebody who has chosen nothing start - and a second preset claiming + // it is a mistake in the build rather than a preference to resolve. + Landing bool + // Requires are the modules this preset needs, from docs/BACKLOG.md. Requires []string // Catches is what this preset typically finds, for the explain mode. @@ -320,6 +337,29 @@ func (e *ImpossibleError) AboutSetting() string { return e.Setting } // comes back and this file goes through that gate. var registry = map[string]Preset{} +// landing is the id of the preset that declared itself the one to open on, +// written at init beside the registry and read after. +var landing string + +// Landing is the preset a surface opens on before anybody has chosen one. +// +// It falls back to the first registered id when nothing claims it, and that +// fallback is deliberate rather than tidy: a window that would not open because +// a declaration went missing is worse than a window opening on the wrong +// preset. TestExactlyOnePresetOpensTheWindow is what turns the missing +// declaration red, so the fallback never has to be the thing anybody relies on. +// +// Empty only when this build registers no preset at all. +func Landing() string { + if landing != "" { + return landing + } + if ids := ids(); len(ids) > 0 { + return ids[0] + } + return "" +} + // Register adds a preset. It panics on a mistake that a build should not // survive, the same way the format registry does. func Register(p Preset) { @@ -332,6 +372,12 @@ func Register(p Preset) { if _, taken := registry[p.ID]; taken { panic(fmt.Sprintf("preset: %s is registered twice", p.ID)) } + if p.Landing { + if first := landing; first != "" { + panic(fmt.Sprintf("preset: %s and %s both open the window, and only one can", first, p.ID)) + } + landing = p.ID + } // A parameter IS a format.Property, so a closed set of values is put in the // same order here as it is over there. One rule for both, in the place each // declaration passes through exactly once. From b85a764a186098ab5e5cb25b50d232fc56b7f13a Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 22 Sep 2026 15:43:13 +0200 Subject: [PATCH 2/5] guard: assert both counts in a joint refusal carry the same noun The mutation taking the unit off the limit stayed green: it was aimed at the guard that reads the DECLARATIONS, and a declaration with a Base can still be printed without one. Nothing asserted the sentence. The guard reading the sentence now pulls the word after each count as well as the count itself, and refuses a bare number. Co-Authored-By: Claude Opus 5 --- internal/guard/jointlimit_test.go | 57 ++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/internal/guard/jointlimit_test.go b/internal/guard/jointlimit_test.go index b473b572..62ce23fb 100644 --- a/internal/guard/jointlimit_test.go +++ b/internal/guard/jointlimit_test.go @@ -37,17 +37,30 @@ func TestNoJointLimitRefusalPrintsTheRequestAndTheLimitAsOneNumber(t *testing.T) l.Of+" times "+l.By, got, l.Max) continue } - asked, allowed, ok := twoNumbersIn(bad) - if !ok { + counts := countsIn(bad) + if len(counts) < 2 { t.Errorf("%s: the refusal does not read as two counts: %q", l.Of+" times "+l.By, bad) continue } - if asked == allowed { + asked, allowed := counts[0], counts[1] + if asked.number == allowed.number { t.Errorf("%s: asked for %d against a limit of %d and the refusal says %q - "+ "the two counts print as the same thing, so it says nothing", l.Of+" times "+l.By, got, l.Max, bad) } + // Both counts carry the same noun. The limit used to be a bare + // number taking its noun from four words earlier, which reads as a + // count of something else - and nothing said so until a mutation + // took the unit off and every guard stayed green. + switch { + case allowed.unit == "": + t.Errorf("%s: the limit in %q is a bare number with no unit after it", + l.Of+" times "+l.By, bad) + case asked.unit != allowed.unit: + t.Errorf("%s: the refusal counts the request in %q and the limit in %q: %q", + l.Of+" times "+l.By, asked.unit, allowed.unit, bad) + } } } } @@ -91,20 +104,32 @@ func declaredJointLimits(t *testing.T) []format.JointLimit { return out } -// twoNumbersIn pulls the two counts out of a refusal, ignoring the spaces that -// group their digits. +// counted is one number in a refusal and the word that follows it. +type counted struct { + number string + unit string +} + +// countsIn pulls the counts out of a refusal with the word after each one, +// ignoring the spaces that group digits. +// +// The word is read with firstWordOf from the doc-comment guard, which trims the +// punctuation a sentence puts after its last noun - so a bare count followed by +// a comma comes back with no unit at all, which is exactly the state being +// looked for. // // Read out of the sentence rather than recomputed, because what is being // checked is what a person SEES. A guard comparing the numbers the rule holds // would agree with the rule and say nothing about the words it chose. -func twoNumbersIn(sentence string) (first, second string, ok bool) { - found := make([]string, 0, 2) +func countsIn(sentence string) []counted { + var found []counted var digits strings.Builder - flush := func() { - if digits.Len() > 0 { - found = append(found, digits.String()) - digits.Reset() + flush := func(rest string) { + if digits.Len() == 0 { + return } + found = append(found, counted{number: digits.String(), unit: firstWordOf(rest)}) + digits.Reset() } for i := 0; i < len(sentence); i++ { c := sentence[i] @@ -115,12 +140,10 @@ func twoNumbersIn(sentence string) (first, second string, ok bool) { sentence[i+1] >= '0' && sentence[i+1] <= '9': // A space inside a grouped number, not the end of one. default: - flush() + flush(sentence[i:]) } } - flush() - if len(found) < 2 { - return "", "", false - } - return found[0], found[1], true + flush("") + return found } + From 2bacdabcdef4b2055a27ad5d3faa17580edc9e65 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 22 Sep 2026 15:44:31 +0200 Subject: [PATCH 3/5] guard: gofmt Co-Authored-By: Claude Opus 5 --- internal/guard/jointlimit_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/guard/jointlimit_test.go b/internal/guard/jointlimit_test.go index 62ce23fb..ac1c5817 100644 --- a/internal/guard/jointlimit_test.go +++ b/internal/guard/jointlimit_test.go @@ -146,4 +146,3 @@ func countsIn(sentence string) []counted { flush("") return found } - From 924ee0d806ad37bb7964b15527c92e7b71644fc4 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 22 Sep 2026 16:31:38 +0200 Subject: [PATCH 4/5] guard: catch the preset regression somebody would actually write CodeRabbit on #121, and true. TestBothScreensOpenOnTheDeclaredPreset cannot tell asking preset.Landing apart from taking the first id, because empty-and-minimal is both - so a screen going back to the line it had until today would keep that guard green. Worse, the mutation that stood as its proof picked the LAST id, which is not a regression anybody would write. The guard flattered itself and the mutation agreed with it. A second guard reads the source instead: whatever a window file binds preset.IDs() to may be offered whole and may not be indexed. It asks where the list came from rather than looking for a shape, because the format menu next to it indexes its own list on purpose. Both screen mutations revert to ids[0] now, which is the real regression, and a third keeps the behavioural half honest by picking a preset nobody declared. The day a preset sorting before empty-and-minimal is written, the behavioural guard starts telling the two apart on its own. Co-Authored-By: Claude Opus 5 --- internal/guard/landing_test.go | 111 +++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/internal/guard/landing_test.go b/internal/guard/landing_test.go index 8a832adc..cdcc5aa8 100644 --- a/internal/guard/landing_test.go +++ b/internal/guard/landing_test.go @@ -1,6 +1,12 @@ package guard import ( + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "strings" "testing" "fyne.io/fyne/v2" @@ -60,6 +66,18 @@ func TestExactlyOnePresetOpensTheWindow(t *testing.T) { // of it: the declaration being right says nothing about whether a screen went // through it, and the two screens reached for the first id separately. A guard // calling Landing directly would agree with Landing and prove nothing. +// +// 🔴 What this one CANNOT tell apart today, and it was named by CodeRabbit on +// 2026-09-22 rather than noticed here: the declared preset is also the first id +// in alphabetical order, so a screen going back to picking by position would +// satisfy every assertion below. The mutation that "proved" this guard picked +// the LAST id, which is not a regression anybody would write. +// +// The pair to it is TestNoScreenChoosesAPresetByItsPlaceInTheList, which reads +// the source instead, because the two cannot be told apart by behaviour while +// one preset is both. The day a preset sorting before empty-and-minimal is +// written, this guard starts telling them apart on its own and the other one +// becomes the belt rather than the braces. func TestBothScreensOpenOnTheDeclaredPreset(t *testing.T) { want := preset.Landing() if want == "" { @@ -91,6 +109,99 @@ func TestBothScreensOpenOnTheDeclaredPreset(t *testing.T) { } } +// No screen picks a preset by where it sits in the list. +// +// The source rather than the behaviour, and that is a confession rather than a +// preference. TestBothScreensOpenOnTheDeclaredPreset above cannot separate "it +// asked preset.Landing" from "it took the first id", because empty-and-minimal +// is both - so a screen reverting to the line it had until 2026-09-22 would +// keep that guard green. Named by CodeRabbit, and it was right. +// +// The list a preset chooser is built from is preset.IDs(). So the rule is about +// THAT list: whatever a file binds it to may be offered whole and may not be +// indexed. The format menu beside it indexes its own list on purpose +// (generate.go picks the first format), which is why this asks where the list +// came from rather than looking for a shape. +func TestNoScreenChoosesAPresetByItsPlaceInTheList(t *testing.T) { + dir := filepath.Join(repoRoot(t), "internal", "gui", "window") + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("reading the window package: %v", err) + } + + looked := 0 + for _, entry := range entries { + name := entry.Name() + if entry.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") { + continue + } + looked++ + checkNoPresetListIndexing(t, filepath.Join(dir, name)) + } + if looked == 0 { + t.Fatal("no source file was read, so this guard would pass against anything") + } +} + +// checkNoPresetListIndexing reports every place one file indexes the list of +// preset ids. +func checkNoPresetListIndexing(t *testing.T, path string) { + t.Helper() + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, path, nil, 0) + if err != nil { + t.Fatalf("parsing %s: %v", path, err) + } + + lists := map[string]bool{} + ast.Inspect(file, func(n ast.Node) bool { + assign, ok := n.(*ast.AssignStmt) + if !ok || len(assign.Lhs) != 1 || len(assign.Rhs) != 1 { + return true + } + if !callsPresetIDs(assign.Rhs[0]) { + return true + } + if name, ok := assign.Lhs[0].(*ast.Ident); ok { + lists[name.Name] = true + } + return true + }) + if len(lists) == 0 { + return + } + + ast.Inspect(file, func(n ast.Node) bool { + index, ok := n.(*ast.IndexExpr) + if !ok { + return true + } + name, ok := index.X.(*ast.Ident) + if !ok || !lists[name.Name] { + return true + } + t.Errorf("%s:%d takes a preset out of the list by its place. A preset says which one "+ + "a surface opens on - see preset.Landing - and the list is in alphabetical order, "+ + "so a position is whichever preset gets written next", + filepath.Base(path), fset.Position(index.Pos()).Line) + return true + }) +} + +// callsPresetIDs reports whether an expression is the call preset.IDs(). +func callsPresetIDs(e ast.Expr) bool { + call, ok := e.(*ast.CallExpr) + if !ok { + return false + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "IDs" { + return false + } + pkg, ok := sel.X.(*ast.Ident) + return ok && pkg.Name == "preset" +} + // basePresetOn is the preset chosen in the batch screen's base section. // // Read through the tree, because the control registered under the recipe key is From 01d8e4f1dc931a90d163b78c807835b5c629fde4 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 22 Sep 2026 16:33:08 +0200 Subject: [PATCH 5/5] guard: no symbol in a comment that ships Co-Authored-By: Claude Opus 5 --- internal/guard/landing_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/guard/landing_test.go b/internal/guard/landing_test.go index cdcc5aa8..94daf57e 100644 --- a/internal/guard/landing_test.go +++ b/internal/guard/landing_test.go @@ -67,7 +67,7 @@ func TestExactlyOnePresetOpensTheWindow(t *testing.T) { // through it, and the two screens reached for the first id separately. A guard // calling Landing directly would agree with Landing and prove nothing. // -// 🔴 What this one CANNOT tell apart today, and it was named by CodeRabbit on +// What this one CANNOT tell apart today, and it was named by CodeRabbit on // 2026-09-22 rather than noticed here: the declared preset is also the first id // in alphabetical order, so a screen going back to picking by position would // satisfy every assertion below. The mutation that "proved" this guard picked