diff options
| -rw-r--r-- | gui/internal/model/preview.go | 19 | ||||
| -rw-r--r-- | gui/internal/model/preview_test.go | 56 | ||||
| -rw-r--r-- | gui/internal/ui/plan.go | 17 | ||||
| -rw-r--r-- | gui/internal/ui/window.go | 17 | ||||
| -rw-r--r-- | man/krino-gui.1 | 2 |
5 files changed, 104 insertions, 7 deletions
diff --git a/gui/internal/model/preview.go b/gui/internal/model/preview.go index 6ca029e..d2a2830 100644 --- a/gui/internal/model/preview.go +++ b/gui/internal/model/preview.go @@ -30,6 +30,10 @@ type Preview struct { Image string // a file to show: the file itself, or a rendered page Text string Note string // what this is, or why there is nothing + // Dir is the directory a rendered page was written to, for the caller + // to remove once it is done with the image; "" when nothing was + // rendered and the file itself is being shown. + Dir string } // previewBytes is how much of a text file is read, and previewLines how @@ -72,17 +76,28 @@ func MakePreview(ctx context.Context, path, tmp string) Preview { // pdfPreview renders the first page if poppler can, and falls back to the // document's text - the same pdftotext krino's own (content ...) tests use. +// +// Each render goes in a directory of its own. pdftoppm names its output +// after the page number, so a shared directory would hold several files +// called page-1.png and the wrong one could be picked up - which is what +// happened: a preview showed the page of a PDF looked at earlier (his +// report, 2026-09-17). func pdfPreview(ctx context.Context, path, tmp string, sz int64) Preview { if _, err := exec.LookPath("pdftoppm"); err == nil { - out := filepath.Join(tmp, "page") + dir, err := os.MkdirTemp(tmp, "page-") + if err != nil { + return Preview{Note: err.Error()} + } + out := filepath.Join(dir, "page") cmd := exec.CommandContext(ctx, "pdftoppm", "-png", "-f", "1", "-l", "1", "-scale-to", "700", "--", path, out) if err := cmd.Run(); err == nil { if rendered := firstMatch(out + "*.png"); rendered != "" { - return Preview{Kind: PreviewImage, Image: rendered, + return Preview{Kind: PreviewImage, Image: rendered, Dir: dir, Note: fmt.Sprintf("page 1, %s", size(sz))} } } + os.RemoveAll(dir) } if _, err := exec.LookPath("pdftotext"); err != nil { return Preview{Note: fmt.Sprintf("PDF, %s - install poppler-utils to see it", size(sz))} diff --git a/gui/internal/model/preview_test.go b/gui/internal/model/preview_test.go index 8c19922..b620a92 100644 --- a/gui/internal/model/preview_test.go +++ b/gui/internal/model/preview_test.go @@ -5,6 +5,7 @@ package model import ( "context" "os" + "os/exec" "path/filepath" "strings" "testing" @@ -96,3 +97,58 @@ func TestPreviewShowsOnlyTheHead(t *testing.T) { t.Error("a cut file does not say it was cut") } } + +// TestPdfPreviewsDoNotMixUp: two PDFs looked at one after the other each +// get their own rendered page. pdftoppm names its file after the page +// number, so previews sharing a directory would collide - one of his ended +// up showing another document's cover. +func TestPdfPreviewsDoNotMixUp(t *testing.T) { + if _, err := exec.LookPath("pdftoppm"); err != nil { + t.Skip("pdftoppm is not installed") + } + if _, err := exec.LookPath("groff"); err != nil { + t.Skip("groff is not installed, so there is nothing to make a PDF with") + } + dir := t.TempDir() + pdf := func(name, text string) string { + p := filepath.Join(dir, name) + f, err := os.Create(p) + if err != nil { + t.Fatal(err) + } + defer f.Close() + cmd := exec.Command("groff", "-Tpdf", "-ms") + cmd.Stdin = strings.NewReader(".TL\n" + text + "\n") + cmd.Stdout = f + if err := cmd.Run(); err != nil { + t.Skipf("groff cannot write a PDF here: %v", err) + } + return p + } + first := pdf("first.pdf", "The first document") + second := pdf("second.pdf", "The second document") + + tmp := t.TempDir() + a := MakePreview(context.Background(), first, tmp) + b := MakePreview(context.Background(), second, tmp) + if a.Kind != PreviewImage || b.Kind != PreviewImage { + t.Fatalf("previews = %+v, %+v", a, b) + } + if a.Image == b.Image { + t.Fatalf("both previews point at %s", a.Image) + } + sameBytes := func(x, y string) bool { + bx, err1 := os.ReadFile(x) + by, err2 := os.ReadFile(y) + return err1 == nil && err2 == nil && string(bx) == string(by) + } + if sameBytes(a.Image, b.Image) { + t.Error("the second preview is a copy of the first page rendered") + } + // The caller is told where to clean up. + for _, p := range []Preview{a, b} { + if p.Dir == "" { + t.Error("a rendered page comes with no directory to remove") + } + } +} diff --git a/gui/internal/ui/plan.go b/gui/internal/ui/plan.go index 97d8599..f4ecb91 100644 --- a/gui/internal/ui/plan.go +++ b/gui/internal/ui/plan.go @@ -45,6 +45,7 @@ type planView struct { previewScroll *gtk.ScrolledWindow previewTmp string previewFor string + renderedDir string previewOff bool startSelected bool menu *gtk.Popover @@ -573,9 +574,16 @@ func (p *planView) showPreview(rel string) { p.picture.SetFilename(pv.Image) p.picture.SetVisible(true) p.pictureFrame.SetVisible(true) + // The picture is loaded, so the page rendered for the row + // before it can go. + p.dropRendered() + p.renderedDir = pv.Dir case model.PreviewText: p.previewText.Buffer().SetText(escapeText(pv.Text)) p.previewScroll.SetVisible(true) + p.dropRendered() + default: + p.dropRendered() } }) } @@ -593,8 +601,17 @@ func (p *planView) applyPrefs(prefs model.Prefs) { } } +// dropRendered removes the page rendered for the previous row. +func (p *planView) dropRendered() { + if p.renderedDir != "" { + os.RemoveAll(p.renderedDir) + p.renderedDir = "" + } +} + // closePreview removes what the previews left behind. func (p *planView) closePreview() { + p.dropRendered() if p.previewTmp != "" { os.RemoveAll(p.previewTmp) p.previewTmp = "" diff --git a/gui/internal/ui/window.go b/gui/internal/ui/window.go index 9727846..819591c 100644 --- a/gui/internal/ui/window.go +++ b/gui/internal/ui/window.go @@ -58,6 +58,19 @@ func NewWindow(app *gtk.Application, e *engine.Engine) *Window { } }) + // Settings sits at the end of the tab strip - the window's top right - + // with a gear beside the word (his request, 2026-09-17). + settings := gtk.NewButton() + settingsBox := gtk.NewBox(gtk.OrientationHorizontal, 6) + settingsBox.Append(gtk.NewImageFromIconName("emblem-system-symbolic")) + settingsBox.Append(gtk.NewLabel("Settings")) + settings.SetChild(settingsBox) + settings.SetHasFrame(false) + settings.SetTooltipText("krino's defaults, and how this window behaves") + settings.SetMarginEnd(6) + settings.ConnectClicked(func() { w.showSettings() }) + notebook.SetActionWidget(settings, gtk.PackEnd) + w.status = gtk.NewLabel("") w.status.SetXAlign(0) w.status.SetMarginStart(8) @@ -65,13 +78,9 @@ func NewWindow(app *gtk.Application, e *engine.Engine) *Window { w.status.SetMarginTop(4) w.status.SetMarginBottom(4) - settings := gtk.NewButtonWithLabel("Settings") - settings.SetHasFrame(false) - settings.ConnectClicked(func() { w.showSettings() }) statusRow := gtk.NewBox(gtk.OrientationHorizontal, 6) w.status.SetHExpand(true) statusRow.Append(w.status) - statusRow.Append(settings) box := gtk.NewBox(gtk.OrientationVertical, 0) notebook.SetVExpand(true) diff --git a/man/krino-gui.1 b/man/krino-gui.1 index 881637b..675ac94 100644 --- a/man/krino-gui.1 +++ b/man/krino-gui.1 @@ -128,7 +128,7 @@ and, if the file changed on disk since it was opened, offers to reload, overwrite or cancel. Nothing on disk changes until Apply, Undo or Save. .Ss Settings .Cm Settings , -at the bottom of the window, holds two things. The first is krino's own +at the top right of the window, holds two things. The first is krino's own defaults - case, fold, recursive, min-age, max-read, max-size, busy, on-conflict and the log path - which every directory inherits and its own file may override; they are written to |