5 files changed, 135 insertions, 3 deletions
diff --git a/gui/internal/model/plan.go b/gui/internal/model/plan.go
index c3e9654..31a783b 100644
--- a/gui/internal/model/plan.go
+++ b/gui/internal/model/plan.go
@@ -16,6 +16,7 @@ import (
 	"git.labunix.xyz/krino/internal/lock"
 	"git.labunix.xyz/krino/internal/plan"
 	"git.labunix.xyz/krino/internal/scan"
+	"git.labunix.xyz/krino/internal/xdg"
 )
 
 // Row is one line of the Plan tab: a file krino would act on, or one it
@@ -292,6 +293,23 @@ func (t *PlanTab) KeepThisCopy(i int) error {
 	if row.Path == "" {
 		return fmt.Errorf("model: %s has no path", row.Rel)
 	}
+	// Only the first step to reach a path may displace it: once another
+	// step of this same plan is going to put its own file there, displacing
+	// it again would destroy that file. internal/plan refuses to build such
+	// a pair; the window writes its step by hand, so it must refuse too.
+	for _, c := range t.dp.Chains {
+		if c.File.Rel == row.Rel {
+			continue
+		}
+		for _, st := range c.Steps {
+			if st.Skip != "" {
+				continue
+			}
+			if st.Dst == row.DuplicateOf || st.Displaces == row.DuplicateOf {
+				return fmt.Errorf("model: %s is already spoken for by %s in this plan", xdg.Abbrev(row.DuplicateOf), c.File.Rel)
+			}
+		}
+	}
 	for j, c := range t.dp.Chains {
 		if c.File.Rel != row.Rel {
 			continue
diff --git a/gui/internal/model/plan_test.go b/gui/internal/model/plan_test.go
index 1bae2c5..241a049 100644
--- a/gui/internal/model/plan_test.go
+++ b/gui/internal/model/plan_test.go
@@ -505,3 +505,41 @@ func TestCloseDuringApplyIsRefused(t *testing.T) {
 		t.Errorf("the apply itself failed: %v", applyErr)
 	}
 }
+
+// TestKeepThisCopyTwiceIsRefused: "keep this copy" writes a Displaces
+// straight into the chain. The engine refuses to plan two steps that
+// displace one path - the second would destroy what the first just put
+// there - but the window went round that code. Both rows then reported
+// "done" while the first file was in the Trash.
+func TestKeepThisCopyTwiceIsRefused(t *testing.T) {
+	conf := "(path \"~/dl\")\n(rule \"dupes\" (when (duplicate)) (move \"Dupes\"))\n"
+	e, h := sandboxDir(t, conf, map[string]string{
+		"x.pdf": "the same bytes", "y.pdf": "the same bytes", "a.pdf": "the same bytes",
+	})
+	_ = h
+	tab, err := Plan(context.Background(), e, e.Dirs[0])
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { tab.Close() })
+
+	var dupes []int
+	for i, r := range tab.Rows {
+		if r.DuplicateOf != "" {
+			dupes = append(dupes, i)
+		}
+	}
+	if len(dupes) < 2 {
+		t.Skipf("fixture produced %d duplicate rows, need two", len(dupes))
+	}
+	if err := tab.KeepThisCopy(dupes[0]); err != nil {
+		t.Fatalf("the first choice was refused: %v", err)
+	}
+	err = tab.KeepThisCopy(dupes[1])
+	if err == nil {
+		t.Fatal("two files were allowed to replace the same one; the second would trash what the first filed")
+	}
+	if !strings.Contains(err.Error(), "already") {
+		t.Errorf("refusal reads %q; it should say the place is already spoken for", err)
+	}
+}
diff --git a/gui/internal/model/prefs.go b/gui/internal/model/prefs.go
index f5ffee5..9ae79a0 100644
--- a/gui/internal/model/prefs.go
+++ b/gui/internal/model/prefs.go
@@ -112,3 +112,41 @@ func (p Prefs) Save() error {
 	}
 	return os.WriteFile(file, append(data, '\n'), 0o644)
 }
