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

crash when pressing ctrl-o #852

Closed
lacygoill opened this issue Aug 16, 2023 · 3 comments
Closed

crash when pressing ctrl-o #852

lacygoill opened this issue Aug 16, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@lacygoill
Copy link

Describe the bug

Pressing ctrl-o makes navi crash.

To Reproduce

Steps to reproduce the behavior:

  1. run $ navi in a terminal emulator
  2. press ctrl-o

Expected behavior

No crash.

Screenshots

N/A

Versions:

  • OS: Ubuntu 20.04
  • Shell Version: GNU bash, version 5.0.17(1)-release (x86_64-pc-linux-gnu)
  • navi version: 2.22.1 ; latest commit

Additional context

Here is a full backtrace:

thread 'main' panicked at 'index out of bounds: the len is 0 but the index is 0', src/commands/core/actor.rs:209:36
stack backtrace:
   0:     0x556699d6f2ba - std::backtrace_rs::backtrace::libunwind::trace::h34aec3ef6cd8ad7e
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
   1:     0x556699d6f2ba - std::backtrace_rs::backtrace::trace_unsynchronized::h8035d38698d0f7a8
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x556699d6f2ba - std::sys_common::backtrace::_print_fmt::hff968f6695a1ba22
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/sys_common/backtrace.rs:65:5
   3:     0x556699d6f2ba - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h7eea0efe77acf1ec
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/sys_common/backtrace.rs:44:22
   4:     0x556699d95a4e - core::fmt::write::hc553f17407eb0b48
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/core/src/fmt/mod.rs:1208:17
   5:     0x556699d6bfe5 - std::io::Write::write_fmt::h62e5f01a089f48c0
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/io/mod.rs:1682:15
   6:     0x556699d6f085 - std::sys_common::backtrace::_print::h52d116aff3db4cd1
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/sys_common/backtrace.rs:47:5
   7:     0x556699d6f085 - std::sys_common::backtrace::print::h9e7d2f98fb7af075
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/sys_common/backtrace.rs:34:9
   8:     0x556699d708af - std::panicking::default_hook::{{closure}}::hf74999dab2d0a95c
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:267:22
   9:     0x556699d705eb - std::panicking::default_hook::hc11ca7d10c44c42f
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:286:9
  10:     0x556699d70fbc - std::panicking::rust_panic_with_hook::hdb4da1ae79c845a5
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:688:13
  11:     0x556699d70d59 - std::panicking::begin_panic_handler::{{closure}}::h02b5b35b126d5cf2
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:579:13
  12:     0x556699d6f76c - std::sys_common::backtrace::__rust_end_short_backtrace::h6c6853376cf416d1
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/sys_common/backtrace.rs:137:18
  13:     0x556699d70a62 - rust_begin_unwind
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:575:5
  14:     0x556699bac813 - core::panicking::panic_fmt::hfd9e949092070b66
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/core/src/panicking.rs:64:14
  15:     0x556699bac962 - core::panicking::panic_bounds_check::h369b89af3d2735a6
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/core/src/panicking.rs:147:5
  16:     0x556699bbf31a - navi::commands::core::actor::act::h1149b5fbcdf3a8ee
  17:     0x556699bd27b2 - navi::commands::core::init::hee5dcb091b63d876
  18:     0x556699bad0e7 - navi::main::h544210fcecb7b531
  19:     0x556699bad1d3 - std::sys_common::backtrace::__rust_begin_short_backtrace::h55c104d56be99089
  20:     0x556699bacf2d - std::rt::lang_start::{{closure}}::h932f711456737d41
  21:     0x556699d65aec - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::h9ab31282e87f134a
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/core/src/ops/function.rs:606:13
  22:     0x556699d65aec - std::panicking::try::do_call::h42ddf5b01d0b4bc7
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:483:40
  23:     0x556699d65aec - std::panicking::try::hfb70320d7386c61a
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:447:19
  24:     0x556699d65aec - std::panic::catch_unwind::h978c9edbad2bb4d4
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panic.rs:137:14
  25:     0x556699d65aec - std::rt::lang_start_internal::{{closure}}::h04ede5bd2f26b553
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/rt.rs:148:48
  26:     0x556699d65aec - std::panicking::try::do_call::ha6b9da35a0885c93
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:483:40
  27:     0x556699d65aec - std::panicking::try::h3325520cab3a642e
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panicking.rs:447:19
  28:     0x556699d65aec - std::panic::catch_unwind::h160beec6f047175b
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/panic.rs:137:14
  29:     0x556699d65aec - std::rt::lang_start_internal::h79190e3a877a769d
                               at /rustc/fc594f15669680fa70d255faec3ca3fb507c3405/library/std/src/rt.rs:148:20
  30:     0x556699bad1c5 - main
  31:     0x7f0a411d8083 - __libc_start_main
                               at /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
  32:     0x556699bace3e - _start
  33:                0x0 - <unknown>

It seems the issue ends at this block of code:

if key == "ctrl-o" {
edit::edit_file(Path::new(&files[file_index.expect("No files found")]))
.expect("Could not open file in external editor");
return Ok(());
}


I can reproduce with no navi config, no shell config, and no terminal config.

@lacygoill lacygoill added the bug Something isn't working label Aug 16, 2023
@lacygoill
Copy link
Author

Never mind. It's only for the few default snippets which are not read from any file. So, closing.


A more annoying issue is the fact that the editor specified in $VISUAL is started in a broken way, when we press ctrl-o after navi has been called from a command substitution:

$ bash --norc --noprofile
$ echo $(navi --path=$HOME/.local/share/navi/)
# press: C-o

I need --path=$HOME/.local/share/navi/ because that's where I put my navi files.

It's broken because the editor's UI is never drawn, and the latter seems stuck, which is very confusing when $VISUAL is nano(1) (less confusing with Vim, which at least prints some message "Vim: Warning: Output is not to a terminal"). To kill the editor, we need to press ctrl-c.

For now, as a workaround, I remove the ctrl-o key from the fzf option --expect in the navi YAML config file:

overrides: >-
  --no-expect
  --expect ctrl-y,enter

Note that we need to specify --no-expect before resetting the option, because:

If --expect option is specified multiple times, fzf will expect the union of
the keys. --no-expect will clear the list.

Source: fzf man page.

@lacygoill
Copy link
Author

For now, as a workaround, I remove the ctrl-o key from the fzf option --expect in the navi YAML config file

Better way: leave --expect alone, but reconnect the stdout of navi to the terminal:

$ echo $(navi --path=$HOME/.local/share/navi/ --print >$(tty))
                                                      ^-----^

This example might look contrived, but it's a simplification of this fish code:

bind \cg\cg shell-snippets

function shell-snippets
    set -f current_process $(commandline --current-process)
    set -f leading_whitespace $(string match --regex -- '^\s*' $current_process)
    set -f current_process $(string trim -- $current_process)

    if test -z "$current_process"
        # `--print` makes navi print the chosen snippet to stdout, instead of executing it.{{{
        #
        # This  lets   us  insert  the   snippet  on  the   command-line,  using
        # `commandline --insert` and a command substitution.
        #}}}
        # `>$(tty)`: for `C-o` to work as expected.{{{
        #
        # Without, the editor  specified in $VISUAL is started in  a broken way,
        # when we press `C-o`.  MRE:
        #
        #     $ bash --norc --noprofile
        #     $ echo $(navi --path=$HOME/.local/share/navi/)
        #     # press: C-o
        #
        # It's broken  because the editor's  UI is  never drawn, and  the latter
        # seems stuck, until we kill it by pressing `C-c`.
        #}}}
        commandline --insert -- $(navi --path=$HOME/.local/share/navi/ --print >$(tty))
        commandline --function repaint
        return
    end

    set -f snippet $(navi $options --query="$current_process")
    commandline --replace --current-process "$(string join -- '' $leading_whitespace $snippet)"
    commandline --function repaint
end

@lacygoill
Copy link
Author

Better way: leave --expect alone, but reconnect the stdout of navi to the terminal:

Actually, that's a bad idea. While it fixed C-o, it also breaks Enter.

So, I ended up removing ctrl-o from --expect, as explained before, and installing a M-e key binding which correctly handles the case where navi is started from a command substitution:

finder:
  command: fzf
  # https://github.com/denisidoro/navi/blob/master/docs/customization.md#overriding-fzf-options
  # `--no-expect`, `--expect ctrl-y,enter`{{{
  #
  # Problem: Pressing `C-o` might start `$VISUAL` in a broken way.
  # Solution: Make navi ignore `C-o`.
  #
  # By default, navi sets `--expect` to `ctrl-y,ctrl-o,enter`:
  #
  #     $ cd <navi source code>
  #     $ rg ctrl-o
  #     src/finder/mod.rs:106:51: command.args(["--expect", "ctrl-y,ctrl-o,enter"]);
  #                                                                 ^----^
  #
  # The purpose is to make navi starts `$VISUAL` to edit the file containing the
  # definition of the  currently selected snippet.  However, it  doesn't work as
  # expected when navi is started from a command substitution:
  #
  #     Vim: Warning: Output is not to a terminal
  #
  # The editor is stuck, and never draws its TUI (`C-c` needs to be pressed).
  #
  # To avoid  this, we remove `C-o`  from `--expect`.  But before  resetting the
  # option, we first need to clear it; hence `--no-expect`:
  #
  #    > If --expect option is specified multiple times, fzf will expect the union of
  #    > the keys. --no-expect will clear the list.
  #
  # MRE:
  #
  #     $ bash --norc --noprofile
  #     $ echo $(navi --path=$HOME/.local/share/navi/)
  #     # press: C-o  (make sure to select a snippet which comes a file; a few hard-coded and navi-related don't)
  #
  # ---
  #
  # Besides, we often press `C-o` by accident while navi is running; e.g.:
  #
  #     start navi
  #     v-----v
  #     C-g C-g C-o
  #         ^^^
  #         accident
  #
  # Instead of:
  #
  #     C-g C-o
  #     ^-----^
  #     run fish omni-TUI function
  #
  # When that happens, it's better for navi not to react.
  #}}}
  # `--bind='alt-e:execute(editor ...)'`{{{
  #
  # Problem: We can't edit the file containing the currently selected snippet.
  # Solution: Install a (new) key binding.
  #
  # Contrary to `C-o`, it correctly handles the case where navi has been started
  # from a command substitution, because we reconnect the editor's stdout to the
  # terminal:
  #
  #     --bind='alt-e:execute(editor ... >"$(tty)")'
  #                                      ^-------^
  #}}}
  overrides: >-
    --no-expect
    --expect ctrl-y,enter
    --bind='alt-e:execute(editor "$HOME/.local/share/navi/$(sed --quiet "s/\([-_a-zA-Z0-9]\+\).*/\1/p" <<<{})".cheat >"$(tty)")'
  # `>-` is a syntax which lets us split a long string on multiple lines.{{{
  #
  # It's called “Line Folding”: https://yaml.org/spec/1.2.2/#65-line-folding
  #}}}

For anyone reading this: if your cheat files are in the default location, you might need to replace $HOME/.local/share/navi/ with $(navi info cheats-path).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant