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 #1219

Closed
jarun opened this issue Nov 4, 2021 · 64 comments
Closed

ToDo list #1219

jarun opened this issue Nov 4, 2021 · 64 comments
Labels

Comments

@jarun
Copy link
Owner

jarun commented Nov 4, 2021

Rolled from #1193.

Cooking

  • disable filter info if file details (option -i) enabled
  • plugin gitroot to jump to git root directory from a subtree
  • icon for opus and webp files
  • preview-tui - fix gif conversion and whitespace name
  • preview-tui - add support for windows terminal split
  • nuke - add support for imv when named imv
  • add GNU sed as a dependency with support for env var SED
  • documentation refresh
  • make option O_NOSORT to load directories unsorted on entry

Up for grabs

None open at the time.

For anything else please discuss in this thread.

Contribution guideline.

@jarun jarun added the bug label Nov 4, 2021
@jarun jarun mentioned this issue Nov 4, 2021
17 tasks
@jarun jarun added planning and removed bug labels Nov 23, 2021
@rieje
Copy link

rieje commented Dec 8, 2021

Regarding this issue, is it not more sensible to add a null character at the end to make $NNN_SEL null-delimited for simpler processing in a simple while loop? Just a minor thing, xargs works and so does while IFS= read -r -d '' file || [ -n "$file" ] ; do but while IFS= read -r -d '' file seems simplest but does not currently include the last item because it doesn't end with the null character. I could not find anywhere that $NNN_SEL should be null-delimited but it seems like a reasonable assumption.

@jarun
Copy link
Owner Author

jarun commented Dec 8, 2021

There was a reason to do that in core nnn I don't remember. Now it would take lot more changes in a stable code.

I don't think tools need null-termination, they need null-separation. @N-R-K @KlzXS can you please help with the script?

@N-R-K
Copy link
Collaborator

N-R-K commented Dec 8, 2021

What's the usecase here, just processing the selection in bash script? If that is indeed the usecase then you can manually append the null char at the end, no?

{ cat "$NNN_SEL" && printf '\0'; } | while read -d $'\0' ...

I suppose something like this would work. Untested.

@rieje
Copy link

rieje commented Dec 8, 2021

Yes, there are many workarounds/solutions. My point was wouldn't it be simpler to make $NNN_SEL null-delimited so that it's more familiar for users where they don't need to resort to manually adding the null character with printf or use xargs in the manner described in the issue? The data should just be processed as-is if possible for the sake of simplicity--null-delimited does that and the user might reasonably assume it is null-delimited if they weren't paying close attention to the lack the last null character.

I actually manually added the null character like you mentioned but also wasn't confident this would not result in unexpected behavior. A familiar null-delimited $NNN_SEL is more explicit and commonly handled to prevent potential confusion or simple "workarounds" that is a bit of an inconvenience.

If there are good reasons to omit the last null character then this is a non-issue. I don't have strong opinions on the matter, just felt like others might encounter this minor annoyance I had, especially when I couldn't find much info about $NNN_SEL other than it's a selection file. So I took a glance at the contents and see null characters between filenames and then assumed it was null-delimited, hence my experience.

@jarun
Copy link
Owner Author

jarun commented Dec 8, 2021

NUL is used as a separator, not a terminator in the code.

@pepper-jelly
Copy link

pepper-jelly commented Dec 10, 2021

In troubleshooting section of wiki written:

Can't list within list

List view is a temporary view of symlinks. To avoid remembering the temporary paths to all listed views the current path is removed when the next list is loaded. This is also inline with the fact that find doesn't look within symlinks to directories. If you need to search the entries within a loaded list, visit the original directory and run the refined search.

Description confusing without context. What if call it e.g. a piped list or add example it will be much clearer?

@dorsiflexion
Copy link

Could you please add a note/music file icon for .opus files?

@rieje
Copy link

rieje commented Dec 18, 2021

When a file is deleted externally (not by plugin), nnn refreshes and cursor goes to the top. Is it possible to keep cursor at same position (or on the next file, if cursor was on file that was deleted) after it refreshes?

@jarun
Copy link
Owner Author

jarun commented Dec 18, 2021

First, this happens only when you are hovering on a file that's one of the deleted files.

External directory change (including deletions) come as events. We do not distinguish between single and multiple events combined together. We don't want to go into every event and check if it's a deletion and the file is hovered and then process the next one and check if the next file is also deleted and so on. That's inefficient. If we don't find the hovered file on refresh we simply move to the top.

@PatrickF1
Copy link
Contributor

Hi (and sorry for butting into the existing conversations--I figured this is a good place to brain dump on ideas), I'm considering writing a plugin called zoxide that simply calls zoxide for the user to quickly cd. I know there's already zoxide integration with autojump, but this would be more instant and fits the paradigm of these tools in that the user only has to execute one command to cd at the risk of going to the wrong directory. Is that something that you'd merge in?

@jarun
Copy link
Owner Author

jarun commented Dec 25, 2021

Please have a personal plugin of your own. It's too basic and a duplicate.

@PatrickF1
Copy link
Contributor

PatrickF1 commented Dec 25, 2021 via email

@PatrickF1
Copy link
Contributor

In case I wasn't clear enough, it's not a duplicate of autojump because it wouldn't scan the data of zoxide and use fzf for the user to select a directory as autojump does. It would simply prompt the user for a directory string and pass it onto zoxide to cd.

@jarun
Copy link
Owner Author

jarun commented Dec 25, 2021

No separate plugin for directory jumps please.

@PatrickF1
Copy link
Contributor

PatrickF1 commented Dec 25, 2021 via email

@jarun
Copy link
Owner Author

jarun commented Dec 25, 2021

We don't want to provide the same solution in multiple plugins. And personally I dislike the idea that we have a plugin for navigation when nnn already has tons of ways to enhance navigation.

You can copy the same plugin in the plugins dir without the .. There's no need to expose it for everyone. nnn provides these minor X-integration points but it's not a file manager that spoon-feeds users.

@PatrickF1
Copy link
Contributor

Thanks for explaining. I can empathize on both remarks. As the plugin author of a fzf plugin myself that also supports a bit of file navigation and previews, I also felt very wary of tons of requests and PRs to bulk up the search files feature when all I wanted to do was just rely on nnn. I ended up caving in to some requests and now have extra code and features to support and regret it. So should I not have even PRed in gitroot? Anyway, I will stop requesting other plugins like it.

@jarun
Copy link
Owner Author

jarun commented Dec 25, 2021

gitroot is useful and also unique. We didn't have a similar plugin before it.

@PatrickF1
Copy link
Contributor

In that case, great!

On the topic of core plugins, I was really confused by how “core plugins” are treated inconsistently and insufficiently. By core plugins I mean plugins such as .cbcp, .nmv, lock, launch. All of them have direct keybindings in nnn (meaning they can be triggered without assigning them to NNN_PLUG). Yet, .cbcp and .nmv are hidden but the others are not. It's also surprising that the man pages and the readme and wiki make no mention that lock and launch are the plugins behind the lock and launch actions in nnn. In my mind, plugins provide secondary functionality and are only ever called if they are assigned to NNN_PLUG, but not all of them. So I would suggest renaming them or moving them to another directory that clearly shows they are core plugins. Or at least add some documentation somewhere to explain their existence. (Sorry I'm kind of rambling here--not sure what the best solution is either)

@PatrickF1
Copy link
Contributor

Oh and one more thing: symlinked bookmarks are conspicuously missing in the manpages. Happy to try adding it myself once I get the green light from you!

@N-R-K
Copy link
Collaborator

N-R-K commented Dec 26, 2021

symlinked bookmarks are conspicuously missing in the manpages

They are mentioned in the wiki https://github.com/jarun/nnn/wiki/Basic-use-cases#symlinked-bookmarks

It was a relatively new feature, so I assume the manpage was forgotten to be updated.

@jarun
Copy link
Owner Author

jarun commented Dec 26, 2021

@N-R-K please add a note on symlinked bookmarks in the manpage.

@PatrickF1 nnn is very stable and users are fine with how things are. I myself have greater priorities in life than cosmetic changes like moving plugins between folders. I understand you just found nnn and probably got overwhelmed but it's just another basic tool. Feel free to share ideas which really enhance the functionality.

@PatrickF1
Copy link
Contributor

Gotcha. Yeah I did get overwhelmed 😅. Just to give you a data point--though one admittedly probably exceptional--I spent about 12 hours over two days doing intense research mastering nnn. It was rather difficult because of holes in the documentation and the things that don't follow conventions--implicit or stated. I now feel very comfortable with it but am also very aware of its odd design flaws. Thanks for the feedback--going forward I will focus more on significant enhancements.

@PatrickF1
Copy link
Contributor

Oh yes--sorry one more thing that's rather cosmetic but could make a big difference for someone new to nnn. The sort prompt that shows up when you hit t is incredibly confusing even. Would recommend just displaying the whole thing or elaborating in the manpages (right now it's only elaborated on the Wiki).
Also it's still not clear to me what the ^T to get notifications on Mac means. Could you clarify that for me please?

@jarun
Copy link
Owner Author

jarun commented Dec 26, 2021

The following is available in the in-program help (press ? to see it):

t ^T Sort toggles

The program keys are not documented in the man page.

^T to get notifications on Mac

This is unrelated to nnn. Mac users can press ^T to see cp/mv progress.

@PatrickF1
Copy link
Contributor

But that will somewhat break the visual for those who like the current sync of statusbar and context color.

I think you could just add a symbol. For example, fzf does a good job here. Try opening up fzf --multi (for multi select) and fzf without multi select. They have a one new symbol with --multi to indicate that multiselect is on.

@jarun
Copy link
Owner Author

jarun commented Dec 30, 2021

+ isn't supposed to have any feedback. You know whether it's on or off when you open the next file.

Type-to-nav is a matter of habit. People who use that mode are supposed to use that all the time. For example, I use it and I never press the filter key. No new indicators would be added.

@jarun
Copy link
Owner Author

jarun commented Dec 31, 2021

Perhaps we can take a page out of less indicates whether or not there's more to view?

Now that you know about v, it shouldn't be an issue to understand what it means. Again, I don't think doing these cosmetic changes at this stage of the program adds any real value. We are also not adding too many new changes so you can change these yourselves in your local copy.

@jarun
Copy link
Owner Author

jarun commented Dec 31, 2021

And I don't even think v and ^ can be replaced by a percentage. How do you know at a glance whether there are more elements if you scroll up or not? And a percentage is an approximation. In a directory having 10000 files, several last entries would show 100%.

In any case, @PatrickF1 I think you need to stop now. We did come across a few contributors/users earlier who would come up with repeat cosmetic improvement suggestions without doing enough homework. And we had to spend time figuring out the actual problems in implementing those ideas and explaining those to them. It's a bad experience and I don't want to spend my time on those. nnn can be modified very easily. Feel free to make changes in your local copy and use them.

@PatrickF1
Copy link
Contributor

PatrickF1 commented Dec 31, 2021 via email

@jarun
Copy link
Owner Author

jarun commented Dec 31, 2021

No problem! Thanks for your understanding.

@N-R-K
Copy link
Collaborator

N-R-K commented Dec 31, 2021

Since the topic of UX/UI came up, I'd like to add that there are many terminal file mangers which focus quite heavily on cosmetics and looks. Many of which I've installed, and then uninstalled within 2 minutes after seeing that there's a noticeable delay when doing basic tasks like scrolling, going in/out of a directory etc.

In attempt to make their UI better, they've ended up making it unuseable for people like me who are sensitive towards latency and responsiveness. When making UI decisions one has to keep in mind, that in the unix world worse is better.

@WanderLanz
Copy link

Yo just leaving this here after altering Preview-tui to work with Windows Terminal with WSL, as I've avoided Xserver for various reasons. I'm pretty new to Bash scripting so do with it what you will I didn't add much, just really liked the project and wanted features to work with what I had.

https://gist.github.com/WanderLanz/fae8b8e09746d611f384dc40d1bd41c9

Obviously haven't used github site alot :P as well.

@jarun
Copy link
Owner Author

jarun commented Jan 15, 2022

@WanderLanz thank you!

@luukvbaal @N-R-K can any of you adapt this to the plugin please?

@luukvbaal
Copy link
Collaborator

@WanderLanz were you dissatisfied with the option of running preview-tui through tmux in Windows Terminal/WSL? Or were you unaware that's an option?

Not sure it's worth adding this just to avoid the tmux dependency inside WSL.

@jarun
Copy link
Owner Author

jarun commented Jan 15, 2022

I myself don't use tmux on wsl. A semi-native solution helps.

@luukvbaal
Copy link
Collaborator

There's no need "use" tmux. Just installing it is enough to make preview-tui work.

Anyhow the gist doesn't work at all for me, it opens a 2 new wt instances and errors out.

@luukvbaal
Copy link
Collaborator

This option is required to make it work
image

@luukvbaal
Copy link
Collaborator

luukvbaal commented Jan 15, 2022

Oh I'm sorry, there's also a -w [0 for current window] flag for the "wt" command but I had forgot that option was a thing. I added it.

That's more like it.

Also, TMUX is installed by default on the Ubuntu WSL 20.04, do you need the TMUX server running for it to work? As I've said, I'm quite new to bash scripting, and was previously just using another machine for Linux development in centOS so I'm probably doing something wrong anyways.

Yes nnn needs to run inside tmux. I don't "use" tmux myself either but I use an alias for nnn like tmux new-session nnn -Pp "$@" to make sure preview-tui works.

A semi-native solution helps.

The tmux solution feels just as, if not more, native in my opinion.

@WanderLanz
Copy link

WanderLanz commented Jan 15, 2022

Oh, by default you need to follow https://github.com/jarun/nnn/wiki/Troubleshooting#tmux-configuration .
To make it work I just did: touch ~/.tmux.conf && echo "set -g default-terminal \"screen-256color\"" >> .tmux.conf
and then add the alias or command: tmux new-session nnn -a[preferred options] "$@"

You can still use the WT workaroud if you aren't going to install tmux if you want, just be aware that I'm running it first though Powershell (powershell.exe on WSL, as C:\Windows and System32 paths are added to your $PATH) then WT to make it work. Along with some few caveats:

WT itself doesn't add "native" support itself for these kind of things, and for some reason simply running C:\path\to\wt.exe doesn't work with straight from WSL so I can only seem to get it working through powershell.exe subterminal

Because it's running through powershell, any ";" in the command has been picked up first by powershell rather than bash, so '&&' has been used.

The PATH and PAGER vars cannot be passed along in any way that I've tried due to spaces, so I just remove PAGER and source $HOME/.profile instead, if you know a way to fix that please share.

Other than that, I really love the work you guys are doing here! The WSL community seems to be growing pretty rapidly these days with WSL2 and WSLg, I'd recommend Chris-Titus Windows Debloater script or another debloater to free up some room for things like ghci which has been pretty instensive for me on WSL lol. Although I'm a gamer first, so I might be more picky about these things : P .

@N-R-K
Copy link
Collaborator

N-R-K commented Jan 16, 2022

@WanderLanz if you're compiling from source then you can use the git status patch to get some git integration.
But I'm not sure why there would be any reason for an nnn plugin for doing things like git push/commit. Those are jobs better suited for your IDE or as a plugin for your text editor.

@WanderLanz
Copy link

@N-R-K Thanks! I was wondering if there were other cool features like that.
I was just mostly wondering what the gitroot plugin in the TODO was exactly, was he referring to something like this? :
https://gist.github.com/WanderLanz/216f53819bdab0b2c2eb4baa6dd3cba1

@N-R-K
Copy link
Collaborator

N-R-K commented Jan 16, 2022

@WanderLanz "Cooking" is a list of features/changes currently present on master branch but not released into a stable version yet. The gitroot plugin is already in master, you can find it here: https://github.com/jarun/nnn/blob/master/plugins/gitroot

@WanderLanz
Copy link

@N-R-K Thanks for letting me know, I'm not the brightest in the crowd : P .

@WanderLanz
Copy link

If you still wanted that preview-tui for use straight through Windows Terminal, although I now know you can just run it through tmux, I updated it to import the PATH and PAGER correctly.
The fix was simple, I'm just an idiot and tried all of the complicated things first.
https://gist.github.com/WanderLanz/fae8b8e09746d611f384dc40d1bd41c9

@luukvbaal
Copy link
Collaborator

luukvbaal commented Jan 17, 2022

Thanks, I made the PR. Although tmux feels more native to me as nnn gets distorted while openening the wt split and is slower too, going through powershell.

It does require less setup I guess, so there's that.

@WanderLanz
Copy link

WanderLanz commented Jan 17, 2022

I mean, there is a "wt.exe" wsl command to (/mnt/c/Users/<User>/AppData/Local/Microsoft/WindowsApps/wt.exe) through the wsl PATH preloading, but I can't figure out how to pass sub-commands or flags to it through bash. As I'm not a very well-versed person in the wt innerworkings or how wsl passes flags to .exe commands, does anyone have any idea if there is a way to make it work?

@WanderLanz
Copy link

As in https://docs.microsoft.com/en-us/windows/terminal/command-line-arguments?tabs=linux
, quote : "Execution aliases do not work in WSL distributions. If you want to use wt.exe from a WSL command line, you can spawn it from CMD directly by running cmd.exe. The /c option tells CMD to terminate after running."
so you can replace "powershell.exe [ command ]" with "cmd.exe /c [command] 2>>/dev/null" if you want a slight improvement since cmd is faster to load as a subshell I guess, although it might not be noticable. (2>/dev/null because of UNC path warning)

@WanderLanz
Copy link

let me whip it up real quick to see if it's noticeable at all...
I mean... kinda? I don't have the tools set up to time it exactly right now as this is my personal machine, but now it's ~600ms ish compared to the ~300ms it takes to do the tmux alias with the preview.
updated: https://gist.github.com/WanderLanz/fae8b8e09746d611f384dc40d1bd41c9

@WanderLanz
Copy link

sorry about the spam btw, do you want me to move under #1289 for now ? I'm well aware I'm being annoying and you guys are probably doing better things with your time than I am.

@luukvbaal
Copy link
Collaborator

You're fine. If you have further additions the discussion is best continued in #1289 yeah.

@jarun jarun mentioned this issue Jan 23, 2022
@jarun jarun closed this as completed Jan 23, 2022
Repository owner locked and limited conversation to collaborators Jan 23, 2022
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

8 participants