search

kyoheiu / felix

tui file manager with vim-like key mapping
https://kyoheiu.dev/felix/
MIT License
729 stars 26 forks source link

Improvements to Linux support #40

Closed mmstick closed 2 years ago

mmstick commented 2 years ago
mmstick commented 2 years ago

Thoughts on getting the default editor from ~/.gitconfig if not set?

kud1ing commented 2 years ago

Or maybe just use https://crates.io/crates/open?

mmstick commented 2 years ago

That could certainly be used for non-Linux systems.

kud1ing commented 2 years ago

As i understand it it uses "xdg-open" among others: https://github.com/Byron/open-rs/blob/9441d6c87419f94e0ebaffdf69f9b01f0aec4ddb/src/lib.rs#L341

mmstick commented 2 years ago

While true, you still need a different path for Linux to get the mime type of a file with the file command, and xdg-open is the standard on Linux.

kud1ing commented 2 years ago

Fair enough.

kyoheiu commented 2 years ago

OK, kind of complicated issue so I'd like to tidy up: Basically you try to set 'life-savers' for application to open files, right? For now, that is the default command written explicitly in config.toml. As I understand, This PR sets several alternatives in the following order:

I agree that it's not so smart to open any file whose ext is not defined in config.toml by default command.
But the thing is, while this PR carefully catch many possibilities, it still can cause a problem: For example when EDITOR and .gitconfig's editor are not defined, and xdg-open is not well-maintained (if you use i3 and install minimal package in minimal distro like Arch this happens... Actually it's me).
In this case, which I think is not so rare, felix should call default command as the last hope, so default command (a-1) must be set. Which means a-2, a-3 and a-4 are not called in this app. And I'm not convinced that use of xdg-open when undefined in config is better than open it by default app in config, because of the reason above. It can be unexpected behavior. And, I think it's good to configure how to open files in felix independently of the default behavior of OS (like xdg-open), i.e. in terminal you want open some image files by feh, but usually (via double-click on desktop icon or whatever) want to use other app.

Also I don't think it's appropriate to include distro-specific ifs like a-3.

In the end, IMO it is clearer and maybe performance-better to set default app in config.

kyoheiu commented 2 years ago

Note: If you split PR and send the refactoring part independently I'll merge it.

mmstick commented 2 years ago

The only reasonable way to determine which application to use with a file is by the content type, rather than the extension. Extensions are often not reliable as a source of information about the contents of files, especially if the extension is not defined. They're also too specific in the generalized case that wants to assign an application to open all files of a given category.

As demonstrated, we can specify a default case for text files which works universally across text files of any extension, even without extensions. I don't have to define a default command for rs, toml, c, cc, cpp, c++, conf, etc. I can also correctly open Makefile and justfile files despite no extensions on these files. The same could be done for audio and video, too.

The default command should also not be a text editor. It's never going to be the case that I want to open a binary or archive with a text editor. It is better to allow the OS to choose the default application for opening a file, given that it is already keeping track of which applications are associated with what content types and extensions. xdg-open is just that.

The implementation here to find the user's preferred editor is not a performance concern at all. The default editor is only guessed once at startup and only if the editor has not already been defined in the configuration file. We're speaking in terms of milliseconds.

People generally prefer to have consistent defaults which are defined by existing standards where possible, instead of having to keep settings in sync across multiple applications. A best effort lookup is an ideal default behavior. We should be able to agree that the ideal default behavior shouldn't require any configuration by the user at all. A user should expect common use cases to work well without setup.

The EDITOR environment variable is already a historical precedent for declaring the user's preferred terminal editor across many applications. /usr/bin/editor is also an establish standard that isn't exactly tied to any specific distribution. Everyone here is already using git, so that's yet another precedence.

I'd also add that all of these are checked for their existence, so if none of them are set then no default is set. If a default is set, none of them are checked.

Finally, this change only uses xdg-open when no preferred command was defined in the configuration file.

kyoheiu commented 2 years ago

Makes sense, thank you for the description! Learned a lot from your PR. I'll merge. After updating README, I'm going to release next version.

kyoheiu commented 2 years ago

Oops, macos install test failed...

kyoheiu commented 2 years ago

mismatched return type. Trying to fix it on macOS

kyoheiu commented 2 years ago

@mmstick It can be fixed by using "usr/bin/open" instead of open::that (this is the implementation in open crate). But the situation is acutually not as easy as it seemed before.

Just playing around with your PR, I feel a kind of irritation that, instead of opening everything in terminal (Though yes it's not smart to open binary by nvim haha), which is the previous behavior, felix opens this file in VS Code or that file in GitKraken, changing focus of window... That's not what I want. Then should I fix xdg-mime in Linux or usr/bin/open in macOS? Maybe. But I don't want users to do that just to use this app.

"Stay inside terminal" is certainly a core value of this app, I just realized.

Edit: Ok, I'm going around in circles. I'll give this some thought.