Avoid refreshing previews for shell pipe commands

[joelim-work]

Follow up of #1164

Like shell-async commands, shell-pipe commands are run in the background and can take some time to complete, so there is no point refreshing the preview immediately. When the shell-pipe command finishes, it internally sends a load command, so that should be sufficient.

For shell and shell-wait commands, the loading is handled in the runCmdSync function, see #1243 for details.

---

I did notice that % triggers the load command afterwards but & doesn't, I'm not sure if there's a reason for this though, but I think it can be addressed separately outside of this PR.

[ilyagr]

I think this is a good idea and it seems safe. In fact, I was considering doing this in #1243. I'm not quite sure if there was some weird bug those two lines fixed, though.

I was also considering adding some form of refresh when the command stops executing, in the two go func()-s below that line. Again, I'm not sure whether that can cause some subtle bug (e.g. an infinite loop).

I'll keep thinking about it, but it's also possible that at this point you and @gokcehan might be better experts on subtleties of changing such behavior than I am.

[joelim-work]

I think this is a good idea and it seems safe. In fact, I was considering doing this in #1243. I'm not quite sure if there was some weird bug those two lines fixed, though.

Looking back at the original code, I'm quite sure app.ui.loadFile was used just for refreshing the preview after a $ or ! command finishes and for & and % commands it would be a no-op since they would still be running in the background. The problem is that app.ui.loadFile actually has the side effect of running the cleaner and previewer` script (noticiable for image previews).

I was also considering adding some form of refresh when the command stops executing, in the two go func()-s below that line. Again, I'm not sure whether that can cause some subtle bug (e.g. an infinite loop).

I did notice that % triggers the load command afterwards but & doesn't, I'm not sure if there's a reason for this though, but I think it can be addressed separately outside of this PR.

[ilyagr]

I think you are likely correct. I went spelunking through the blame, and the code seems to originate in 52fbe46.

I think that, unless @gokcehan sees a reason not to, we can merge this and see if any bugs appear (which is, hopefully, unlikely).

The problem is that app.ui.loadFile actually has the side effect of running the cleaner and previewer` script (noticiable for image previews).

I'd put a sentence about this in the PR description, perhaps with a link to #414 (comment).

I did notice that % triggers the load command afterwards but & doesn't, I'm not sure if there's a reason for this though, but I think it can be addressed separately outside of this PR.

👍

I also realized we already talked about this in #1164 (comment)

[joelim-work]

I also realized we already talked about this in #1164 (comment)

BTW I did some quick testing, adding a load call at the end of a & command (similar to %) actually reintroduces the bug in #414 (comment), at least on my setup. The reason is that image previews are always volatile (they cannot be cached inside lf and the cleaner/previewer scripts need to run every time to display the image preview), and load will trigger this, causing the image to flash.

This is also why the period timer performs a non-volatile load, otherwise image previews would constantly be flashing, see #546:

lf/app.go

Lines 467 to 470 in a9d90bc

	case <-app.ticker.C:
	app.nav.renew()
	app.ui.loadFile(app, false)
	app.ui.draw(app.nav)

If you can make the load command accept some kind of nonvolatile argument and pass it to app.ui.loadFile then that should work.

[gokcehan]

@ilyagr I don't remember anything specific about this behavior. Let's merge it and see how it goes.

@joelim-work Thanks for the patch.