Nnn: preview-tabbed and ^C

[cdarkly]

Environment details (Put x in the checkbox along with the information)

[x] Operating System: Debian Testing
[x] Desktop Environment: Spectrwm
[x] Terminal Emulator: xterm
[x] Shell: bash
[ ] Custom desktop opener (if applicable):
[x] Program options used: ac
[ ] Configuration options set:
[x] Issue exists on nnn master

Right now, the only way to cleanly stop preview-tabbed and all of its running processes is to quit nnn. If the preview-tabbed process is interrupted with CTRL+C, the "preview-tabbed" shell script is successfully terminated, but "tabbed" and "viewer" processes remain running.

On the "Live Previews" wiki it is noted that:

"To close the preview-tui and preview-kitty preview windows, press ^C on the preview window. Run the plugin again if you want to preview."

It would be nice if preview-tabbed could also be closed with ^C, and then simply restarted within nnn.

Exact steps to reproduce the issue

1. Start nnn
2. Start the preview-tabbed plugin
3. Press CTRL+C while in nnn
4. Look at the process list with ps or htop. It will indicate that tabbed and viewer processes are still running.

One possible implementation

The bash shell supports catching signals such as ^C using the built-in trap command. Run 'trap --help' in a bash shell for more details.

This code can be added to "preview-tabbed":

sigint_kill () {
kill_viewer
kill "$TABBEDPID"
#disown
exit 0
}

previewer_loop () {
...
trap sigint_kill SIGINT
while read -r FILE ; do
...
done
...
}

During initial tests, setting the SIGINT trap at the beginning of the the preview-tabbed script this did not work. The trap must be set inside the previewer_loop() function, because that function is forced to run in the background using &:

previewer_loop < "$NNN_FIFO" &

If "exit 0" is left out from sigint_kill(), than "tabbed" and "viewer" processes will get killed, but "preview-tabbed" will continue running.

Not sure if disown command also needs to be run before the script is terminated, so it's commented out in the above example.

More testing of this might be needed.

[jarun]

This defect looks to be the same as #723. Is there a difference? If not, please do not open duplicate defects unnecessarily OR explicitly point out the difference first.

From the fix perspective, did you try master which has #725?

[jarun]

Note that you'll have to manually get the affected plugin.

[cdarkly]

It is a similar problem. But, this is different. And I am using the #725 master in testing.

The original issue was with lingering processes when you quit from nnn.

This isssue is with lingering processes when you press ^C while running nnn.

[jarun]

Where are you pressing ^C? Inside nnn or inside the previewer?

[0xACE]

This isssue is with lingering processes when you press ^C while running nnn.

What are you expecting to happen when you hit ^C?

[leovilok]

It won't be as easy:

• if you press ^C in the nnn terminal window, the terminal emulator will send the signal to the foreground process, so it will be sent to nnn, not the prewiew plugin
• if you press ^C in the preview window, tabbed itself doesn't catch it, so it's transfered to the previewer program (so a terminal, image/video/document viewer, ...) ; again, the preview plugin never gets notified, it all happens in a child process

If you are using dwm, you can maybe comment this line:
https://github.com/jarun/nnn/blob/master/plugins/preview-tabbed#L191
and just switch to the preview window and close it (meta+shift+c)

If you are not using tabbed for anything else, you could also setup a global shotcut to kill all tabbed processes. In dwm's config.h:

{ MODKEY, XK_c, spawn,  {.v = (const char*[]){"pkill","tabbed",NULL}}},

[cdarkly]

jarun: I'm pressing ^C while inside nnn

0xACE: I only tried ^C in preview-tabbed after reading about using it for preview-tui in the "Live Previews" wiki. The expectation was that there could also be a way to "close" preview-tabbed and restart it without quitting nnn.

leovilok: It appears that ^C does get passed to the preview plugin from nnn. It certainly works for preview-tui. However, in the case of preview-tabbed, for the SIGINT bash trap to work, it has to be set inside the previewer_loop() function which is set to run in the background.

Here is an example of the implementation described above.
https://github.com/cdarkly/temporary-tests/blob/master/preview-tabbed.sigint

Maybe there is a better way of doing it, but this seems to work. As noted earlier, more testing is likely needed. I tried it on Debian running "dwm" and "spectrwm", and so far no problems.

[leovilok]

@cdarkly OK, great, I tested it, it works. I think that you can raise a PR.

[cdarkly]

There is one oddity with this so far:

1. If you start nnn and run "preview-tabbed", the file being previewed is displayed immediately. So this works as expected.
2. However, if you close "preview-tabbed" with ^C, and then reopen it, "tabbed" is launched, but the viewer on the current file is not All you see is a blank/gray tabbed window. Only if you select the next file within nnn using j or k, is the viewer lanuched.

What seems to be happening is that the preview-tabbed script stops on the "while read -r FILE" command, waiting for "path/filename/EOL" to show up in the NNN_FIFO pipe. Once the j or k keys are pressed in nnn, the "path/filename/EOL" of the newly select entry is sent to the NNN_FIFO pipe, and while loop gets executed.

What's interesting is that if you setup a simple "fifo_test" script like this:

!/bin/bash

while read -r FILE
do
echo "$FILE"
done < $1

Then

1. Run "nnn -a"
2. In a separate terminal window launch "./fifo_test /tmp/nnn-fifo.xxxx", where nnn-fifo.xxxx is the name of the current auto generated pipe.

When the fifo_test script is first lancuhed, it has the same problem. The first iteration of the while loop does not execute because there is nothing in the nnn fifo pipe. However, if you select another entry in nnn using j or k, then the "path/filename/EOL" gets printed, and everything behaves normally.

[cdarkly]

After some additional testing, the easy workaround to the "oddity" described above is to use ESC to resend the current "path/filename/EOL" to the FIFO.

So when preview-tabbed is closed with ^C and the restarted, simply press ESC to get the preview to show back up for the current item selected.

One question remaining is why does nnn send the "path/filename/EOL" to the FIFO when preview-tabbed is first launched, but not when it is relaunched after closing it with ^C?

[cdarkly]

Posted an alternative implementation:

https://github.com/cdarkly/temporary-tests/blob/master/preview-tabbed.sigint_flag

This is different in that it allows the preview script to exit using its normal path. Relevant changes are:

#Backup current FIFO path before it is unset
NNN_FIFO_INUSE=$NNN_FIFO
SIGINT_FLAG=0

sigint_exit () {
    SIGINT_FLAG=1
    #Send a new line to the FIFO to force the while loop to iterate and then exit with a break
    echo "" > $NNN_FIFO_INUSE
 }

trap sigint_exit SIGINT

while read -r FILE ; do
    if [ $SIGINT_FLAG -eq 1 ] ; then
         break
    fi
done

The previous implemenation was more concise.

Just posting this for your consideration.

[jarun]

@leovilok please take a look if this works fine.

[leovilok]

@jarun @cdarkly's alternative version seems to work fine too :+1:
I'm not sure why we would like to follow the normal path, but it seems like a generally good idea, so that if we need to do some additional clean-up at some point, we won't have to worry about the ^C case.

[jarun]

So can we have the PR?

[jarun]

@cdarkly I can't keep this defect open forever. Please raise the PR with your proposed solution above.

[cdarkly]

Sorry guys, did not realize you were waiting on me. I'm kind of new to this. :)