aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--gui/internal/model/plan.go18
-rw-r--r--gui/internal/model/plan_test.go38
-rw-r--r--gui/internal/model/prefs.go38
-rw-r--r--gui/internal/model/prefs_test.go27
-rw-r--r--gui/internal/ui/settings.go17
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")
generated by cgit v1.3 (git 2.53.0) at 2026-09-19 09:19:03 +0000