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 on completion for an alias #12090

Open
KAAtheWiseGit opened this issue Mar 6, 2024 · 16 comments · May be fixed by #12596
Open

Crash on completion for an alias #12090

KAAtheWiseGit opened this issue Mar 6, 2024 · 16 comments · May be fixed by #12596
Labels
alias Issues around support for command aliases, touches parser and name resolution 🐛 bug Something isn't working completions Issues related to tab completion line editor Issues related to reedline panic

Comments

@KAAtheWiseGit
Copy link
Contributor

Describe the bug

I have an alias to cal. When I try to tab-complete outside of half-complete flags, it crashes.

How to reproduce

  1. Have an alias alias calendar = cal --week-start monday.
  2. Type calendar and hit tab.

Expected behavior

Normal completion.

Screenshots

No response

Configuration

key value
version 0.91.0
branch main
commit_hash 3016d7a
build_os linux-x86_64
build_target x86_64-alpine-linux-musl
rust_version rustc 1.76.0 (07dca489a 2024-02-04) (Alpine Linux 1.76.0-r1)
cargo_version cargo 1.76.0
build_time 2024-03-06 15:32:43 +03:00
build_rust_channel debug
allocator mimalloc
features default, sqlite, trash, which, zip
installed_plugins

Additional context

Backtrace

