Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ToDo list #594

Closed
5 of 11 tasks
jarun opened this issue May 25, 2020 · 38 comments
Closed
5 of 11 tasks

ToDo list #594

jarun opened this issue May 25, 2020 · 38 comments
Labels

Comments

@jarun
Copy link
Owner

jarun commented May 25, 2020

Rolled from #506.

Ready for next release

  • key Esc or left click to resend hovered file path to NNN_FIFO
  • preview-tui: various improvements
  • F5 removed (misfit for toggle hidden), ^S removed (redundant and bug-prone)
  • gracefully handle abnormal terminations
  • make option O_CTX8 for 8 contexts (NOT backward compatible with 4 contexts)

Proposed features and tasks (up for grabs)

Anything else which would add value (please discuss in this thread).

@jarun jarun added the planning label May 25, 2020
@githububub
Copy link

githububub commented May 29, 2020

If you do proceed with additional contexts, please ensure that the feature can be compiled out. I personally do not see the need for more than four. My buddy aka rddit-nix pointed me in the direction of nnn and we both agree that launching a new instance is certainly more effective than tabbing through x contexts or hunting for the correct context number. YMMV.

@githububub
Copy link

githububub commented May 29, 2020

Not really a feature request or a bug, but if nnn -a is not closed gracefully (e.g. closed via a TWM or DE key bound to SIGTERM/SIGKILL), it seems as if /tmp will be littered with truncated nnn fifo files. I noticed around 100 such files in /tmp earlier today. While empty, they do constitute a trail of metadata. The proper resolution I suppose is to close gracefully and allow nnn to auto remove the fifos.

@jarun
Copy link
Owner Author

jarun commented May 29, 2020

The proper resolution I suppose is to close gracefully and allow nnn to auto remove the fifos.

The thing is, we do have that code in place. See cleanup(). I am not sure why it doesn't remove the file on a bad exit.

@leovilok any ideas?

@d3ni5
Copy link

d3ni5 commented May 29, 2020

Hi @jarun , I appreciate what you do, nnn is awesome and you know it.
But could you please tell me how to catch output from the plugin within nnn file manager?

I'm trying to get to work preview-tui plugin, but all that I see is a quick blink of the screen with no messages. Though I can see that inside of the plugin are some "echos". Is there a way to see them. Or the only thing is troubleshoot this is to mimic the environment of nnn and execute plugin externally?

Thanks for the answer in advance.

@0xACE
Copy link
Collaborator

0xACE commented May 29, 2020

Hello,

If you do proceed with additional contexts, please ensure that the feature can be compiled out. I personally do not see the need for more than four.

At the moment the plan is paused, but the intention is to keep nnn to use 4 contexts by default. Any additional contexts will be a compile time decision. But it mostly depends on how that discussion evolves. But the goal is to not change the current state of nnn, only provide the option as the current implementation is limited to 4 contexts or less.

is not closed gracefully (e.g. closed via a TWM or DE key bound to SIGTERM/SIGKILL),

I'm not sure we handle SIGTERM nor SIGKILL explicitly, they should fallback to the defaults. Could you try closing nnn by hitting q to see if the fifo disappears as expected?

@d3ni5

I think that script was reworked the last couple of days.

It should look something like this out of the box:
show case preview-tui
taken from #578 (comment)

Also what do you have $PAGER, $TERMINAL set to? Do you have less and xterm installed? Because that's what the script defaults to. In the gif i linked I'm running within a tmux environment which I find works best.

@d3ni5
Copy link

d3ni5 commented May 29, 2020

@jarun, thanks for your reply.

Recently I ran getplug plugin so I believe I have the most recent copy of preview-tui.
My $PAGER is set to less -R, and I didn't set $TERMINAL explicitly. But I see that it should fallback to xterm which I have installed.

I've just launched nnn without alias and it helped a bit. I see a spawned tmux panel but with an error:
No FIFO available! ($NNN_FIFO='')

My alias for nnn is the following:

# Start nnn with cd on quit functionality
n () {
    # Block nesting of nnn in subshells
    if [ "${NNNLVL:-0}" -ge 1 ]; then
        echo "nnn is already running"
        return
    fi

    # Unmask ^Q  (, ^V etc.)  (if required, see `stty -a`) to Quit nnn
    stty start undef
    stty stop undef

    NNN_FIFO="$(mktemp)" nnn -er "$@"

    # The default behaviour is to cd on quit  (nnn checks if NNN_TMPFILE is set)
    # To cd on quit only on ^G, export NNN_TMPFILE after the call to nnn
    # NOTE: NNN_TMPFILE is fixed, should not be modified
    NNN_TMPFILE="${XDG_CONFIG_HOME:-$HOME/.config}/nnn/.lastd"

    if [ -f "$NNN_TMPFILE" ]; then
        . "$NNN_TMPFILE"
        rm -f "$NNN_TMPFILE" > /dev/null
    fi
}

Most of the code I've got from the wiki, the only thing I've changed is the line:
NNN_FIFO="$(mktemp)" nnn -er "$@"
where I set $NNN_FIFO and launch nnn with desired parameters.
Can you please point out what is wrong here?

@githububub
Copy link

@0xACE A graceful q sure does remove the fifo (expected). Workstations will run nnn with the default options and servers will run O_NOFIFO := 1 for now :)

@0xACE
Copy link
Collaborator

0xACE commented May 29, 2020

I spent too much time on another issue so i have to keep my self short here

Recently I ran getplug plugin so I believe I have the most recent copy of preview-tui.

i think getplug targets "release candidates" and not necessarily master branch

It seems like you haven't read the docs, NNN_FIFO must be set or you can use the absolutely new flag that was added last week (not sure what it is, but it was probably nnn -a)

I can't review your script as im literally standing as im typing this message ready to leave. protip: there is an abundance of documentation see the wiki above.

@0xACE A graceful q sure does remove the fifo (expected). Workstations will run nnn with the default options and servers will run O_NOFIFO := 1 for now :)

Maybe we could hook in a SIGTERM signal handler, but I can't take this discussion at this moment, at least I think it's possible to handle SIGTERM correctly. But iirc SIGKILL is out of user space program's scope, and you shouldn't expect any program to clean up after that... Maybe SIGQUIT or w/e is more appropriate, but again, i gotta go. peace

@d3ni5
Copy link

d3ni5 commented May 29, 2020

@0xACE , thanks for your review.

I read the docs and I believe that I'm setting $NNN_FIFO while I'm launching nnn by this line:
NNN_FIFO="$(mktemp)" nnn -er "$@"
Please correct me if I'm wrong.

I tried to do exactly as docs stated with mktemp -u flag but it doesn't work and I'm not surprised as it doesn't create the file itself.

@jarun
Copy link
Owner Author

jarun commented May 29, 2020

Please use the -a flag to generate random fifo file.

@d3ni5
Copy link

d3ni5 commented May 29, 2020

OK, thanks, I guess I need to wait for a release branch as I don't have this flag still.

@jarun
Copy link
Owner Author

jarun commented May 29, 2020

It's available on v3.2. Fifo support is also on v3.2.

@leovilok
Copy link
Collaborator

The proper resolution I suppose is to close gracefully and allow nnn to auto remove the fifos.

The thing is, we do have that code in place. See cleanup(). I am not sure why it doesn't remove the file on a bad exit.

@leovilok any ideas?

As @0xACE said, we should call cleanup and exit on SIGTERM.

@jarun
Copy link
Owner Author

jarun commented May 29, 2020

Can you raise a PR?

@leovilok
Copy link
Collaborator

@jarun see #607

@d3ni5
Copy link

d3ni5 commented May 29, 2020

Guys, thanks for your support, I got 3.2-1 and everything went smoothly well.
Is it possible to start some plugins at the startup of nnn, for instance can I have some preview plugin automatically launched together with nnn?

@0xACE
Copy link
Collaborator

0xACE commented May 29, 2020

Is it possible to start some plugins at the startup of nnn, for instance can I have some preview plugin automatically launched together with nnn?

I'm guessing you could wrap nnn in a function/script that does this for you.

@leovilok
Copy link
Collaborator

@d3ni5 from nnn -h output:

 -P key  run plugin key

But I can't find it in the man, so that's an issue.
The second issue is that it doesn't work for me (I never tried it before) so I'll have to check that out 😆

@leovilok
Copy link
Collaborator

@d3ni5 : example

NNN_PLUG='p:preview-tui' nnn -a -P p

but you need to have no NNN_OPTS eported (I don't see the rationale behind that, sounds like a bug to me...)

@d3ni5
Copy link

d3ni5 commented May 29, 2020

Is it possible to start some plugins at the startup of nnn, for instance can I have some preview plugin automatically launched together with nnn?

I'm guessing you could wrap nnn in a function/script that does this for you.

@0xACE , I guess I'm not that advanced to catch your idea. Could you please clarify that? how would you wrap it? do you mean xdotool or there is another more elegant, easy and more efficient way to pass key stokes / invoke plugin automatically?

@d3ni5
Copy link

d3ni5 commented May 29, 2020

@d3ni5 from nnn -h output:

 -P key  run plugin key

But I can't find it in the man, so that's an issue.
The second issue is that it doesn't work for me (I never tried it before) so I'll have to check that out 😆

Thanks, @leovilok , I will try that. But I'm really curious about the approach that @0xACE suggests especially if this flag is not working.

@0xACE
Copy link
Collaborator

0xACE commented May 29, 2020

@0xACE , I guess I'm not that advanced to catch your idea. Could you please clarify that? how would you wrap it? do you mean xdotool or there is another more elegant, easy and more efficient way to pass key stokes / invoke plugin automatically?

I guess it stems from the unix philosophy of building one tool that does one job and does that job well, and then supplying an environment where you can adjust it to your needs.

Technically I will be over reaching with this explanation but I'm doing it in hope to inspire you.

The idea is that linux provides a shell that supports basic scripting where you can setup your requirements.

Earlier in this thread you posted a nnn wrapper. Imagine if you put:

...
NNN_FIFO=$(mktemp) # maybe export is required? my bashisms is based on trial-and-error
my_preview_script &
nnn
...

into that function. This way you have ensure that your preview script is executed whenever you launch nnn via this function.

Personally I have my path setup like such: export PATH=~/pre-bin:$PATH:~/bin

  • in ~/bin i put my various scripts for w/e
  • in ~/pre-bin i put my scripts that overwrite defaults of programs in my environment

Say I'm unhappy with chromium launching without --incognito, I put a script in ~/pre-bin/chromium which actually executes /usr/bin/chromium --incognito "$@"

You could have a simliar setup for nnn.

image

You will have to tinker with it though. Imho bash is so annoying to use I wish there was a better language for it. But I insist on using sh/bash because of the legacy of it, it practically exists on every machine based on unix

Regarding xdotool I can't recommend such tools under X11, I feel they all behave bad compared to Windows's autohotkey. But it is mostly because of X11 restrictions. For e.g. global hotkeys have to temporarily activate the target window to send the keystroke, I find it very annoying when I change my volume and trying to perform another action at the same time.

Become a master of your own environment.

I'll leave the exercise of it as homework for you, if you are interested. Cheers.

P.S. I don't use any of those *.desktop files provided by various files, my environment is very naked...

I spent too much time on this now. Good night.

@leovilok
Copy link
Collaborator

@d3ni5 the flag works ... just not with NNN_OPTS, but I submitted PR #610 to fix that.

About the wrapper method, to complement what @0xACE wrote:

You can use NNN_FIFO to write your wrapper preview script, checkout this exemple in the wiki for instance.

And remeber that plugins are just scripts so a simple wrapper function would be:

n () {
    export NNN_FIFO="$(mktemp -u)"
    mkfifo $NNN_FIFO
    ~/.config/nnn/plugins/preview-tui . &
    nnn
    rm $NNN_FIFO
    unset NNN_FIFO
}

(@jarun should this go in wiki too?)

@jarun
Copy link
Owner Author

jarun commented May 29, 2020

~/.config/nnn/plugins/preview-tui . &

What's this? It's a plugin and should be launched from within nnn with a plugin key.

@d3ni5
Copy link

d3ni5 commented May 30, 2020

Thanks @leovilok. And a great thanks to @0xACE for his wide explanation, really inspiring one.
Now I want to revisit my custom desktop files and think of implementing separate pre-bin like folder as well. Currently I'm using a single ~/bin directory which I put as the first in the $PATH. So here resides regular bins of mine and also the ones with modified behavior / additional flags. But your way feels more "readable".

I haven't even thought about running plugin script before actually launching nnn. I thought it'll not work by default, and now as I see you both mentioning this I will think again.

@githububub
Copy link

githububub commented May 30, 2020

Still some instances where fifos are truncated in /tmp with nnn -a. I'll try and figure out when and how they become truncated. But instead of 100+ fifos over a daily session, I now have only 4.

@jarun
rddit-nix is having suspend issues with some shiny new DRAM sticks, so he will be in strict hardware mode for the next few days (although he managed to post some nnn love in r/unixporn 🥇 earlier today). He wanted me to share what we have done with imv which is nothing other than basic substitution. Here is the section he uses in nuke:

abspath() {
    case "$1" in
        /*) printf "%s\n" "$1";;
        *)  printf "%s\n" "$PWD/$1";;
    esac
}

listimages() {
    find -L "$(dirname "$target")" -maxdepth 1 -type f -iregex \
      '.*\(jpe?g\|bmp\|png\|gif\|tiff\|svg\)$' -print0 | sort -z
}


img_load_dir() {
    target="$(abspath "$1")"
    count="$(listimages | grep -a -m 1 -ZznF "$target" | cut -d: -f1)"


    if [ -n "$count" ]; then
        listimages | xargs -0 imv-wayland -n "$count" --
    fi
}

handle_multimedia() {
    mimetype="${1}"
    case "${mimetype}" in
        image/*)
            if which imv-wayland >/dev/null 2>&1; then
                img_load_dir "${FPATH}" >/dev/null 2>&1 &
                exit 0
            fi
            exit 1;;

And here is the relevant section in my imgview:

abspath() {
    case "$1" in
        /*) printf "%s\n" "$1";;
        *)  printf "%s\n" "$PWD/$1";;
    esac
}

listimages() {
    find -L "$(dirname "$target")" -maxdepth 1 -type f -iregex \
      '.*\(jpe?g\|bmp\|png\|gif\|tiff\|svg\)$' -print0 | sort -z
}

img_load_dir() {
    target="$(abspath "$1")"
    count="$(listimages | grep -a -m 1 -ZznF "$target" | cut -d: -f1)"

    if [ -n "$count" ]; then
        listimages | xargs -0 imv -n "$count" --
    fi
}

if [ -z "$1" ] || ! [ -s "$1" ]; then
    printf "empty file"
    read -r _
    exit 1
fi

if command -v imv >/dev/null 2>&1; then
    if [ -f "$1" ]; then
        img_load_dir "$1" >/dev/null 2>&1 &
    elif [ -d "$1" ] || [ -h "$1" ]; then
        imv "$1" >/dev/null 2>&1 &
    fi
else
    printf "install imv"
    read -r _
    exit 2
fi

Essentially the same as the defaults 👍 . imv is a simple wrapper script which attempts to launch imv-wayland else imv-x11. Debian removed the imv script due to a naming conflict and hence, Debian users must call the desired executable directly. imv is able to preview hovered directories with imgview (though there is a known issue with unsorted views) and is able to view images according to folder sort within a directory (both imgview and nuke). Hope that helps.

Offhand, I am not sure how suckless imv intends to be now having moved to a meson/ninja build system :(

@jarun
Copy link
Owner Author

jarun commented May 30, 2020

Thanks for sharing!

@leovilok can we integrate imv as well?

@githububub please try master and confirm if FIFO removal works as expected.

@githububub
Copy link

githububub commented May 30, 2020

Will do. FYI, I was testing with e47a048 all day today. I'll try the updated master and report back in a day or two.

@jarun
Copy link
Owner Author

jarun commented May 30, 2020

I was testing with e47a048 all day today.

That's the relevant commit. In that case can help us figure in which case the FIFO is not deleted?

@jarun
Copy link
Owner Author

jarun commented May 30, 2020

I added removal of NNN_PIPE as well.

@leovilok
Copy link
Collaborator

@githubhub do you kill nnn with SIGKILL (kill -9) ? or do you close nnn term just after launching nnn, before it's even started?

imv looks nice, but it conflicts with renameutils (imv is an interactive file move/renamer using readline), I'll try it later.

@githububub
Copy link

githububub commented May 31, 2020

I was testing with e47a048 all day today.

That's the relevant commit. In that case can help us figure in which case the FIFO is not deleted?

Absolutely. And thanks for taking care of the pipe as well. As an aisde, advcpmv is interesting. The progress meter flies by fast 100 MB files :)

@githubhub do you kill nnn with SIGKILL (kill -9) ? or do you close nnn term just after launching nnn, before it's even started?

imv looks nice, but it conflicts with renameutils (imv is an interactive file move/renamer using readline), I'll try it later.

SIGTERM (kill -15). And the latter situation where nnn terms are closed in rapid SIGTERM succession where multiple instances of a terminal are open (not through a multiplexer) seems to be a case where fifos are left truncated. Will observe over the next couple of days.

@leovilok
Copy link
Collaborator

@githububub I can't replicate 😞
I did the following little experiment:

  1. Open 100 xterms with nnn:
    for i in {1..100} ; do xterm -e ./nnn -a & done
  2. Look in /tmp:
    ls /tmp/nnn-fifo* | wc -l # returns 100
  3. kill every nnn window
    pkill nnn
    # I also tried `killall -15 nnn` and `pkill xterm` to be sure
  4. Look in tmp again
    ls /tmp/nnn-fifo* | wc -l # returns 0

Do you have the same result?

@0xACE
Copy link
Collaborator

0xACE commented May 31, 2020

Do you have the same result?

I tried your test and origin/master works for me. Haven't reviewed the code though, and can't set aside time for that atm

@githububub
Copy link

I too receive the same results when following the experiment above. When testing under those standard conditions, all is well. There is something else at play here. Perhaps an edge case which we need not be concerned about. I have seven truncated fifos in /tmp over the course of six hours. Again, perhaps an edge case that involves the WM/term/other variables (though logs indicate nothing out of the ordinary). No pipes, only fifos. I will keep an instance of nnn open on the side (watching over as fifos are created and destroyed) while working over the next few days and try to pinpoint what is causing this.

@harriott
Copy link

harriott commented Jun 3, 2020

Reinstate alignment of nodes listed in Batch rename #529. It was implemented for a while, and was very helpful for me, then, before I could give my thanks, the thread was closed, and it was de-implemented. Can it be brought back?

@jarun
Copy link
Owner Author

jarun commented Jun 3, 2020

@harriott it's still there. Can you please confirm your copy of the plugins is the latest?

@jarun jarun mentioned this issue Jun 3, 2020
7 tasks
@jarun jarun closed this as completed Jun 3, 2020
Repository owner locked as resolved and limited conversation to collaborators Jun 3, 2020
@jarun
Copy link
Owner Author

jarun commented Jun 3, 2020

Rolled at #629.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants