[go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

[RFC] refactor and improve shell options #1476

Open
3 tasks done
pirafrank opened this issue Aug 13, 2024 · 4 comments
Open
3 tasks done

[RFC] refactor and improve shell options #1476

pirafrank opened this issue Aug 13, 2024 · 4 comments
Labels
feature New feature request

Comments

@pirafrank
Copy link
pirafrank commented Aug 13, 2024 •
edited
Loading

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:

  • 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.
  • 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.
  • As of now the user can specify the --block option to wait for a command to finish its execution, in this case Yazi will hide into a secondary screen and display the program on the main screen until it exits. Yet this pose a challenge for scripts or programs that exit successfully but do not wait for a user input, in such a case the shell will flash the output and Yazi will return to the primary screen with the user having no time to read the output. For example, the current behavior it's ok for commands like git log but is not for ls. A today workaround may be appending ; read to the command to run so that the output stays in place until the user presses return.

Would you be willing to contribute this feature?

  • Yes, I'll give it a shot

Describe the solution you'd like

The following changes are proposed:

  • Rename --confirm to --no-prompt and --interactive to --prompt to better esplicitate which one asks the user to insert a command and which not, and to clarify that the two are mutually exclusive.
  • Add an option in yazi.toml to specify in which shell to run commands. Run in /bin/sh by default but let the user use an alternative non interactive shell of choice, one for example where $PATH has been customized.
  • 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.

Additional context

No response

Validations

  • I have searched the existing issues
  • The latest nightly build of Yazi doesn't already have this feature
@pirafrank pirafrank added the feature New feature request label Aug 13, 2024
@sxyazi
Copy link
Owner
sxyazi commented Aug 14, 2024

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?

@sxyazi sxyazi added the waiting on op Waiting for more information from the original poster label Aug 14, 2024
@pirafrank
Copy link
Author

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.

@github-actions github-actions bot removed the waiting on op Waiting for more information from the original poster label Aug 17, 2024
@sxyazi
Copy link
Owner
sxyazi commented Aug 17, 2024

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature New feature request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@pirafrank @sxyazi