+
+// Display is everything the Settings window can change about the window
+// itself. It deliberately leaves out the divider positions: those are set
+// by dragging, and a settings change must not disturb them. Composing a
+// whole Prefs in the Settings window instead is what used to write
+// PreviewWidth, ListWidth and ListHeight back as zeros, so dragging the
+// panes to taste and then ticking any checkbox threw the panes away.
+type Display struct {
+	Sort          string
+	Layout        string
+	ShowSize      bool
+	ShowAge       bool
+	ShowRule      bool
+	Colours       bool
+	Preview       bool
+	SelectAll     bool
+	PreviewHeight int
+}
+
+// DisplayOf is the part of p the Settings window shows.
+func (p Prefs) DisplayOf() Display {
+	return Display{
+		Sort: p.Sort, Layout: p.Layout,
+		ShowSize: p.ShowSize, ShowAge: p.ShowAge, ShowRule: p.ShowRule,
+		Colours: p.Colours, Preview: p.Preview, SelectAll: p.SelectAll,
+		PreviewHeight: p.PreviewHeight,
+	}
+}
+
+// WithDisplay is p with d applied and everything else - where each divider
+// was left - kept as it was.
+func (p Prefs) WithDisplay(d Display) Prefs {
+	p.Sort, p.Layout = d.Sort, d.Layout
+	p.ShowSize, p.ShowAge, p.ShowRule = d.ShowSize, d.ShowAge, d.ShowRule
+	p.Colours, p.Preview, p.SelectAll = d.Colours, d.Preview, d.SelectAll
+	p.PreviewHeight = d.PreviewHeight
+	return p
+}
diff --git a/gui/internal/model/prefs_test.go b/gui/internal/model/prefs_test.go
index 328ac31..69cc678 100644
--- a/gui/internal/model/prefs_test.go
+++ b/gui/internal/model/prefs_test.go
@@ -57,3 +57,30 @@ func TestPrefsWithoutAFile(t *testing.T) {
 		t.Errorf("with damaged text: %+v, want the defaults", got)
 	}
 }
+
+// TestWithDisplayKeepsTheDividers: the Settings window used to compose a
+// whole Prefs from its own controls, which wrote the remembered pane
+// positions back as zeros - so dragging the dividers and then ticking any
+// checkbox in Settings threw the positions away and the next window opened
+// at the defaults.
+func TestWithDisplayKeepsTheDividers(t *testing.T) {
+	p := Prefs{
+		PreviewWidth: 640, ListWidth: 800, ListHeight: 300, PreviewHeight: 281,
+		Sort: SortName, ShowSize: true, ShowAge: true, ShowRule: true,
+		Colours: true, Preview: true, SelectAll: true, Layout: LayoutSide,
+	}
+	d := p.DisplayOf()
+	d.ShowRule = false
+	d.Sort = SortSize
+	got := p.WithDisplay(d)
+
+	if got.PreviewWidth != 640 || got.ListWidth != 800 || got.ListHeight != 300 {
+		t.Errorf("the dividers were disturbed: %+v", got)
+	}
+	if got.ShowRule || got.Sort != SortSize {
+		t.Errorf("the change did not take: %+v", got)
+	}
+	if !got.ShowSize || !got.ShowAge || !got.Colours || !got.Preview || !got.SelectAll {
+		t.Errorf("something else changed: %+v", got)
+	}
+}
diff --git a/gui/internal/ui/settings.go b/gui/internal/ui/settings.go
index 32a6066..361f415 100644
--- a/gui/internal/ui/settings.go
+++ b/gui/internal/ui/settings.go
@@ -137,6 +137,9 @@ func (w *Window) showSettings() {
 		box.Append(l)
 	}
 
+	// Everything this window can change, applied over the preferences as
+	// they are: the divider positions belong to the panes, not here, and
+	// composing a whole Prefs from these controls wrote them back as zeros.
 	apply := func() {
 		which := model.LayoutSide
 		if layout.Selected() == 1 {
@@ -146,8 +149,9 @@ func (w *Window) showSettings() {
 		if i := int(sortOrder.Selected()); i >= 0 && i < len(model.SortOrders) {
 			order = model.SortOrders[i]
 		}
-		p := model.Prefs{
+		p := w.prefs.WithDisplay(model.Display{
 			Sort:          order,
+			Layout:        which,
 			ShowSize:      showSize.Active(),
 			ShowAge:       showAge.Active(),
 			ShowRule:      showRule.Active(),
@@ -155,17 +159,24 @@ func (w *Window) showSettings() {
 			Preview:       preview.Active(),
 			SelectAll:     selectAll.Selected() == 0,
 			PreviewHeight: int(previewHeight.Value()),
-			Layout:        which,
-		}
+		})
 		w.applyPrefs(p)
 		if err := p.Save(); err != nil {
 			w.setStatus("settings: %v", err)
 		}
 	}
+	// Every control, not four of them: sort, the three column toggles and
+	// the preview height were read by apply but never connected to it, so
+	// changing them did nothing until some other control happened to fire.
 	colours.ConnectToggled(apply)
 	preview.ConnectToggled(apply)
+	showSize.ConnectToggled(apply)
+	showAge.ConnectToggled(apply)
+	showRule.ConnectToggled(apply)
 	selectAll.Connect("notify::selected", func() { apply() })
 	layout.Connect("notify::selected", func() { apply() })
+	sortOrder.Connect("notify::selected", func() { apply() })
+	previewHeight.ConnectValueChanged(apply)
 
 	s.save = gtk.NewButtonWithLabel("Save krino.conf")
 	s.save.AddCSSClass("suggested-action")