rabite0 / hunter

The fastest file manager in the galaxy!
Do What The F*ck You Want To Public License
1.31k stars 64 forks source link

Issues with show_hidden and hidden folders #20

Closed meain closed 5 years ago

meain commented 5 years ago

There are a few issues with the show_hidden implementation from what I can tell.

Backtrace

thread 'main' panicked at 'index out of bounds: the len is 0 but the index is 0', /rustc/bc44841ad2a2ad5f6c5e67b9e35ed8e7e71d4dc7/src/libcore/slice/mod.rs:2539:10
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
   1: std::sys_common::backtrace::_print
   2: std::panicking::default_hook::{{closure}}
   3: std::panicking::default_hook
   4: hunter::die_gracefully::{{closure}}
   5: std::panicking::rust_panic_with_hook
   6: std::panicking::continue_panic_fmt
   7: rust_begin_unwind
   8: core::panicking::panic_fmt
   9: core::panicking::panic_bounds_check
  10: hunter::file_browser::FileBrowser::cache_files
  11: <hunter::file_browser::FileBrowser as hunter::widget::Widget>::refresh
  12: <hunter::file_browser::FileBrowser as hunter::widget::Widget>::on_key
  13: hunter::widget::Widget::handle_input
  14: hunter::main
  15: std::rt::lang_start::{{closure}}
  16: std::panicking::try::do_call
  17: __rust_maybe_catch_panic
  18: std::rt::lang_start_internal
  19: main

The error triggers if I go back one more step. I started hunter in the folder ~/.config/hunter

rabite0 commented 5 years ago

Oh crap, that's not good :(

You're right, the toggle_hidden mapping was on h, but since movement is matched before that, it never happens. I pushed a fix for that. It's now mapped to "H". I'm not sure if that's the best choice though.. What do you think?

I have to admit, I don't really use the vi-bindings so they're not tested as well as they probably should. I also have show_hidden enabled, so that never happened to me.

The show_hidden setting works for me, are you sure you spelled it right? It has to be "show_hidden=off", although spaces in between should be ok, if I remember right. Does "animation=off" do anything? Maybe it can't find the file, is there anything in the log? (Press "L").

EDIT: Oh right, "L" is also used for movement... Will fix that now.

And that crash.. ugh, that's really bad. I will fix that soon.

Thanks for reporting.

rabite0 commented 5 years ago

Ok, right, "L" should be open_bg, analogous to how "F" with the emacs-bindings. "g" opens the log now with vi-bindings.

Btw, open_bg actually doesn't work as it should right now. It only works on executables, but it should also work on other kind of files and open them in the background.

meain commented 5 years ago
rabite0 commented 5 years ago

Oh, I didn't realize you're on macOS!

Yeah, that might be the reason. I was under the impression that dirs-2 handles this gracefully, resorting to some kind of fallback directory if those environment variables aren't set.

The docs say it resolves to "$HOME/Library/Preferences", so hunter would, or should be looking for "$HOME/Library/Preferences/hunter/config"

I definitely want to get first-class macOS support, but it's hard without a mac ;). I'm probably going to install it in a VM to iron out the issues.

rabite0 commented 5 years ago

And yes, key bindings are a real sore point right now. Those need a complete overhaul. I want to make them configurable eventually, but I still need to think of a way to implement it (read: hack away at it until it works and looks nice ;) )

meain commented 5 years ago

Hmm, it is unusual to have configs at $HOME/Library/Preferences. I think most people would prefer it at ~/.config/hunter. Most of the other tools I have used seems to just use ~/.config/ itself.

meain commented 5 years ago

hack away at it until it works and looks nice

Pretty much every software project.

You could probably find some inspiration from how lf does keybindings for the vim part.

rabite0 commented 5 years ago

See: https://github.com/Larusso/dirs-rs

I don't know why it's like that, but apparently it's adhering to some Apple guidelines. I could easily hardcode it do ~/.config/hunter though. Just to clarify, that's not what most other (unixy/terminaly) tools do?

meain commented 5 years ago

No, most of them just use ~/.confg itself. At times I have seen them use home dir, but that is usually it.

This is all the stuff in my ~/.config dir.

I think $HOME/Library/Preferences thing more or less applies to GUI programs.

rabite0 commented 5 years ago

Ok, I pushed a fix, but since I can't test it, I'm not sure if it works.

meain commented 5 years ago

Which branch, I didn't see any updates on master. I could test it out for you.

EDIT: got it

rabite0 commented 5 years ago

On "mac-fix". master doesn't even work on mac unless i screwed up and merged those branches.

meain commented 5 years ago

No, I don't think that fixed it. I do not think that it is reading the config.

Let me see if I can send you a PR some time tomorrow on this.

Btw, it does run for me on master without any errors. ( not the config part )

meain commented 5 years ago

I even tried this

pub fn hunter_path() -> HResult<PathBuf> {
    let mut hunter_path = if cfg!(mac_os) {
        PathBuf::from("~/.config/").canonicalize()?
    } else {
        dirs_2::config_dir()?
    };
    hunter_path.push("hunter/");
    Ok(PathBuf::from("~/.config/hunter/").canonicalize()?)
}

But for some reason it still fails to work. I am confused as to what is happening.

rabite0 commented 5 years ago

Interesting. Some people had issues related to the thread pool, which made hunter crash. I assumed it was due to some differences between Linux/macOS, but maybe it's actually something else..

meain commented 5 years ago

Well, I did not see anything in related to the directory but I found this reset: standard error: Inappropriate ioctl for device.

rabite0 commented 5 years ago

BTW, I did it wrong. It's supposed to be "cfg!(macos)". I already pushed a new version. It also prints the directory to stderr, so start it with hunter 2> dbg.txt or something like that. It will then print the directory it's looking for.

If it doesn't print anything it's not working...

Weird that it doesn't work even if you hardcode that path. I can only image that there's something fishy in config.rs, when it checks for the file with

if !config_path.exists() {
return Ok(Config::new()); }

But that shouldn't be a problem if the file exists... Other that that it just uses read_to_string.

So when Config::load runs it just gets the path from paths::config_path() and then reads the contents with read_to_string(). Nothing else going on there.

IDK, I'm going to check this out tomorrow when I have a macOS VM.

Anyway, thanks for working this through with me.

meain commented 5 years ago

Glad I could help you out.

One more thing. When I was trying out something earlier, I think I found that the load function (in config.rs ) was not getting called. But then I saw it was working for you, I dropped that thought.

But I think it might have some errors somewhere and so it might not be calling load, only on macOS.

You might wanna check in that direction.

rabite0 commented 5 years ago

Hmm.. well the config is loaded asynchronously to speed up the starting process. This might be related to the threading issues someone had on macros.

I'll probably start by making that synchronous. Then it's going to be obvious why loading fais.

Thanks!

rabite0 commented 5 years ago

Unfortunately I was unable to install macOS on VirtualBox.. I'm going to try with a preinstalled image, but if that doesn't work either, IDK. Since the only problem seems to be that it doesn't load the configuration it's not too pressing of an issue anyway. Just wondering: It doesn't load the config at all, right? Not even under $HOME/Library/Preferences? Does it save/load the tags and bookmarks?

Anyway, onto fixing the crash :).

rabite0 commented 5 years ago

Crash should be fixed now. This could always happen when the directory became empty due to only containing hidden files.

meain commented 5 years ago

Yup, it does read the config when put inside $HOME/Library/Preferences. Bookmarks work too.

rabite0 commented 5 years ago

Great. Since the crash and keybind issues are fixed I'm closing this issue. I'll see what I can do about the config location when I can get a macOS VM running, but since it's working and at least technically correct I don't really consider it a bug. If you disagree and are willing to test a few iterations in case I can't get a VM running feel free to open a new issue for that.

Anyway, thanks for reporting and working with me on this.

EDIT: You might also wan to try setting $XDG_CONFIG_HOME, maybe it actually uses that, even on macOS.