diff --git a/CHANGELOG.md b/CHANGELOG.md index ddb5031..b944048 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -509,6 +509,10 @@ because it turns other people's test suites red. long as before. On the `Several batches` screen with a preset switched on, a key typed into a box now works the preset out once rather than twice. +- **Choosing a format, switching the base or choosing a base preset on + `Several batches` works the form out once instead of twice.** Switching + the base to `upload-validation` took half as long as before. + - **A preview or a run refused while it was being planned no longer leaves "Working out what this would cost..." standing over the refusal.** diff --git a/internal/guard/settleonce_test.go b/internal/guard/settleonce_test.go index c12a3d3..cd9432a 100644 --- a/internal/guard/settleonce_test.go +++ b/internal/guard/settleonce_test.go @@ -6,6 +6,7 @@ import ( "github.com/donislawdev/TestingFilesGenerator/internal/engine" "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/gui/window" "github.com/donislawdev/TestingFilesGenerator/internal/recipe" ) @@ -67,6 +68,86 @@ func TestOneChangeOfABoxReadsTheFormOnce(t *testing.T) { } } +// A menu, a switch or a press that lays the batch screen out again reads the +// form once. +// +// The batch screen lays itself out again when a batch's format changes, when +// the base is switched or chosen, and when a press adds, copies or takes away +// a batch or what an archive holds. The layout said what the form comes to, +// and until 2026-09-24 a menu or a switch was then followed by the live check, +// which said it again: two readings, and with a base preset two expansions - +// switching the base to upload-validation took 278-321 ms in the real window +// (docs/GUI-MEMORY-2026-09-23.md section 4j). +// +// Exactly one, for the reason the guard above gives, and it matters here in +// the other direction too: a press is followed by no check, so nought is a +// press after which the line under the buttons still describes the old form. +func TestAChangeThatLaysTheBatchScreenOutAgainReadsTheFormOnce(t *testing.T) { + host := newFakeHost(t) + rec := window.NewRecipe(host) + body := rec.Object() + once := func(what string, act func()) { + t.Helper() + host.settles = 0 + act() + switch { + case host.settles == 0: + t.Fatalf("%s told the host of no reading of the form - either nothing changed or the line under the buttons was not said, and this guard cannot tell which", + what) + case host.settles > 1: + t.Errorf("%s read the form %d times, expected once", what, host.settles) + } + } + press := func(name string) func() { + return func() { + t.Helper() + b := buttonNamed(body, name) + if b == nil { + t.Fatalf("the batch screen has no %q button, so this guard cannot press it", name) + } + b.OnTapped() + } + } + base := func() *parts.Chooser { + for _, c := range reportingControls(findField(rec.Fields(), recipe.KeyExtends).Control) { + if pick, is := c.(*parts.Chooser); is { + return pick + } + } + t.Fatal("the base is switched on and there is no menu of presets under it") + return nil + } + + once("choosing png for the first batch", func() { + chooserIn(t, rec.Fields(), recipe.TargetAddress(1, recipe.KeyFormat)).SetSelected("png") + }) + once("switching the base on", func() { toggleIn(t, rec.Fields(), "start_from_preset").SetChecked(true) }) + once("choosing text-encoding as the base", func() { base().SetSelected("text-encoding") }) + once("switching the base off", func() { toggleIn(t, rec.Fields(), "start_from_preset").SetChecked(false) }) + + once("adding a batch", press(text.ButtonAddBatch())) + once("duplicating a batch", press(text.ButtonDuplicateBatch())) + once("removing a batch", press(text.ButtonRemoveBatch())) + once("choosing zip for the first batch", func() { + chooserIn(t, rec.Fields(), recipe.TargetAddress(1, recipe.KeyFormat)).SetSelected("zip") + }) + once("adding what an archive holds", press(text.ButtonAddContents())) + once("removing what an archive holds", press(text.ButtonRemoveContents())) + + // The base switched off with no batch left, which brings a batch back and + // is the one way a switch and a press meet. Raised by the review of #133 + // (docs/REVIEW-133-2026-09-24.md): the batch came back through the press's + // path, which says the line, and the switch's check then said it again. + once("switching the base on again", func() { toggleIn(t, rec.Fields(), "start_from_preset").SetChecked(true) }) + for findField(rec.Fields(), recipe.TargetAddress(1, recipe.KeyID)) != nil { + once("removing a batch with the base on", press(text.ButtonRemoveBatch())) + } + once("switching the base off with no batch left", func() { toggleIn(t, rec.Fields(), "start_from_preset").SetChecked(false) }) + if findField(rec.Fields(), recipe.TargetAddress(1, recipe.KeyID)) == nil { + t.Error("the base was switched off with no batch left and no batch came back, so the screen can produce nothing") + } +} + // Typing into a box a preset is not given does not expand the preset again. // // The seed and the output directory are read with the preset's values but are diff --git a/internal/gui/window/recipe.go b/internal/gui/window/recipe.go index 4d4c013..bdf2d2b 100644 --- a/internal/gui/window/recipe.go +++ b/internal/gui/window/recipe.go @@ -354,8 +354,20 @@ func (r *Recipe) rebuild() { r.baseBox.Refresh() r.batchBox.Refresh() r.outBox.Refresh() - // A batch added, copied or taken away changes what the form comes to, - // and none of those goes through a box somebody typed in. + // Nothing is said here about what the form comes to. Whoever changed the + // form says it: a menu or a switch is followed by recheck, which reads the + // form once for the line and the box, and a press no box reports goes + // through afterAPress. This said it too until 2026-09-24, so a new format, + // base or preset read the form twice - and with a base preset, each + // reading expanded it (docs/GUI-MEMORY-2026-09-23.md section 4j). +} + +// afterAPress lays the screen out again and says what the form now comes to, +// for a change no box, menu or switch reports: a batch added, copied or taken +// away, contents added or taken away. A function rather than a method, because +// the screen stands near its ceiling of methods. +func afterAPress(r *Recipe) { + r.rebuild() r.runner.refreshLine() } @@ -496,7 +508,7 @@ func (r *Recipe) contentsBlock(index int, b *batch) fyne.CanvasObject { addContents := parts.NewButton(parts.Secondary, text.ButtonAddContents(), func() { b.contents = append(b.contents, r.newContent()) - r.rebuild() + afterAPress(r) }) if len(b.contents) == 0 { if !holds { @@ -583,12 +595,21 @@ func (r *Recipe) outputSection() fyne.CanvasObject { // addBatch puts another batch at the end of the list. func (r *Recipe) addBatch() { + newBatchAtTheEnd(r) + afterAPress(r) +} + +// newBatchAtTheEnd puts a batch after the last one, without saying what the +// form comes to. Apart from addBatch for the one caller that is not a press: +// the base switched off with no batch left brings one back, and the switch's +// own check says the line. Through addBatch it was said twice - found by the +// review of #133 (docs/REVIEW-133-2026-09-24.md). +func newBatchAtTheEnd(r *Recipe) { r.batches = append(r.batches, r.newBatch()) // A new batch has no format until one is chosen, and its declared settings // come with that choice. Chosen here rather than left empty so that a batch // arrives looking like the one above it. r.batches[len(r.batches)-1].formatPick.SetSelected(format.IDs()[0]) - r.rebuild() } // removeBatch drops one batch. The last cannot go, unless the screen builds @@ -629,7 +650,7 @@ func (r *Recipe) duplicateBatch(index int) { rest := append([]*batch{to}, r.batches[index+1:]...) r.batches = append(r.batches[:index+1], rest...) - r.rebuild() + afterAPress(r) } func (r *Recipe) removeBatch(index int) { @@ -640,7 +661,7 @@ func (r *Recipe) removeBatch(index int) { return } r.batches = append(r.batches[:index], r.batches[index+1:]...) - r.rebuild() + afterAPress(r) } func (r *Recipe) removeContent(b *batch, index int) { @@ -648,7 +669,7 @@ func (r *Recipe) removeContent(b *batch, index int) { return } b.contents = append(b.contents[:index], b.contents[index+1:]...) - r.rebuild() + afterAPress(r) } // onFormatChosen replaces the declared settings of one batch. diff --git a/internal/gui/window/recipebase.go b/internal/gui/window/recipebase.go index 5c01875..729bd26 100644 --- a/internal/gui/window/recipebase.go +++ b/internal/gui/window/recipebase.go @@ -49,10 +49,10 @@ 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. + // batch comes back - the one the screen opened with. What the form + // then comes to is said by the switch's own check, as for any switch. if !on && len(r.batches) == 0 { - r.addBatch() - return + newBatchAtTheEnd(r) } r.rebuild() })