~/fork/nushell> calendar Error:   × Main thread panicked.
  ├─▶ at crates/nu-cli/src/completions/command_completions.rs:99:43
  ╰─▶ attempt to subtract with overflow
         0: 0x56021407ba00 - rust_begin_unwind
         1: 0x560211417275 - core::panicking::panic_fmt::h914dbe9d19fb3535
         2: 0x560211417333 - core::panicking::panic::ha1e97a8c9b8dbf22
         3: 0x5602128b9156 -
      nu_cli::completions::command_completions::CommandCompletion::complete_commands
      ::{{closure}}::h90df70ef45a6d236
                      at /home/kaathewise/fork/nushell/crates/nu-cli/src/
      completions/command_completions.rs:99
         4: 0x5602128adf3e - core::iter::adapters::map::map_fold::
      {{closure}}::h5a4864bec900b524
                      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/
      core/src/iter/adapters/map.rs:85
         5: 0x5602128a4e92 -
      core::iter::traits::iterator::Iterator::fold::h1d8fcd0ac6e85bb6
                      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/
      core/src/iter/traits/iterator.rs:2640
         6: 0x5602128a8bdf - <core::iter::adapters::map::Map<I,F> as
      core::iter::traits::iterator::Iterator>::fold::h253d84410d4f3d4d
                      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/
      core/src/iter/adapters/map.rs:125
         7: 0x5602128ac9d8 -
      core::iter::traits::iterator::Iterator::for_each::h24bdee46414a669f
                      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/
      core/src/iter/traits/iterator.rs:858
         8: 0x5602128fbcf5 - alloc::vec::Vec<T,A>::extend_trusted::he7e74c7be83d089a
                      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/
      alloc/src/vec/mod.rs:2923
         9: 0x56021290461b - <alloc::vec::Vec<T,A> as
      alloc::vec::spec_extend::SpecExtend<T,I>>::spec_extend::hafe212401d087fbf
                      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/
      alloc/src/vec/spec_extend.rs:26
        10: 0x5602128f921a - <alloc::vec::Vec<T> as
      alloc::vec::spec_from_iter_nested::SpecFromIterNested<T,I>>::from_iter::hce085
      215ac09c056
                      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/
      alloc/src/vec/spec_from_iter_nested.rs:62
        11: 0x560212900a6e - alloc::vec::in_place_collect::<impl
      alloc::vec::spec_from_iter::SpecFromIter<T,I> for
      alloc::vec::Vec<T>>::from_iter::h4b8c5e6f0cb0445b
                      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/
      alloc/src/vec/in_place_collect.rs:237
        12: 0x560212903fa7 - <alloc::vec::Vec<T> as
      core::iter::traits::collect::FromIterator<T>>::from_iter::h42637383c7969e1e
                      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/
      alloc/src/vec/mod.rs:2791
        13: 0x5602128ac70e -
      core::iter::traits::iterator::Iterator::collect::h63471a478d06ba86
                      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/
      core/src/iter/traits/iterator.rs:2054
        14: 0x5602128e0a19 -
      nu_cli::completions::command_completions::CommandCompletion::complete_commands
      ::h42df181777a3d104
                      at /home/kaathewise/fork/nushell/crates/nu-cli/src/
      completions/command_completions.rs:91
        15: 0x5602128e1449 -
      <nu_cli::completions::command_completions::CommandCompletion as
      nu_cli::completions::base::Completer>::fetch::he5f5646cdcd08316
                      at /home/kaathewise/fork/nushell/crates/nu-cli/src/
      completions/command_completions.rs:174
        16: 0x5602128bf554 -
      nu_cli::completions::completer::NuCompleter::process_completion::h1d7275f0be0e
      89aa
                      at /home/kaathewise/fork/nushell/crates/nu-cli/src/
      completions/completer.rs:51
        17: 0x5602128d8839 -
      nu_cli::completions::completer::NuCompleter::completion_helper::hd263b8895ae68
      5a4
                      at /home/kaathewise/fork/nushell/crates/nu-cli/src/
      completions/completer.rs:359
        18: 0x5602128d92c4 - <nu_cli::completions::completer::NuCompleter as
      reedline::completion::base::Completer>::complete::hc0d89a76e5caef80
                      at /home/kaathewise/fork/nushell/crates/nu-cli/src/
      completions/completer.rs:416
        19: 0x5602128bf195 -
      reedline::completion::base::Completer::complete_with_base_ranges::h88d6d236ed4
      9ce28
                      at /home/kaathewise/.local/state/cargo/registry/src/
      index.crates.io-6f17d22bba15001f/reedline-0.30.0/src/completion/base.rs:45
        20: 0x560212aeddcf - <reedline::menu::columnar_menu::ColumnarMenu as
      reedline::menu::Menu>::update_values::ha0ae642bc0fca99f
                      at /home/kaathewise/.local/state/cargo/registry/src/
      index.crates.io-6f17d22bba15001f/reedline-0.30.0/src/menu/columnar_menu.rs:497
        21: 0x560212adc75d -
      reedline::menu::ReedlineMenu::update_values::h716045a7d0d19857
                      at /home/kaathewise/.local/state/cargo/registry/src/
      index.crates.io-6f17d22bba15001f/reedline-0.30.0/src/menu/mod.rs:334
        22: 0x560212a31abe -
      reedline::engine::Reedline::handle_editor_event::h43f080a14074c43c
                      at /home/kaathewise/.local/state/cargo/registry/src/
      index.crates.io-6f17d22bba15001f/reedline-0.30.0/src/engine.rs:924
        23: 0x560212a3167e -
      reedline::engine::Reedline::handle_editor_event::h43f080a14074c43c
                      at /home/kaathewise/.local/state/cargo/registry/src/
      index.crates.io-6f17d22bba15001f/reedline-0.30.0/src/engine.rs:1232
        24: 0x560212a2de6d -
      reedline::engine::Reedline::handle_event::hdf54c8b3e0974f93
                      at /home/kaathewise/.local/state/cargo/registry/src/
      index.crates.io-6f17d22bba15001f/reedline-0.30.0/src/engine.rs:799
        25: 0x560212a2d567 -
      reedline::engine::Reedline::read_line_helper::h6eb92ca7a94c7c16
                      at /home/kaathewise/.local/state/cargo/registry/src/
      index.crates.io-6f17d22bba15001f/reedline-0.30.0/src/engine.rs:774
        26: 0x560212a2ca40 - reedline::engine::Reedline::read_line::h0eae5bcfcfd36ecc
                      at /home/kaathewise/.local/state/cargo/registry/src/
      index.crates.io-6f17d22bba15001f/reedline-0.30.0/src/engine.rs:633
        27: 0x5602128c73a9 - nu_cli::repl::loop_iteration::h2594b2cf01789510
                      at /home/kaathewise/fork/nushell/crates/nu-cli/src/repl.rs:463
        28: 0x5602128d4725 - nu_cli::repl::evaluate_repl::
      {{closure}}::hf0983c49275ae83b
                      at /home/kaathewise/fork/nushell/crates/nu-cli/src/repl.rs:139
        29: 0x56021290f6ff - <core::panic::unwind_safe::AssertUnwindSafe<F> as
      core::ops::function::FnOnce<()>>::call_once::h6107931275d50bc4
                      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/
      core/src/panic/unwind_safe.rs:272
        30: 0x560212906ab0 - std::panicking::try::do_call::hb158827e2d6f5e15
                      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/
      std/src/panicking.rs:552
        31: 0x560212906b8b - __rust_try
        32: 0x5602129069ad - std::panicking::try::haa1bc36603962107
                      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/
      std/src/panicking.rs:516
        33: 0x5602128de1ee - std::panic::catch_unwind::h56042bac87a17784
                      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/
      std/src/panic.rs:142
        34: 0x5602128c418b - nu_cli::repl::evaluate_repl::hf0503d4f1b546231
                      at /home/kaathewise/fork/nushell/crates/nu-cli/src/repl.rs:138
        35: 0x560211433c0c - nu::run::run_repl::h2c2973aec0f47a2c
                      at /home/kaathewise/fork/nushell/src/run.rs:273
        36: 0x56021145a830 - nu::main::hcbba518c96dbe3ac
                      at /home/kaathewise/fork/nushell/src/main.rs:353
        37: 0x56021145db4b -
      core::ops::function::FnOnce::call_once::h305e3e4bbe292faf
                      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/
      core/src/ops/function.rs:250
  help: set the `RUST_BACKTRACE=1` environment variable to display a backtrace.
@KAAtheWiseGit KAAtheWiseGit added the needs-triage An issue that hasn't had any proper look label Mar 6, 2024
@fdncred fdncred added line editor Issues related to reedline panic labels Mar 6, 2024
@fdncred
Copy link
Collaborator

fdncred commented Mar 6, 2024

ugh, thanks for reporting it.

@fdncred
Copy link
Collaborator

fdncred commented Mar 6, 2024

This is kind of strange.
No panic

calendar<tab>

panic

calendar <tab>

I guess this is just showing the available calendar commands versus showing the parameters for a command.

@fdncred
Copy link
Collaborator

fdncred commented Mar 6, 2024

@Tastaturtaste or @maxomatic458 do either of you have time to look into this completion bug?

@fdncred fdncred added the 🐛 bug Something isn't working label Mar 6, 2024
@Ayanrocks
Copy link

Can this be taken up by a beginner? @fdncred

@KAAtheWiseGit
Copy link
Contributor Author

@Ayanrocks it seems to be a logical error. The panic happens here:

        let mut results = working_set
            .find_commands_by_predicate(filter_predicate, true)
            .into_iter()
            .map(move |x| SemanticSuggestion {
                suggestion: Suggestion {
                    value: String::from_utf8_lossy(&x.0).to_string(),
                    description: x.1,
                    style: None,
                    extra: None,
>                   span: reedline::Span::new(span.start - offset, span.end - offset),
                    append_whitespace: true,
                },
                kind: Some(SuggestionKind::Command(x.2)),
            })
            .collect::<Vec<_>>();

offset is bigger than span's start or end. But determining why this happens will require digging into the Nushell completion code.

@sholderbach sholderbach added completions Issues related to tab completion alias Issues around support for command aliases, touches parser and name resolution and removed needs-triage An issue that hasn't had any proper look labels Apr 14, 2024
@Ayanrocks
Copy link

@Ayanrocks it seems to be a logical error. The panic happens here:

        let mut results = working_set
            .find_commands_by_predicate(filter_predicate, true)
            .into_iter()
            .map(move |x| SemanticSuggestion {
                suggestion: Suggestion {
                    value: String::from_utf8_lossy(&x.0).to_string(),
                    description: x.1,
                    style: None,
                    extra: None,
>                   span: reedline::Span::new(span.start - offset, span.end - offset),
                    append_whitespace: true,
                },
                kind: Some(SuggestionKind::Command(x.2)),
            })
            .collect::<Vec<_>>();

offset is bigger than span's start or end. But determining why this happens will require digging into the Nushell completion code.

I've traced logic to this calling function which is calling the above code block. This is the original code now 👇🏽

// The last item here would be the earliest shape that could possible by part of this subcommand
 let subcommands = if let Some(last) = last {
            println!("Coming HERE ---->");
            self.complete_commands(
                working_set,
                Span::new(last.0.start, pos), <- passing new span instead of passing the original span from the arguments
                offset,
                false,
                options.match_algorithm,
            )
 }

Here I traced the issue to this span that we're creating for new instead of passing the original span

My Fixes 👇🏽

fix #1 -> self.complete_commands(
                working_set,
                Span::new(last.0.start, last.0.end), // change here
                offset,
                false,
                options.match_algorithm,
   )
OR
fix #2 -> self.complete_commands(
                working_set,
                span, // change here
                offset,
                false,
                options.match_algorithm,
       )

Here above ☝🏽 the two possible fixes I could do. Either pass the original span from the arguments of this function or create a new span with (start, end).
Then the panic fixes, but both has different suggestions, The 1st one suggests directories while the second one suggests commands, Can anyone help me if this is the right approach or am I missing something.

@Ayanrocks
Copy link

Any help on this??

@kubouch
Copy link
Contributor

kubouch commented Apr 18, 2024

@Ayanrocks I didn't check everything, but since it is related to complete_commands, then suggesting commands instead of directories sounds more desirable? Feel free to open up a PR with the fix, it's easier to review than in the comments here.

Ayanrocks added a commit to Ayanrocks/nushell that referenced this issue Apr 20, 2024
@Ayanrocks Ayanrocks linked a pull request Apr 20, 2024 that will close this issue
@Ayanrocks
Copy link

Raised a PR: #12596

@sholderbach
Copy link
Member

This is purely incidental but the command cal --week-start monday in the MRE fails at runtime for unrelated reason as made obvious by #12597

@Ayanrocks
Copy link

This is purely incidental but the command cal --week-start monday in the MRE fails at runtime for unrelated reason as made obvious by #12597

even if we change the weekday from monday to mo, still the completion crashes if use calendar TAB

@maxomatic458
Copy link
Contributor

@Ayanrocks can you replicate this on the latest version (so compiled from current main) of the repo.
for me that fixed it, so maybe its been fixed already

@Ayanrocks
Copy link

Ayanrocks commented Apr 21, 2024

@Ayanrocks can you replicate this on the latest version (so compiled from current main) of the repo. for me that fixed it, so maybe its been fixed already
@maxomatic458

Screenshot 2024-04-21 at 11 24 50 PM

Compiled a fresh main branch after taking a pull. Still the panic is happening.

p.s Add a space after calendar and then press TAB

@maxomatic458
Copy link
Contributor

ah ok, i still tried the old command so monday instead of mo so that was probably it.

Thanks for checking

@Ayanrocks
Copy link

Hey @maxomatic458 Can you help me on one doubt, if the offset is the main culprit then what should be the actual value of offset in this case?
Screenshot 2024-04-21 at 11 35 23 PM

it has to be less than the span.start but not more than line.len() (9 in this case) cause reedline panics in that case. But the span.end - span.start diff is 11.

We can connect on discord as well if needed more clarification: https://discord.com/channels/601130461678272522/1229901178696106147

@maxomatic458
Copy link
Contributor

well i guess the panic is just caused because offset is bigger than either the span start or the span end

could be easily fixed by using .saturating_sub instead of -

but i think the root of the problem is probably somewhere else
not sure where that could be (maybe in the code above)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alias Issues around support for command aliases, touches parser and name resolution 🐛 bug Something isn't working completions Issues related to tab completion line editor Issues related to reedline panic
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants