sxyazi / yazi

💥 Blazing fast terminal file manager written in Rust, based on async I/O.
https://yazi-rs.github.io
MIT License
14.45k stars 330 forks source link

[RFC] refactor and improve shell options #1476

Open pirafrank opened 1 month ago

pirafrank commented 1 month ago

yazi --debug output

Yazi
    Version: 0.3.0 (7a380c2 2024-08-01)
    Debug  : false
    OS     : linux-x86_64 (unix)

Ya
    Version: 0.3.0

Emulator
    Emulator.via_env: ("xterm-256color", "")
    Emulator.via_csi: Ok(Unknown([]))
    Emulator.detect : Unknown([])

Adapter
    Adapter.matches: Chafa

Desktop
    XDG_SESSION_TYPE: Some("tty")
    WAYLAND_DISPLAY : None
    DISPLAY         : None

SSH
    shared.in_ssh_connection: true

WSL
    /proc/sys/fs/binfmt_misc/WSLInterop: false

Variables
    SHELL              : Some("/usr/bin/zsh")
    EDITOR             : Some("vim")
    YAZI_FILE_ONE      : None
    YAZI_CONFIG_HOME   : None
    ZELLIJ_SESSION_NAME: None

Text Opener
    default: Some(Opener { run: "${EDITOR:=vi} \"$@\"", block: true, orphan: false, desc: "$EDITOR", for_: None, spread: true })
    block  : Some(Opener { run: "${EDITOR:=vi} \"$@\"", block: true, orphan: false, desc: "$EDITOR", for_: None, spread: true })

tmux
    TMUX   : false
    Version: tmux 3.3a

Dependencies
    file             : 5.44
    ueberzugpp       : No such file or directory (os error 2)
    ffmpegthumbnailer: No such file or directory (os error 2)
    magick           : No such file or directory (os error 2)
    fzf              : 0.54.3
    fd               : No such file or directory (os error 2)
    rg               : No such file or directory (os error 2)
    chafa            : No such file or directory (os error 2)
    zoxide           : 0.9.4
    7z               : 16.02
    7zz              : No such file or directory (os error 2)
    jq               : 1.6

--------------------------------------------------
When reporting a bug, please also upload the `yazi.log` log file - only upload the most recent content by time.
You can find it in the "/home/francesco/.local/state/yazi" directory.

Please describe the problem you're trying to solve

Right now existing 'shell' configuration options in keymap.toml offer a way to run commands based on keymaps, which may include an explicit pre-defined command or one a user is prompted to type. Yet the following apply:

Would you be willing to contribute this feature?

Describe the solution you'd like

The following changes are proposed:

Additional context

No response

Validations

sxyazi commented 1 month ago

Existing options may look confusing to some users, for example --confirm sounds as the user do need to confirm, while it means it won't be necessary. Also since version 0.3.0 the option is mutually exclusive with --interactive, which do ask the user to insert the command to be run.

Introducing --interactive in version 0.3 is to safely remove --confirm in the future — it's a design flaw, running a shell shouldn't be that painful.

Another limit is running commands in a shell different than /bin/sh. Power users often customize their shell environment, including using different shells (such as bash, zsh, or fish) and customizing $PATH for them. For Yazi to run shell commands in /bin/sh may turn into errors as $PATH is almost never customized there.

Sorry, no, I explained the reasoning in this PR, but I welcome any patches as long as they work with most mainstream shells (bash, zsh, fish, nushell, cmd, powershell).

Keep the --block option as-is (good for programs that behaves like git log), add another option called --blockAndWait that does what the appended ; read would do, thus keeping the output of the command run in the primary screen and Yazi in the second, waiting for the user to press return.

It's doable, but I'm a bit skeptical of its value. It seems like --blockAndWait is longer than just ; read, and what advantages does it have over ; read?

pirafrank commented 3 weeks ago

Hi, sorry for late reply, back after a couple of days off.

Introducing --interactive in version 0.3 is to safely remove --confirm in the future — it's a design flaw, running a shell shouldn't be that painful.

I didn't know about the future plans of refactoring shell options, but I totally agree.

Sorry, no, I explained the reasoning in this PR, but I welcome any patches as long as they work with most mainstream shells (bash, zsh, fish, nushell, cmd, powershell).

Sorry, maybe I am missing the point here but I think it's totally doable.

Vim, Neovim, and other programs gives the chance to run commands in the default shell of the user, the one from which themselves have been run. It's up to the user to write commands that work for the shell he/she configured in Yazi config. If I write a Bash command on PowerShell I do not expect it to work. Yazi should just handle the error internally not to crash and propagate it to the user for him/her to understand.

Defaulting to /bin/sh could be a safety net here, as well, albeit only on *nix platforms.

It's doable, but I'm a bit skeptical of its value. It seems like --blockAndWait is longer than just ; read, and what advantages does it have over ; read?

Just my opinion. Yazi as a tool has a lot of potential now and in the future to be customised to the needs of power users alike who surely already know about subprocesses, exit codes, and more happening under the hood when a shell command is invoked. Yet it's simple and nice looking to attract more beginner-to-medium experienced users who may not understand the 'flashing' issue it's not a real one and think it's about Yazi instead. The option would ease the onboarding and UX of those users.

sxyazi commented 3 weeks ago

Sorry, maybe I am missing the point here but I think it's totally doable

Yes, this is indeed doable, we just need to find solutions tailored for each shell, such as addressing the issue with the fish shell where $argv includes $0$0 is a variable Yazi uses to store the currently hovered file, not the list of files selected by the user. We might also need to write a separate parser for PowerShell, similar to what we've done for cmd.

What I want to say is that while this would require significant effort, the benefits might not be very substantial because Yazi is more focused on implementing complex features through plugins rather than shells and I've explained why in that PR.

I can't invest too much time into this because there are other more important things to address, but I won't reject any PRs as long as they resolve these differences between shells.

Yet it's simple and nice looking to attract more beginner-to-medium experienced users who may not understand the 'flashing' issue it's not a real one and think it's about Yazi instead

Fair enough, I'll try to implement it in the future.

pirafrank commented 2 days ago

Adding up to this issue after discovering this nice plugin, on which I just made a PR about the read support above. It may be a viable alternative for now I guess.