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

Closed
44 of 50 tasks
jarun opened this issue May 19, 2021 · 61 comments
Closed
44 of 50 tasks

ToDo list #1022

jarun opened this issue May 19, 2021 · 61 comments
Labels

Comments

@jarun
Copy link
Owner

jarun commented May 19, 2021

Rolled from #935.

Cooking

Up for grabs

For anything else please discuss in this thread.

Contribution guideline.

@jarun jarun added the planning label May 19, 2021
@mizlan
Copy link
Collaborator

mizlan commented May 20, 2021

predefined filters like bookmarks

I've read a past thread but am still confused as to what this means.

@jarun
Copy link
Owner Author

jarun commented May 20, 2021

Updated now. Basically filter keys to apply a filter at the start of the program. However, I don't see much benefit.
See if the multi-threaded du interests you.

However, I would like the nnn.vim plugin enhancement to support sessions first.

@jarun
Copy link
Owner Author

jarun commented May 22, 2021

@Kabouik can you please audit nuke in the lines of #935 (comment)?

It would be good to have that done before the next release.

@jarun
Copy link
Owner Author

jarun commented May 22, 2021

Leave it. I took a look, it's more or less clean.

Repository owner deleted a comment from jayp0501 May 25, 2021
@jarun
Copy link
Owner Author

jarun commented May 26, 2021

@CodeforEvolution does Haiku have libpthread? Planning to implement threaded disk usage calculator.

@CodeforEvolution
Copy link
Collaborator

@jarun Hello, and I apologize for the recent silence. While Haiku does have a pthread implementation, you'll need to make sure that you link against a library called "libroot". If the build system requires the availability of libpthread, Haiku at least has a "fake" libpthread library file to satisfy that.

@jarun
Copy link
Owner Author

jarun commented May 26, 2021

Can you please do the necessary modification to Makefile for this change to work: 284a3c4#diff-94e6e1efb3367ceccb57baea06ea5c8ab89faaa9d720ee5e599039710dfcae49R127

@jarun
Copy link
Owner Author

jarun commented May 26, 2021

Note that we will also need fts.

@CodeforEvolution
Copy link
Collaborator

Sure thing! And yes, it looks like we have fts. :)

@jarun
Copy link
Owner Author

jarun commented May 26, 2021

Cool! This is very important for the next release.

@CodeforEvolution
Copy link
Collaborator

@jarun Well, better yet, I can confirm that no additional changes are required! I was able to successfully build the latest commit of nnn and run the test you posted in the commit message.

@jarun
Copy link
Owner Author

jarun commented May 26, 2021

Just awesome!!! 👍

@jarun
Copy link
Owner Author

jarun commented May 28, 2021

@luukvbaal are we generating thumbnails for preview-tui* in the same dir within .thumbs?

@luukvbaal
Copy link
Collaborator

luukvbaal commented May 28, 2021

Not sure what .thumbs is but no. Thumbnails go to $TMPDIR/nnn/previews by default and a suggestion to set $NNN_PREVIEWDIR to $XDG_CACHE_HOME/nnn/previews is present in the docs.

@signed-log
Copy link
Collaborator

Hello,
I can make a multi-arch docker container to build the arm images if needed

@jarun
Copy link
Owner Author

jarun commented May 28, 2021

@luukvbaal

Not sure what .thumbs is but no.

Thanks for confirming! Yes, I confirmed it's not from our previewers.

@Stig124

I can make a multi-arch docker container to build the arm images if needed

@KlzXS @luukvbaal @0xACE what do you guys say?

@0xACE
Copy link
Collaborator

0xACE commented May 28, 2021

@Stig124

I can make a multi-arch docker container to build the arm images if needed

@KlzXS @luukvbaal @0xACE what do you guys say?

Not sure what to say about it. Do as you please...

@KlzXS
Copy link
Collaborator

KlzXS commented May 28, 2021

If it can be nicely integrated into your release workflow and produces a useable build, sure.

Do we have someone with these architectures? All I have that is not x86 is a pi zero.

@jarun
Copy link
Owner Author

jarun commented May 28, 2021

I have a Pi 4 but can't guarantee I'll be able to test on every release.
However, I think it's a good idea in general if we can auto-generate these. We do have a line item.

I think @Stig124 you can go ahead. Please do the CI integration as well.

@signed-log
Copy link
Collaborator

signed-log commented May 28, 2021

Sure, will do

@KlzXS
Copy link
Collaborator

KlzXS commented May 28, 2021

can't guarantee I'll be able to test on every release.

Users will test it out in time. Let's just make sure the first binary we release doesn't have any major issues.

@jarun
Copy link
Owner Author

jarun commented May 28, 2021

The next release is going to be a massive one with the features that are in.

@0xACE
Copy link
Collaborator

0xACE commented May 28, 2021

One of my ARM Cortex-A73 is running on my personal branch based on 950a8f6 and it has been running fine for 3 years, nor have i experienced any issues exclusive to my arm machines...

@signed-log
Copy link
Collaborator

signed-log commented May 28, 2021

Had time to tinker with the image, it builds just fine, but I'm afraid that it won't go fine in CircleCI, I know so little about how CircleCI works that It will take me a while to figure out how to build it multiarch on CircleCI.

More precisely, run it, as I build the images, the executable is blocked into the image and you need to run the image before having a single possibility of getting the executable out of the image. The only way I can think of is to find a way to run QEMU, but without root access on the host system it is impossible, I'll think about that issue tonight and will see

@signed-log
Copy link
Collaborator

signed-log commented May 29, 2021

Hello,

I thank about that, it will not be possible to build executables on ARM using CircleCI.
But I spoke of that w/my boss, and he can loan me for free a ARM64-based machine that I can configure to respond to webhook so that It will be building the executables for nnn in both ARM64 and ARM32 (because I would have QEMU up and running), what do you think @jarun?

@KlzXS
Copy link
Collaborator

KlzXS commented May 29, 2021

The main issue with that becomes whether that machine will be available in the future or not. It would really suck if suddenly the arm builds stopped being built.

@jarun
Copy link
Owner Author

jarun commented May 29, 2021

Can we use some service other than CircleCi? How about GitHub's own service?

@signed-log
Copy link
Collaborator

Can we use some service other than CircleCi? How about GitHub's own service?

Travis seems to build ARM executable just fine, but I don't see any that would build armhf executables

@luukvbaal
Copy link
Collaborator

luukvbaal commented May 30, 2021

If I replace type bat with ! type bat, then it works correctly.

This makes 0 sense to me. If bat in your PATH, type bat should return 0. If bat is not in your PATH ,! type bat would return zero but then it should fail executing the bat command that follows.

NNN_PREVIEWDIR being empty meaning the environment variable as seen by preview-tui, not an empty directory. That must be what's going on if it's creating the previews in PWD i.e. mkdir -p "$NNN_PREVIEWDIR/${3%/*}". No idea why that would be the case if you're on master and exporting NNN_PREVIEWDIR in your .bashrc, and bash is your default shell.

@Kabouik
Copy link
Collaborator

Kabouik commented May 30, 2021

type bat returns bat is /home/user/.cargo/bin/bat on my end, not 0. ! type bat too, but that inverts the condition I guess.

@luukvbaal
Copy link
Collaborator

luukvbaal commented May 30, 2021

I'm talking about the exit status, we're redirecting the actual return value to /dev/null.

@luukvbaal
Copy link
Collaborator

It must not return /home/user/.cargo/bin/bat in your "nnn plugin environment" or else you wouldn't have to invert the condition.

@Kabouik
Copy link
Collaborator

Kabouik commented May 30, 2021

I think the issue with NNN_PREVIEWDIR is simply that it doesn't accept the ~/ abbreviation. I got confused by my other nnn environment variables where ~/ works (like plugins, bookmarks).

One issue though is that if you preview images in the preview directory, it will create another subdirectory for that, but then that means it can loop on itself. Wouldn't it be best to prevent preview-tui* from creating previews cache for images viewed inside a sub-folder of $NNN_PREVIEWDIR? I know no one should do that, but I feel it would be safer to keep the script from caching previews for existing cache previews themselves. Excluding the folder would perhaps be enough?

@luukvbaal
Copy link
Collaborator

I've come across that situation while developing the plugin yeah but indeed no one should open preview-tui in the cache directory. Not sure if it's worth preventing that, perhaps it is.

@luukvbaal
Copy link
Collaborator

Indeed always prefer using $HOME over ~ in shell variables. ~ is not expanded when quoted.

@Kabouik
Copy link
Collaborator

Kabouik commented May 30, 2021

I feel it could be useful even if it's weird to preview that folder on purpose. If the preview pane is already opened, then anything hovered will be previewed, meaning one would have to know about that issue in advance before going into the cache folder with nnn to remember that the preview pane should be closed beforehand. Going into that folder is not a rare use case I think, I would expect people to keep an eye on it to monitor its size and delete unnecessary cache.

@Kabouik
Copy link
Collaborator

Kabouik commented May 30, 2021

It must not return /home/user/.cargo/bin/bat in your "nnn plugin environment" or else you wouldn't have to invert the condition.

Right, I was just testing the command directly in terminal here, I misunderstood from your message that 0 was the expected result. The above reports in my previous messages are all from executing the full script through nnn though, and adding ! does fix the use of bat on that machine. Can't tell yet if I observe the same on other machines, but I assume I won't since you have no such issue on yours.

@Kabouik
Copy link
Collaborator

Kabouik commented May 30, 2021

Indeed always prefer using $HOME over ~ in shell variables. ~ is not expanded when quoted.

Thanks! Yeah I already use $HOME everywhere else in my .bashrc but got spoiled for nnn variables because nnn tends to be flexible: https://github.com/jarun/nnn/wiki/Basic-use-cases#add-bookmarks

Maybe we should edit the wiki to discourage this use, or is there a way to make NNN_PREVIEWDIR equally tolerant to ~/?

@luukvbaal
Copy link
Collaborator

luukvbaal commented May 30, 2021

Yeah export NNN_BMS="d:$HOME/Documents;u:$HOME/Cam Uploads;D:$HOME/Downloads/" is more appropriate as an environment variable in my opinion. Not sure how @jarun feels about it.

The reason it works for NNN_BMS is because ~ is expanded by nnn itself. There are ways to expand quoted ~ in shell scripts but I don't feel that's appropriate for nnn plugins to do.

@jarun
Copy link
Owner Author

jarun commented May 30, 2021

Can you summarize the NNN_BMS thing for me?

@luukvbaal
Copy link
Collaborator

luukvbaal commented May 30, 2021

@Kabouik ran into an issue with preview-tui because they used "~" in NNN_PREVIEWDIR like suggested in the wiki for NNN_BMS. This does not work for shell scripts without manually expanding "~", using "$HOME" instead is more appropriate.

@jarun
Copy link
Owner Author

jarun commented May 30, 2021

Why can't the script expand ~?

@luukvbaal
Copy link
Collaborator

Shells don't expand quoted ~'s.

@luukvbaal
Copy link
Collaborator

@luukvbaal
Copy link
Collaborator

Nothing needs to change, it's just a matter of whether you want to change the wiki to reflect using $HOME over ~ to possibly avoid users running into the same problem.

@jarun
Copy link
Owner Author

jarun commented May 30, 2021

OK. I was expecting at least double quotes to work. Why can't you specify the unquoted string in a controlled script?

If space is a problem, doesn't the following work?

~/"Arun Pictures"/something

Anyway, the NNN_BMS thing stays.

@jarun
Copy link
Owner Author

jarun commented May 30, 2021

it's just a matter of whether you want to change the wiki to reflect using $HOME over ~ to possibly avoid users running into the same problem.

@Kabouik mentioned NNN_BMS which is parsed by nnn. It resolves ~ but doesn't understand $HOME. hence the explicit ~ usage in the example. I think @Kabouik is mixing up stuff. The shell limitation is not nnn issue.

@luukvbaal
Copy link
Collaborator

$HOME doesn't need to be parsed by nnn as it is expanded by the shell when assigned to a variable.

@luukvbaal
Copy link
Collaborator

It's just generally more advised to use $HOME in shell variables as it works in all cases as opposed to ~.

@luukvbaal
Copy link
Collaborator

@Kabouik is indeed at fault for using ~ in NNN_PREVIEWDIR although I can understand why he would expect it to work when it does work for NNN_BMS since it is parsed by nnn.

Again, changing the wiki to use $HOME instead of ~ could prevent other users making the same mistake.

@jarun
Copy link
Owner Author

jarun commented May 30, 2021

OK. Please update. You may have t do it in the man page as well (for NNN_BMS).

@Kabouik
Copy link
Collaborator

Kabouik commented May 30, 2021

I think @Kabouik is mixing up stuff. The shell limitation is not nnn issue.

No that's my understanding too now, but yeah initially I was thinking I could use the same syntax in environment variables parsed directly by nnn or by its plugins, since I didn't know about the shell limitation and all those environment variables look similar (NNN_*), which led me to misuse NNN_PREVIEWDIR.

I just find it misleading that the wiki shows ~ abbreviation working for variables parsed directly by nnn whereas it won't for those parsed by scripts, but I don't think every nnn user would be as ignorant as me when it comes to shells. :> I have no strong opinion about whether the wiki should show $HOME to avoid the ambiguity, or if scripts could be made to work with ~ in environment variables, I just feel harmonizing would save mistakes for some users.

@jarun
Copy link
Owner Author

jarun commented May 30, 2021

That's fine. It's good to be uniform.

This was referenced May 30, 2021
@jarun jarun mentioned this issue Jun 2, 2021
10 tasks
@jarun jarun closed this as completed Jun 2, 2021
Repository owner locked as resolved and limited conversation to collaborators Jun 2, 2021
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