Refresh preview even if selection doesn't change

[joelim-work]

Fixes #1055

The loadFile function is responsible for refreshing the preview, which should happen even if the selected file hasn't changed (e.g. if the terminal is resized or if a shell command is run). I believe this is a bug that was accidentally introduced when adding the on-select command.

[ilyagr]

@laktak, I think you last changed this code. Any concerns about this PR?

[laktak]

@laktak, I think you last changed this code. Any concerns about this PR?

Haven't tested it but it looks reasonable.

[joelim-work]

It shouldn't be too hard to setup image previews. I was able to reproduce the issue by following the instructions in the wiki, which is just creating a few shell scripts and ensuring ueberzug and ffmpegthumbnailer are installed.

If anyone does have image previews setup already, I would appreciate it if they could verify that this patch works for them and not just myself.

[eeeXun]

I've tested it. It works for me

[DusanLesan]

I see issues when moving window from screen to screen in dwm with ctpv (I think only ctpv has this issue but it worked fine before r28). Unlike in current behavior, it is not updated when moving focus up or down and all other images are affected. There are issues even when moving in the same size tile as it had before the move.

Image of ctpv issue

``

[joelim-work]

@DusanLesan Sorry I didn't quite understand - when you say that you're having issues, are you referring to version r28 or my patch?

I don't use ctpv myself, but if you still have issues then you can try to see when the previewer and cleaner are being called, by making them shell scripts which write to a log file before invoking ctpv.

[DusanLesan]

@joelim-work Sorry, my bad. I checked out your patch and saw this issue. Later I didn't manage to reproduce it on the main branch. But ctpv issue is not new and I have now seen it on the main too.

[joelim-work]

@joelim-work Sorry, my bad. I checked out your patch and saw this issue. Later I didn't manage to reproduce it on the main branch. But ctpv issue is not new and I have now seen it on the main too.

In my case, the image previews were working fine up to version r27, but then after upgrading to r28 they now have issues refreshing properly. However after making this change, this issue doesn't happen anymore, at least not for me 😃.

What should happen normally is that the loadFile function will signal the preview to be cleared by writing an empty string to the app.nav.previewChan channel, and then call loadReg, which in turn signals the current file to be previewed by writing its path also to app.nav.previewChan.

lf/ui.go

Lines 692 to 700 in b47cf6d

	if volatile {
	app.nav.previewChan <- ""
	}
	if curr.IsDir() {
	ui.dirPrev = app.nav.loadDir(curr.path)
	} else if curr.Mode().IsRegular() {
	ui.regPrev = app.nav.loadReg(curr.path, volatile)
	}

These are then eventually picked up in the previewLoop function, which will then invoke the previewer and cleaner programs accordingly:

lf/nav.go

Lines 632 to 661 in b47cf6d

	for path := range nav.previewChan {
	clear := len(path) == 0
	loop:
	for {
	select {
	case path = <-nav.previewChan:
	clear = clear || len(path) == 0
	default:
	break loop
	}
	}
	win := ui.wins[len(ui.wins)-1]
	if clear && len(gOpts.previewer) != 0 && len(gOpts.cleaner) != 0 && nav.volatilePreview {
	nav.exportFiles()
	exportOpts()
	cmd := exec.Command(gOpts.cleaner, prev,
	strconv.Itoa(win.w),
	strconv.Itoa(win.h),
	strconv.Itoa(win.x),
	strconv.Itoa(win.y))
	if err := cmd.Run(); err != nil {
	log.Printf("cleaning preview: %s", err)
	}
	nav.volatilePreview = false
	}
	if len(path) != 0 {
	nav.preview(path, win)
	prev = path
	}
	}

The problem in r28 is that the loadFile function returns early if the selected file hasn't changed, in which case none of the above code will be called.

@DusanLesan If you want to get to the bottom of your issue (again I'm not sure if it's caused by lf or ctpv), you can try inserting log statements like below and then running lf -log <logfile> and tail -f <logfile> to verify that the cleaner and previewer programs are invoked as expected:

--- a/nav.go
+++ b/nav.go
@@ -644,6 +644,7 @@ func (nav *nav) previewLoop(ui *ui) {
                if clear && len(gOpts.previewer) != 0 && len(gOpts.cleaner) != 0 && nav.volatilePreview {
                        nav.exportFiles()
                        exportOpts()
+                       log.Printf("cleaning: %s", prev)
                        cmd := exec.Command(gOpts.cleaner, prev,
                                strconv.Itoa(win.w),
                                strconv.Itoa(win.h),
@@ -749,6 +750,7 @@ func (nav *nav) preview(path string, win *win) {
        if len(gOpts.previewer) != 0 {
                nav.exportFiles()
                exportOpts()
+               log.Printf("previewing: %s", path)
                cmd := exec.Command(gOpts.previewer, path,
                        strconv.Itoa(win.w),
                        strconv.Itoa(win.h),

[DusanLesan]

@joelim-work Looks like ctpv issue. In this code everything looks to be sent as expected. If I open lf on second screen I get the same preview parameters like when I open on the first screen and then move to second.

[ilyagr]

Thinking out loud: I wonder whether this PR also helps with #914. I don't have a very good understanding of that issue, though.

[joelim-work]

@ilyagr I commented on that issue in regards to what I think is happening, see #914 (comment)

I'm quite sure it's a separate issue - this pull request is about the previewer/cleaner not being invoked, whereas #914 is caused by the directory cache being stale (the default directory preview doesn't invoke the previewer/cleaner anyway).

[ilyagr]

Your theory in that bug sounds very reasonable. Thank you so much for investigating it!