Merge branch 'go-readability' of bajdev/mpckit into main

cmd/mpckit-cli/main.go

@@ -18,9 +18,15 @@ import (
 	"git.monoexp.com/bajdev/mpckit/internal/project"
 )
 
+// opFunc is one change to the project, made from a flag's value.
+type opFunc func(p *project.Project) error
+
+// parseFunc turns a flag's value into an opFunc, or explains what is wrong.
+type parseFunc func(arg string) (opFunc, error)
+
 type op struct {
 	flag, arg string
-	run       func(p *project.Project) error
+	run       opFunc
 }
 
 var ops []op
@@ -28,7 +34,7 @@ var ops []op
 // opFlag is a repeatable flag whose values become ops.
 type opFlag struct {
 	name  string
-	parse func(arg string) (func(p *project.Project) error, error)
+	parse parseFunc
 }
 
 func (f opFlag) String() string { return "" }
@@ -41,7 +47,7 @@ func (f opFlag) Set(arg string) error {
 	return nil
 }
 
-func addOp(name, usage string, parse func(arg string) (func(p *project.Project) error, error)) {
+func addOp(name, usage string, parse parseFunc) {
 	flag.Var(opFlag{name: name, parse: parse}, name, usage)
 }
 
@@ -80,108 +86,158 @@ func parseSeqNum(s string) (int, error) {
 }
 
 func init() {
-	addOp("pad", "assign a WAV: `PAD=FILE` (e.g. A1=kick.wav)", func(arg string) (func(*project.Project) error, error) {
-		label, file, ok := strings.Cut(arg, "=")
-		if !ok {
-			return nil, errors.New("want PAD=FILE")
-		}
-		slot, err := parseSlot(label)
-		if err != nil {
-			return nil, err
-		}
-		return func(p *project.Project) error { return p.AssignSample(slot, file) }, nil
-	})
-	addOp("tune", "tune a pad: `PAD=SEMITONES,CENTS` (e.g. B1=-24,0)", func(arg string) (func(*project.Project) error, error) {
-		label, val, ok := strings.Cut(arg, "=")
-		c, f, ok2 := strings.Cut(val, ",")
-		if !ok || !ok2 {
-			return nil, errors.New("want PAD=SEMITONES,CENTS")
-		}
-		slot, err := parseSlot(label)
-		if err != nil {
-			return nil, err
-		}
-		coarse, err1 := strconv.Atoi(strings.TrimSpace(c))
-		fine, err2 := strconv.Atoi(strings.TrimSpace(f))
-		if err1 != nil || err2 != nil {
-			return nil, errors.New("semitones and cents must be integers")
-		}
-		if coarse < project.MinCoarse || coarse > project.MaxCoarse || fine < project.MinFine || fine > project.MaxFine {
-			return nil, fmt.Errorf("tuning out of range (%d..%d semitones, %d..%d cents)", project.MinCoarse, project.MaxCoarse, project.MinFine, project.MaxFine)
-		}
-		return func(p *project.Project) error { return p.SetTune(slot, coarse, fine) }, nil
-	})
-	addOp("mute", "set a pad's mute group: `PAD=GROUP` (0 is off)", func(arg string) (func(*project.Project) error, error) {
-		label, val, ok := strings.Cut(arg, "=")
-		if !ok {
-			return nil, errors.New("want PAD=GROUP")
-		}
-		slot, err := parseSlot(label)
-		if err != nil {
-			return nil, err
+	for _, f := range []struct {
+		name, usage string
+		parse       parseFunc
+	}{
+		{"pad", "assign a WAV: `PAD=FILE` (e.g. A1=kick.wav)", parsePadOp},
+		{"tune", "tune a pad: `PAD=SEMITONES,CENTS` (e.g. B1=-24,0)", parseTuneOp},
+		{"mute", "set a pad's mute group: `PAD=GROUP` (0 is off)", parseMuteOp},
+		{"clear", "clear a pad: `PAD`", parseClearOp},
+		{"copy", "copy pads: `FROM:TO`", parseCopyOp},
+		{"move", "move pads: `FROM:TO`", parseMoveOp},
+		{"swap", "swap pads: `FROM:TO`", parseSwapOp},
+		{"seq", "set a sequence, adding it if needed: `N=NAME,BPM,BARS` (N is 1-128)", parseSeqOp},
+		{"rmseq", "remove a sequence: `N`", parseRmseqOp},
+	} {
+		addOp(f.name, f.usage, f.parse)
+	}
+}
+
+// parsePadOp reads -pad PAD=FILE.
+func parsePadOp(arg string) (opFunc, error) {
+	label, file, ok := strings.Cut(arg, "=")
+	if !ok {
+		return nil, errors.New("want PAD=FILE")
+	}
+	slot, err := parseSlot(label)
+	if err != nil {
+		return nil, err
+	}
+	return func(p *project.Project) error { return p.AssignSample(slot, file) }, nil
+}
+
+// parseTuneOp reads -tune PAD=SEMITONES,CENTS.
+func parseTuneOp(arg string) (opFunc, error) {
+	label, val, ok := strings.Cut(arg, "=")
+	c, f, ok2 := strings.Cut(val, ",")
+	if !ok || !ok2 {
+		return nil, errors.New("want PAD=SEMITONES,CENTS")
+	}
+	slot, err := parseSlot(label)
+	if err != nil {
+		return nil, err
+	}
+	coarse, err1 := strconv.Atoi(strings.TrimSpace(c))
+	fine, err2 := strconv.Atoi(strings.TrimSpace(f))
+	if err1 != nil || err2 != nil {
+		return nil, errors.New("semitones and cents must be integers")
+	}
+	if coarse < project.MinCoarse || coarse > project.MaxCoarse || fine < project.MinFine || fine > project.MaxFine {
+		return nil, fmt.Errorf("tuning out of range (%d..%d semitones, %d..%d cents)",
+			project.MinCoarse, project.MaxCoarse, project.MinFine, project.MaxFine)
+	}
+	return func(p *project.Project) error { return p.SetTune(slot, coarse, fine) }, nil
+}
+
+// parseMuteOp reads -mute PAD=GROUP.
+func parseMuteOp(arg string) (opFunc, error) {
+	label, val, ok := strings.Cut(arg, "=")
+	if !ok {
+		return nil, errors.New("want PAD=GROUP")
+	}
+	slot, err := parseSlot(label)
+	if err != nil {
+		return nil, err
+	}
+	group, err := strconv.Atoi(val)
+	if err != nil {
+		return nil, errors.New("group must be an integer")
+	}
+	return func(p *project.Project) error { return p.SetMuteGroup(slot, group) }, nil
+}
+
+// parseClearOp reads -clear PAD.
+func parseClearOp(arg string) (opFunc, error) {
+	slot, err := parseSlot(arg)
+	if err != nil {
+		return nil, err
+	}
+	return func(p *project.Project) error { return p.ClearPad(slot) }, nil
+}
+
+// parseCopyOp reads -copy FROM:TO.
+func parseCopyOp(arg string) (opFunc, error) {
+	from, to, err := parsePair(arg)
+	if err != nil {
+		return nil, err
+	}
+	return func(p *project.Project) error { return p.CopyPad(from, to) }, nil
+}
+
+// parseMoveOp reads -move FROM:TO. Recorded notes stay on FROM, as in the
+// app, so it warns when there are any.
+func parseMoveOp(arg string) (opFunc, error) {
+	from, to, err := parsePair(arg)
+	if err != nil {
+		return nil, err
+	}
+	return func(p *project.Project) error {
+		if p.PadHasNotes(from) {
+			fmt.Fprintf(os.Stderr, "warning: %s has recorded notes; they stay on %s\n",
+				project.SlotName(from), project.SlotName(from))
 		}
-		g, err := strconv.Atoi(val)
-		if err != nil {
-			return nil, errors.New("group must be an integer")
+		return p.MovePad(from, to)
+	}, nil
+}
+
+// parseSwapOp reads -swap FROM:TO.
+func parseSwapOp(arg string) (opFunc, error) {
+	from, to, err := parsePair(arg)
+	if err != nil {
+		return nil, err
+	}
+	return func(p *project.Project) error { return p.SwapPads(from, to) }, nil
+}
+
+// parseSeqOp reads -seq N=NAME,BPM,BARS. The sequence is added first if
+// slot N is empty.
+func parseSeqOp(arg string) (opFunc, error) {
+	num, val, ok := strings.Cut(arg, "=")
+	parts := strings.Split(val, ",")
+	if !ok || len(parts) != 3 {
+		return nil, errors.New("want N=NAME,BPM,BARS")
+	}
+	slot, err := parseSeqNum(num)
+	if err != nil {
+		return nil, err
+	}
+	bpm, err1 := strconv.ParseFloat(strings.TrimSpace(parts[1]), 64)
+	bars, err2 := strconv.Atoi(strings.TrimSpace(parts[2]))
+	if err1 != nil || err2 != nil {
+		return nil, errors.New("BPM must be a number and BARS an integer")
+	}
+	return func(p *project.Project) error {
+		exists := false
+		for _, seq := range p.Sequences() {
+			exists = exists || seq.Slot == slot
 		}
-		return func(p *project.Project) error { return p.SetMuteGroup(slot, g) }, nil
-	})
-	addOp("clear", "clear a pad: `PAD`", func(arg string) (func(*project.Project) error, error) {
-		slot, err := parseSlot(arg)
-		return func(p *project.Project) error { return p.ClearPad(slot) }, err
-	})
-	for _, name := range []string{"copy", "move", "swap"} {
-		addOp(name, name+" pads: `FROM:TO`", func(arg string) (func(*project.Project) error, error) {
-			from, to, err := parsePair(arg)
-			if err != nil {
-				return nil, err
+		if !exists {
+			if err := p.AddSequence(slot); err != nil {
+				return err
 			}
-			return func(p *project.Project) error {
-				switch name {
-				case "copy":
-					return p.CopyPad(from, to)
-				case "move":
-					if p.PadHasNotes(from) {
-						fmt.Fprintf(os.Stderr, "warning: %s has recorded notes; they stay on %s\n", project.SlotName(from), project.SlotName(from))
-					}
-					return p.MovePad(from, to)
-				}
-				return p.SwapPads(from, to)
-			}, nil
-		})
-	}
-	addOp("seq", "set a sequence, adding it if needed: `N=NAME,BPM,BARS` (N is 1-128)", func(arg string) (func(*project.Project) error, error) {
-		num, val, ok := strings.Cut(arg, "=")
-		parts := strings.Split(val, ",")
-		if !ok || len(parts) != 3 {
-			return nil, errors.New("want N=NAME,BPM,BARS")
-		}
-		slot, err := parseSeqNum(num)
-		if err != nil {
-			return nil, err
-		}
-		bpm, err1 := strconv.ParseFloat(strings.TrimSpace(parts[1]), 64)
-		bars, err2 := strconv.Atoi(strings.TrimSpace(parts[2]))
-		if err1 != nil || err2 != nil {
-			return nil, errors.New("BPM must be a number and BARS an integer")
 		}
-		return func(p *project.Project) error {
-			exists := false
-			for _, s := range p.Sequences() {
-				exists = exists || s.Slot == slot
-			}
-			if !exists {
-				if err := p.AddSequence(slot); err != nil {
-					return err
-				}
-			}
-			return p.SetSequence(slot, parts[0], bpm, bars)
-		}, nil
-	})
-	addOp("rmseq", "remove a sequence: `N`", func(arg string) (func(*project.Project) error, error) {
-		slot, err := parseSeqNum(arg)
-		return func(p *project.Project) error { return p.RemoveSequence(slot) }, err
-	})
+		return p.SetSequence(slot, parts[0], bpm, bars)
+	}, nil
+}
+
+// parseRmseqOp reads -rmseq N.
+func parseRmseqOp(arg string) (opFunc, error) {
+	slot, err := parseSeqNum(arg)
+	if err != nil {
+		return nil, err
+	}
+	return func(p *project.Project) error { return p.RemoveSequence(slot) }, nil
 }
 
 func main() {
@@ -202,39 +258,54 @@ func main() {
 		fail(errors.New("give exactly one of -out or -zip"))
 	}
 
+	p := openProject(*load, *name)
+	for _, o := range ops {
+		if err := o.run(p); err != nil {
+			fail(fmt.Errorf("-%s %s: %w", o.flag, o.arg, err))
+		}
+	}
+	dest := write(p, *out, *zipPath, *force)
+	summary(p)
+	fmt.Println("wrote", dest)
+}
+
+// openProject loads the project at load, or starts a blank one, and names
+// it. It exits on error.
+func openProject(load, name string) *project.Project {
 	var p *project.Project
 	var err error
-	if *load != "" {
+	if load != "" {
 		var warnings []string
-		p, warnings, err = project.Load(*load)
+		p, warnings, err = project.Load(load)
 		for _, w := range warnings {
 			fmt.Fprintln(os.Stderr, "warning:", w)
 		}
 	} else {
-		if *name == "" {
+		if name == "" {
 			fail(errors.New("-name is required for a new project"))
 		}
-		p, err = project.New(*name)
+		p, err = project.New(name)
 	}
 	if err != nil {
 		fail(err)
 	}
-	if *name != "" {
-		if err := p.SetName(*name); err != nil {
+	if name != "" {
+		if err := p.SetName(name); err != nil {
 			fail(err)
 		}
 	}
-	for _, o := range ops {
-		if err := o.run(p); err != nil {
-			fail(fmt.Errorf("-%s %s: %w", o.flag, o.arg, err))
-		}
-	}
+	return p
+}
 
+// write exports p to the folder out or the zip file zipPath, whichever is
+// set, and returns where it went. It exits on error.
+func write(p *project.Project, out, zipPath string, force bool) string {
 	var dest string
-	if *out != "" {
-		dest, err = export.Folder(p, *out, *force)
+	var err error
+	if out != "" {
+		dest, err = export.Folder(p, out, force)
 	} else {
-		dest, err = *zipPath, export.Zip(p, *zipPath, *force)
+		dest, err = zipPath, export.Zip(p, zipPath, force)
 	}
 	var exists *export.ExistsError
 	if errors.As(err, &exists) {
@@ -243,8 +314,7 @@ func main() {
 	if err != nil {
 		fail(err)
 	}
-	summary(p)
-	fmt.Println("wrote", dest)
+	return dest
 }
 
 func summary(p *project.Project) {

internal/app/app.go

@@ -217,63 +217,99 @@ func (s *Service) About() About {
 	}
 }
 
+// state builds what the frontend shows about the project. Callers hold s.mu.
 func (s *Service) state() State {
-	p := s.p
+	proj := s.p
+	missing := missingSources(proj)
 	st := State{
-		Name:        p.Name,
-		Dirty:       p.Dirty(),
-		Pads:        make([]Pad, 0, project.Pads),
-		Sequences:   []Sequence{},
-		SampleCount: p.SampleCount(),
-		Limits: Limits{
-			Pads:         project.Pads,
-			MaxSamples:   project.MaxSamples,
-			MaxSequences: project.MaxSequences,
-			MinCoarse:    project.MinCoarse,
-			MaxCoarse:    project.MaxCoarse,
-			MinFine:      project.MinFine,
-			MaxFine:      project.MaxFine,
-			MaxMuteGroup: project.MaxMuteGroup,
-			MinBPM:       project.MinBPM,
-			MaxBPM:       project.MaxBPM,
-			MaxBars:      project.MaxBars,
-		},
-	}
-	missing := map[string]bool{} // pool file -> source missing, as export sees it
-	for _, f := range p.PoolFiles() {
-		src, _ := p.Source(f)
-		missing[f] = src == "" || !export.Usable(src)
+		Name:         proj.Name,
+		Dirty:        proj.Dirty(),
+		Pads:         make([]Pad, 0, project.Pads),
+		Sequences:    sequenceStates(proj),
+		SampleCount:  proj.SampleCount(),
+		MissingCount: len(missing),
+		Limits:       limits(),
 	}
 	for slot := 0; slot < project.Pads; slot++ {
-		v, err := p.Pad(slot)
-		if err != nil {
-			// A loaded file with an odd pad; show it as empty.
-			st.Pads = append(st.Pads, Pad{Slot: slot, Label: project.SlotName(slot)})
-			continue
-		}
-		pad := Pad{Slot: slot, Label: project.SlotName(slot), Name: v.SampleName, File: v.File,
-			Coarse: v.Coarse, Fine: v.Fine, MuteGroup: v.MuteGroup, Start: v.Start, End: v.End}
-		if !v.Empty() {
-			pad.HasNotes = p.PadHasNotes(slot)
-			pad.Source, _ = p.Source(v.File)
-			pad.Missing = missing[v.File]
-			if !pad.Missing {
-				pad.Audio = audioURL(slot, pad.Source)
-			}
-		}
-		st.Pads = append(st.Pads, pad)
-	}
-	for _, q := range p.Sequences() {
-		st.Sequences = append(st.Sequences, Sequence{Slot: q.Slot, Label: project.SlotName(q.Slot),
-			Name: q.Name, BPM: q.BPM, Bars: q.Bars, HasNotes: q.HasNotes})
+		st.Pads = append(st.Pads, padState(proj, slot, missing))
 	}
 	st.SequenceCount = len(st.Sequences)
-	for _, m := range missing {
-		if m {
-			st.MissingCount++
+	return st
+}
+
+// limits returns the device and format limits the frontend enforces.
+func limits() Limits {
+	return Limits{
+		Pads:         project.Pads,
+		MaxSamples:   project.MaxSamples,
+		MaxSequences: project.MaxSequences,
+		MinCoarse:    project.MinCoarse,
+		MaxCoarse:    project.MaxCoarse,
+		MinFine:      project.MinFine,
+		MaxFine:      project.MaxFine,
+		MaxMuteGroup: project.MaxMuteGroup,
+		MinBPM:       project.MinBPM,
+		MaxBPM:       project.MaxBPM,
+		MaxBars:      project.MaxBars,
+	}
+}
+
+// missingSources returns the pool files that export could not copy: no
+// source recorded, or one export refuses (see export.Usable).
+func missingSources(proj *project.Project) map[string]bool {
+	missing := map[string]bool{}
+	for _, file := range proj.PoolFiles() {
+		if src, _ := proj.Source(file); src == "" || !export.Usable(src) {
+			missing[file] = true
 		}
 	}
-	return st
+	return missing
+}
+
+// padState describes the pad in slot. missing is from missingSources.
+func padState(proj *project.Project, slot int, missing map[string]bool) Pad {
+	view, err := proj.Pad(slot)
+	if err != nil {
+		// A loaded file with an odd pad; show it as empty.
+		return Pad{Slot: slot, Label: project.SlotName(slot)}
+	}
+	pad := Pad{
+		Slot:      slot,
+		Label:     project.SlotName(slot),
+		Name:      view.SampleName,
+		File:      view.File,
+		Coarse:    view.Coarse,
+		Fine:      view.Fine,
+		MuteGroup: view.MuteGroup,
+		Start:     view.Start,
+		End:       view.End,
+	}
+	if view.Empty() {
+		return pad
+	}
+	pad.HasNotes = proj.PadHasNotes(slot)
+	pad.Source, _ = proj.Source(view.File)
+	pad.Missing = missing[view.File]
+	if !pad.Missing {
+		pad.Audio = audioURL(slot, pad.Source)
+	}
+	return pad
+}
+
+// sequenceStates describes the project's sequences, in slot order.
+func sequenceStates(proj *project.Project) []Sequence {
+	seqs := []Sequence{}
+	for _, seq := range proj.Sequences() {
+		seqs = append(seqs, Sequence{
+			Slot:     seq.Slot,
+			Label:    project.SlotName(seq.Slot),
+			Name:     seq.Name,
+			BPM:      seq.BPM,
+			Bars:     seq.Bars,
+			HasNotes: seq.HasNotes,
+		})
+	}
+	return seqs
 }
 
 // audioURL is the preview URL for a pad. The key in the query changes with
@@ -283,9 +319,9 @@ func audioURL(slot int, src string) string {
 	return fmt.Sprintf("%s%d?k=%s", AudioPrefix, slot, hex.EncodeToString(sum[:6]))
 }
 
-// edit runs fn on the project under the lock and returns the new state.
+// apply runs fn on the project under the lock and returns the new state.
 // The state is returned even on error, so the frontend stays in step.
-func (s *Service) edit(fn func(p *project.Project) error) (State, error) {
+func (s *Service) apply(fn func(p *project.Project) error) (State, error) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	err := fn(s.p)
@@ -294,7 +330,7 @@ func (s *Service) edit(fn func(p *project.Project) error) (State, error) {
 
 // SetName renames the project.
 func (s *Service) SetName(name string) (State, error) {
-	return s.edit(func(p *project.Project) error { return p.SetName(strings.TrimSpace(name)) })
+	return s.apply(func(p *project.Project) error { return p.SetName(strings.TrimSpace(name)) })
 }
 
 // AssignSampleDialog asks for a WAV file and puts it on the pad in slot.
@@ -308,22 +344,22 @@ func (s *Service) AssignSampleDialog(slot int) (State, error) {
 }
 
 func (s *Service) assignSample(slot int, path string) (State, error) {
-	return s.edit(func(p *project.Project) error { return p.AssignSample(slot, path) })
+	return s.apply(func(p *project.Project) error { return p.AssignSample(slot, path) })
 }
 
 // ClearPad empties the pad in slot.
 func (s *Service) ClearPad(slot int) (State, error) {
-	return s.edit(func(p *project.Project) error { return p.ClearPad(slot) })
+	return s.apply(func(p *project.Project) error { return p.ClearPad(slot) })
 }
 
 // SetTune sets a pad's tuning in semitones and cents.
 func (s *Service) SetTune(slot, coarse, fine int) (State, error) {
-	return s.edit(func(p *project.Project) error { return p.SetTune(slot, coarse, fine) })
+	return s.apply(func(p *project.Project) error { return p.SetTune(slot, coarse, fine) })
 }
 
 // SetMuteGroup sets a pad's mute group; 0 is off.
 func (s *Service) SetMuteGroup(slot, group int) (State, error) {
-	return s.edit(func(p *project.Project) error { return p.SetMuteGroup(slot, group) })
+	return s.apply(func(p *project.Project) error { return p.SetMuteGroup(slot, group) })
 }
 
 // ChooseExportFolder asks for the folder to export into and remembers it
@@ -412,17 +448,17 @@ func (s *Service) exported(path string, err error) (ExportResult, error) {
 // MovePad moves the pad in from to slot to, replacing what was there.
 // Recorded notes stay on their slots; the frontend warns first.
 func (s *Service) MovePad(from, to int) (State, error) {
-	return s.edit(func(p *project.Project) error { return p.MovePad(from, to) })
+	return s.apply(func(p *project.Project) error { return p.MovePad(from, to) })
 }
 
 // CopyPad copies the pad in from to slot to, replacing what was there.
 func (s *Service) CopyPad(from, to int) (State, error) {
-	return s.edit(func(p *project.Project) error { return p.CopyPad(from, to) })
+	return s.apply(func(p *project.Project) error { return p.CopyPad(from, to) })
 }
 
 // SwapPads exchanges two pads.
 func (s *Service) SwapPads(a, b int) (State, error) {
-	return s.edit(func(p *project.Project) error { return p.SwapPads(a, b) })
+	return s.apply(func(p *project.Project) error { return p.SwapPads(a, b) })
 }
 
 // Load asks for a project file and opens it in place of the current one.
@@ -453,19 +489,19 @@ func (s *Service) loadPath(path string) (LoadResult, error) {
 // AddSequence adds a sequence in an empty slot: "Sequence NN", 2 bars, at
 // the BPM of the most recently edited sequence (else 120).
 func (s *Service) AddSequence(slot int) (State, error) {
-	return s.edit(func(p *project.Project) error { return p.AddSequence(slot) })
+	return s.apply(func(p *project.Project) error { return p.AddSequence(slot) })
 }
 
 // RemoveSequence deletes a sequence and its recorded notes. The last
 // sequence cannot be removed.
 func (s *Service) RemoveSequence(slot int) (State, error) {
-	return s.edit(func(p *project.Project) error { return p.RemoveSequence(slot) })
+	return s.apply(func(p *project.Project) error { return p.RemoveSequence(slot) })
 }
 
 // SetSequence sets a sequence's name, BPM and length in bars. Notes past a
 // new, shorter end are left in the file.
 func (s *Service) SetSequence(slot int, name string, bpm float64, bars int) (State, error) {
-	return s.edit(func(p *project.Project) error {
+	return s.apply(func(p *project.Project) error {
 		return p.SetSequence(slot, strings.TrimSpace(name), bpm, bars)
 	})
 }

internal/project/load_test.go

@@ -171,7 +171,7 @@ func FuzzLoad(f *testing.F) {
 
 // TestBrokenFieldsDoNotPanic changes, one at a time, each field that project
 // reads in DemoFile: replaced by null, text, a negative or large number, an
-// empty list or object, or deleted. Whatever wrap makes of the result, reads
+// empty list or object, or deleted. Whatever fromFile makes of the result, reads
 // and edits must not panic. Only the first item of each list is changed, so
 // the test stays fast; a full sweep (all fields to depth 5, 12,726 cases)
 // found nothing either and takes about 10 minutes.
@@ -253,7 +253,7 @@ func TestBrokenFieldsDoNotPanic(t *testing.T) {
 						t.Errorf("%s %s: panic: %v", path, names[c], r)
 					}
 				}()
-				p, err := wrap(f, "Broken", "")
+				p, err := fromFile(f, "Broken", "")
 				if err != nil {
 					return
 				}

internal/project/pads.go

@@ -151,7 +151,7 @@ func (p *Project) AssignSample(slot int, src string) error {
 	if err != nil {
 		return err
 	}
-	var e edit
+	var e setter
 	e.set(pad, "tempo", xpj.NewFloat(padTempo))
 	e.set(l, "sampleName", xpj.NewString(stem(newFile)))
 	e.set(l, "sampleFile", xpj.NewString(newFile))
@@ -161,7 +161,7 @@ func (p *Project) AssignSample(slot int, src string) error {
 		return e.err
 	}
 
-	// Apply. None of these steps can fail: wrap checked that the pool
+	// Apply. None of these steps can fail: fromFile checked that the pool
 	// and the pads are lists.
 	p.clear(slot, file)
 	p.pads().Items[slot] = pad
@@ -239,19 +239,21 @@ func poolFileName(name string) string {
 func stem(file string) string { return strings.TrimSuffix(file, filepath.Ext(file)) }
 
 // uniqueFile returns base, or base with a 001, 002 ... suffix before the
-// extension, so that no two pool files share a name. Names are compared
-// without case, as on Windows, macOS and FAT.
+// extension, so that no two pool files share a file name or a sample name
+// (the file name without its extension). Names are compared without case,
+// as on Windows, macOS and FAT.
 func (p *Project) uniqueFile(base string, freed map[string]bool) string {
-	taken := map[string]bool{}
+	takenFiles := map[string]bool{}
+	takenNames := map[string]bool{}
 	for _, e := range p.pool().Items {
 		if freed[text(e.Lookup("path"))] {
 			continue // about to be dropped
 		}
-		taken[strings.ToLower(text(e.Lookup("path")))] = true
-		taken[strings.ToLower(text(e.Lookup("name")))+"\x00"] = true
+		takenFiles[strings.ToLower(text(e.Lookup("path")))] = true
+		takenNames[strings.ToLower(text(e.Lookup("name")))] = true
 	}
 	free := func(f string) bool {
-		return !taken[strings.ToLower(f)] && !taken[strings.ToLower(stem(f))+"\x00"]
+		return !takenFiles[strings.ToLower(f)] && !takenNames[strings.ToLower(stem(f))]
 	}
 	if free(base) {
 		return base
@@ -285,7 +287,7 @@ func poolEntry(file string) *xpj.Node {
 	return e
 }
 
-// addPoolEntry adds file to both pools. Both are lists (checked by wrap),
+// addPoolEntry adds file to both pools. Both are lists (checked by fromFile),
 // so this cannot fail.
 func (p *Project) addPoolEntry(file string) {
 	_ = p.pool().Append(poolEntry(file))
@@ -393,7 +395,7 @@ func (p *Project) SetTune(slot, coarse, fine int) error {
 		return fmt.Errorf("project: pad %s layer 1 is %s, not an object", SlotName(slot), l.Kind)
 	}
 	// Setting keys on an object cannot fail, so all three are written.
-	var e edit
+	var e setter
 	e.set(l, "coarseTune", xpj.NewInt(int64(coarse)))
 	e.set(l, "fineTune", xpj.NewInt(int64(fine)))
 	e.set(l, "pitch", xpj.NewFloat32(float64(coarse)+float64(fine)/100))

internal/project/project.go

@@ -64,7 +64,7 @@ type Project struct {
 
 	file *xpj.File
 	data *xpj.Node
-	// The lists every operation works on, found once by wrap.
+	// The lists every operation works on, found once by fromFile.
 	nodes struct{ pool, trackPool, pads, sequences *xpj.Node }
 
 	lastBPM float64 // BPM of the most recently edited sequence; 0 if none
@@ -103,7 +103,7 @@ func New(name string) (*Project, error) {
 	if err != nil {
 		return nil, err
 	}
-	return wrap(f, name, "")
+	return fromFile(f, name, "")
 }
 
 // Load reads the .xpj at path. The project name is taken from the file name
@@ -121,7 +121,7 @@ func Load(path string) (*Project, []string, error) {
 	}
 	name := strings.TrimSuffix(filepath.Base(abs), filepath.Ext(abs))
 	dataDir := filepath.Join(filepath.Dir(abs), name+DataSuffix)
-	p, err := wrap(f, name, dataDir)
+	p, err := fromFile(f, name, dataDir)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -141,7 +141,9 @@ func Load(path string) (*Project, []string, error) {
 	return p, warnings, nil
 }
 
-func wrap(f *xpj.File, name, sourceDir string) (*Project, error) {
+// fromFile builds a Project around a decoded file: it finds the lists every
+// operation uses and refuses a file the operations could not use safely.
+func fromFile(f *xpj.File, name, sourceDir string) (*Project, error) {
 	d, err := f.Data()
 	if err != nil {
 		return nil, err
@@ -360,12 +362,12 @@ func (p *Project) trackPool() *xpj.Node { return p.nodes.trackPool }
 
 func (p *Project) sequences() *xpj.Node { return p.nodes.sequences }
 
-// edit applies several Sets and keeps the first error, so an operation can
-// list its writes plainly. Paths come from the template; a failure means the
-// loaded file has a different shape.
-type edit struct{ err error }
+// setter applies several Sets and keeps the first error, so an operation
+// can list its writes plainly. Paths come from the template; a failure means
+// the loaded file has a different shape.
+type setter struct{ err error }
 
-func (e *edit) set(n *xpj.Node, path string, v *xpj.Node) {
+func (e *setter) set(n *xpj.Node, path string, v *xpj.Node) {
 	if e.err == nil {
 		if err := n.Set(path, v); err != nil {
 			e.err = fmt.Errorf("project: %w", err)

internal/project/sequences.go

@@ -76,7 +76,7 @@ func (p *Project) AddSequence(slot int) error {
 		bpm = DefaultBPM
 	}
 	s := template.sequence.Clone()
-	var e edit
+	var e setter
 	e.set(s, "key", xpj.NewInt(int64(slot)))
 	v := s.Lookup("value")
 	e.set(v, "name", xpj.NewString(fmt.Sprintf("Sequence %02d", slot+1)))
@@ -133,6 +133,8 @@ func (p *Project) SetSequence(slot int, name string, bpm float64, bars int) erro
 	if err := ValidName(name); err != nil {
 		return fmt.Errorf("sequence name: %w", err)
 	}
+	// Written as a negated range so that NaN, for which every comparison is
+	// false, is refused too.
 	if !(bpm >= MinBPM && bpm <= MaxBPM) {
 		return fmt.Errorf("BPM must be %g-%g", MinBPM, MaxBPM)
 	}
@@ -144,7 +146,7 @@ func (p *Project) SetSequence(slot int, name string, bpm float64, bars int) erro
 	if err != nil {
 		return err
 	}
-	// wrap checked that value is an object, and lengthClips that each clip
+	// fromFile checked that value is an object, and lengthClips that each clip
 	// has a value object, so none of these writes can fail.
 	_ = v.SetKey("name", xpj.NewString(name))
 	_ = v.SetKey("bpm", xpj.NewFloat(bpm))

internal/xpj/json.go

@@ -54,13 +54,13 @@ func parseValue(dec *json.Decoder) (*Node, error) {
 				n.Items = append(n.Items, v)
 			}
 			_, err := dec.Token() // ]
-			return n, wrap(err)
+			return n, wrapErr(err)
 		case '{':
 			n := &Node{Kind: Object, Members: []Member{}}
 			for dec.More() {
 				kt, err := dec.Token()
 				if err != nil {
-					return nil, wrap(err)
+					return nil, wrapErr(err)
 				}
 				v, err := parseValue(dec)
 				if err != nil {
@@ -69,13 +69,14 @@ func parseValue(dec *json.Decoder) (*Node, error) {
 				n.Members = append(n.Members, Member{Key: kt.(string), Value: v})
 			}
 			_, err := dec.Token() // }
-			return n, wrap(err)
+			return n, wrapErr(err)
 		}
 	}
 	return nil, fmt.Errorf("xpj: unexpected token %v", tok)
 }
 
-func wrap(err error) error {
+// wrapErr prefixes a JSON decoding error with the package name.
+func wrapErr(err error) error {
 	if err == nil {
 		return nil
 	}

main.go

@@ -43,6 +43,19 @@ func init() {
 func main() {
 	d := &dialogs{}
 	svc, host := app.NewService(d)
+	a := newApp(svc, host)
+	d.a = a
+	w := newWindow(a)
+	guardClose(a, w, host)
+	handleFileDrops(a, w, host)
+	if err := a.Run(); err != nil {
+		log.Fatal(err)
+	}
+}
+
+// newApp creates the application. Only svc is bound; host serves the
+// sample previews.
+func newApp(svc *app.Service, host *app.Host) *application.App {
 	var a *application.App
 	a = application.New(application.Options{
 		Name:        "MPCkit",
@@ -65,11 +78,12 @@ func main() {
 			ApplicationShouldTerminateAfterLastWindowClosed: true,
 		},
 	})
+	return a
+}
 
-	d.a = a
-
-	// Start hidden and size the window once the screens are known, so it
-	// fits the display instead of being clamped by it.
+// newWindow opens the main window. It starts hidden and is sized once the
+// screens are known, so it fits the display instead of being clamped by it.
+func newWindow(a *application.App) *application.WebviewWindow {
 	w := a.Window.NewWithOptions(application.WebviewWindowOptions{
 		Title:            "MPCkit",
 		Width:            maxWidth,
@@ -81,7 +95,6 @@ func main() {
 		BackgroundColour: application.NewRGB(0x15, 0x17, 0x1A),
 		URL:              "/",
 	})
-
 	a.Event.OnApplicationEvent(events.Common.ApplicationStarted, func(*application.ApplicationEvent) {
 		if s := a.Screen.GetPrimary(); s != nil {
 			width, height := fitSize(s.WorkArea.Width, s.WorkArea.Height)
@@ -90,10 +103,13 @@ func main() {
 		w.Center()
 		w.Show()
 	})
+	return w
+}
 
-	// Closing with unsaved changes: stop, and let the frontend ask. If the
-	// user chooses to discard them, the frontend calls CloseWindow, which
-	// closes for real.
+// guardClose handles closing with unsaved changes: stop, and let the
+// frontend ask. If the user chooses to discard them, the frontend calls
+// CloseWindow, which closes for real.
+func guardClose(a *application.App, w *application.WebviewWindow, host *app.Host) {
 	var closeConfirmed atomic.Bool
 	host.SetCloser(func() {
 		closeConfirmed.Store(true)
@@ -106,9 +122,11 @@ func main() {
 		e.Cancel()
 		a.Event.Emit(eventClose)
 	})
+}
 
-	// A WAV dropped on a pad (an element with data-file-drop-target and
-	// data-slot) is assigned to that pad.
+// handleFileDrops assigns a WAV dropped on a pad (an element with
+// data-file-drop-target and data-slot) to that pad.
+func handleFileDrops(a *application.App, w *application.WebviewWindow, host *app.Host) {
 	w.OnWindowEvent(events.Common.WindowFilesDropped, func(e *application.WindowEvent) {
 		ctx := e.Context()
 		target := ctx.DropTargetDetails()
@@ -131,10 +149,6 @@ func main() {
 		}
 		a.Event.Emit(eventError, "Only WAV files can be assigned to a pad.")
 	})
-
-	if err := a.Run(); err != nil {
-		log.Fatal(err)
-	}
 }
 
 // fitSize returns a 16:9 window size that takes screenShare of the work

pull-requests/go-readability/go-readability.md

@@ -0,0 +1,100 @@
+# Go readability
+
+Branch: `go-readability` → `main`
+
+PR 10 of the review fixes plan (`docs/review-fixes-plan.md`), a readability PR: no behaviour change, no version change. It was done after PR 11; the two touch different files.
+
+## What changed
+
+Four commits, one per area:
+
+1. **`Service.state` split into named helpers (R3, R4).**
+   - **The helpers:** `state()` in `internal/app/app.go` used `p`, `v`, `q`, `f`, `m` and `st` across a 58-line function. It now builds the `State` from four helpers:
+     - `limits()`;
+     - `missingSources(proj)`, the pool files export could not copy;
+     - `padState(proj, slot, missing)`;
+     - `sequenceStates(proj)`.
+   
+     They use names such as `proj`, `view` and `seq`.
+   - **The rename:** `Service.edit` becomes `apply`.
+2. **Clearer names and notes in `xpj` and `project` (R4, R6).**
+   - **Renames, with the comments that mention them:**
+
+     | Was | Now | What it does |
+     | --- | --- | --- |
+     | `xpj.wrap` | `wrapErr` | Prefixes JSON errors with `xpj:`. |
+     | `project.wrap` | `fromFile` | Builds a `Project` from a decoded file. |
+     | `project.edit` (type) | `setter` | Applies several `Set`s and keeps the first error. |
+
+     Together with `apply`, no two things share a name any more.
+   - **`uniqueFile`:** it kept taken file names and sample names in one map, told apart by a `"\x00"` suffix. Now it uses two maps, `takenFiles` and `takenNames`, and its comment says both kinds of name are kept unique.
+   - **The BPM check:** `!(bpm >= MinBPM && bpm <= MaxBPM)` now has a comment saying it is written that way so NaN, for which every comparison is false, is refused.
+3. **`main()` reads as a list of steps (R5).**
+   - `main()` in `main.go` goes from 96 lines to 12. It calls four new functions, each keeping the comment that was there:
+     - `newApp`: the application options and the panic handler;
+     - `newWindow`: the hidden window, sized and shown once the screens are known;
+     - `guardClose`: the unsaved-changes guard;
+     - `handleFileDrops`: the file-drop handler.
+   - The steps run in the same order as before.
+4. **The CLI's flags each have a named parse function (R5).**
+   - **Parsing:** `init()` in `cmd/mpckit-cli/main.go` was 104 lines of closures. Each flag now has its own function: `parsePadOp`, `parseTuneOp`, `parseMuteOp`, `parseClearOp`, `parseCopyOp`, `parseMoveOp`, `parseSwapOp`, `parseSeqOp` and `parseRmseqOp`. `init()` registers them from a nine-row table.
+   - **`main()`:** it was 62 lines, over the plan's 60. It now hands opening the project to `openProject` and exporting to `write`.
+   - **Unchanged:** flag names, usage text, error messages and output.
+
+**Longest functions** in `main.go`, `cmd/mpckit-cli/main.go` and `internal/app/app.go`:
+
+| | Longest | Next |
+| --- | --- | --- |
+| Before (`main`) | CLI `init` 104 lines | `main()` 96, CLI `main` 62, `state` 58 |
+| Now | 28 lines: `padState`, `parseSeqOp` and the CLI `main` | `newApp` and `openProject`, 25 each |
+
+## Notes
+
+- **Name choices.** I used the plan's names: `wrapErr`, `fromFile`, `apply` and `setter`.
+  - **New in the CLI:** a function type for one flag's change to the project, which I named `opFunc`, beside the existing `op` and `opFlag`. I avoided calling it `edit`, which this PR removes as a name elsewhere.
+  - **The two CLI helpers:** `openProject` and `write`.
+- **Two small changes in how the CLI's parse functions are written, with the same result:**
+  - `-clear` and `-rmseq` used to return a function together with the error. They now return nil on error. `flag` never stored the function when there was an error, so nothing a user sees changes.
+  - `-tune`'s long out-of-range message is now split across two lines of source; the text is the same.
+- **Not changed:** what each function does, the order of `main()`'s steps, and the tests. `load_test.go` changes only for the `fromFile` rename.
+
+## Testing
+
+**On the server, every command with `GOTOOLCHAIN=go1.27.4`:**
+- `go test ./...` passes. No test was removed or loosened; one renamed call and comment in `load_test.go`.
+- `go vet ./...` (also with `GOOS=windows`) and `staticcheck ./...` are clean.
+- `govulncheck ./...`: "No vulnerabilities found."
+- In `frontend`: `npm ci`, `npm run check` (0 errors, 0 warnings), `npm run build` and `npm run format:check` pass. The frontend is unchanged.
+- `wails3 build` builds and the app starts under Xvfb.
+
+**"No behaviour change", checked against `main` before the first commit:**
+- **Exports are byte-identical.** I ran `go run ./cmd/mpckit-cli -load testdata/device/DemoFile.xpj -out <dir>` on `main` and after the last commit. `cmp` finds the two `DemoFile.xpj` identical, and `diff -r` finds the data folders identical.
+- **`go run ./cmd/mpckit-cli -h`** prints exactly the same text.
+- **22 CLI runs give the same results.** Two runs use every flag (one exports to a folder, one to a zip). The rest are 15 bad flag values, an existing export with and without `-force`, a stray argument, both `-out` and `-zip`, and no `-name`. For each run I compared stdout, stderr, exit code, the exported files, and the zip's entries and their contents (the zip itself stores its write time).
+- **The State the window gets is byte-identical.** A scratch test, not committed, wrote the full `State` JSON after loading DemoFile, ten edits, two refused edits, loading a copy with a missing WAV, and the refused export.
+- **No bound method changed:** `TestBoundMethods` passes untouched, and `frontend/bindings` did not change.
+
+**On the laptop (PowerShell, in the repo).** This changes the window set-up in `main.go`, so watch the window size, the close prompt and file drop in particular:
+1. `git fetch; git checkout go-readability`
+2. `go test ./...`: all `ok`.
+3. `go run ./cmd/mpckit-cli -h`: the same usage text as before.
+4. `wails3 dev`. The window opens at the usual size, centred, with nothing scrolling.
+5. **About:** open About and check the version and links. Close it with Escape.
+6. **Pads tab:** load `testdata\device\DemoFile.xpj`.
+   - Click pads to hear them. The pad panel shows names and files.
+   - Change a tune and a mute group.
+   - Drop a WAV onto an empty pad: it is assigned. Drop a non-WAV file: "Only WAV files can be assigned to a pad." shows.
+   - Move/Copy to pad: the picker opens. Drag A1 onto A5: the notes warning shows; Cancel.
+7. **Sequences tab:**
+   - Select Intro and Sequence 02.
+   - Change a name and a BPM.
+   - Set Sequence 02 to 1 bar: the "Shorten…" dialog shows; Cancel.
+   - Add a sequence in B5, then remove it: the "Remove…" dialog shows.
+8. **Export:**
+   - Export to a new folder.
+   - Export there again and Cancel at "Replace existing files?".
+   - Export a zip.
+9. **Missing file:** load a copy of DemoFile with one WAV removed. The missing badge and the footer warning show, and export is disabled.
+10. **New and close:**
+    - New, with unsaved changes: the confirm dialog shows.
+    - Make a change, then close the window: "Close without exporting?" shows. Cancel keeps the app open; confirming closes it.