Skip to content

Commit 524146f

Browse files
authored
fix(completion): an edge case of aliased external head command (nushell#16882)
Fixes this ignored test case: https://github.com/nushell/nushell/blob/a9263cf13489517b3a185ae1e33ecc41bddedb5f/crates/nu-cli/tests/completions/mod.rs#L2643-L2660 Previously `e<tab>` gets empty results because it dives deep into the external head expression, `^$env.EDITOR` in this case. This PR fixes it using the same mechanism as nushell#16876 and nushell#16640 ## Release notes summary - What our users need to know Fixed a bug in aliased external command completion where `alias ea = ^$env.EDITOR /tmp/test.s; e<tab>` gets empty result. ## Tasks after submitting
1 parent 33bba81 commit 524146f

File tree

2 files changed

+23
-22
lines changed

2 files changed

+23
-22
lines changed

crates/nu-cli/src/completions/completer.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::completions::{
77
use nu_color_config::{color_record_to_nustyle, lookup_ansi_color_style};
88
use nu_parser::{parse, parse_module_file_or_dir};
99
use nu_protocol::{
10-
CommandWideCompleter, Completion, Span, Type, Value,
10+
CommandWideCompleter, Completion, GetSpan, Span, Type, Value,
1111
ast::{
1212
Argument, Block, Expr, Expression, FindMapResult, ListItem, PipelineRedirection,
1313
RedirectionTarget, Traverse,
@@ -55,14 +55,25 @@ fn find_pipeline_element_by_position<'a>(
5555
Expr::ExternalCall(head, arguments) => arguments
5656
.iter()
5757
.find_map(|arg| arg.expr().find_map(working_set, &closure))
58-
.or(head.as_ref().find_map(working_set, &closure))
58+
.or_else(|| {
59+
// For aliased external_call, the span of original external command head should fail the
60+
// contains(pos) check, thus avoiding recursion into its head expression.
61+
// See issue #7648 for details.
62+
let span = working_set.get_span(head.span_id);
63+
if span.contains(pos) {
64+
// This is for complicated external head expressions, e.g. `^(echo<tab> foo)`
65+
head.as_ref().find_map(working_set, &closure)
66+
} else {
67+
None
68+
}
69+
})
5970
.or(Some(expr))
6071
.map(FindMapResult::Found)
6172
.unwrap_or_default(),
6273
// complete the operator
6374
Expr::BinaryOp(lhs, _, rhs) => lhs
6475
.find_map(working_set, &closure)
65-
.or(rhs.find_map(working_set, &closure))
76+
.or_else(|| rhs.find_map(working_set, &closure))
6677
.or(Some(expr))
6778
.map(FindMapResult::Found)
6879
.unwrap_or_default(),

crates/nu-cli/tests/completions/mod.rs

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -760,17 +760,13 @@ fn dotnu_completions_const_nu_lib_dirs() {
760760
}
761761

762762
#[test]
763-
#[ignore]
764763
fn external_completer_trailing_space() {
765764
// https://github.com/nushell/nushell/issues/6378
766765
let block = "{|spans| $spans}";
767766
let input = "gh alias ";
768767

769768
let suggestions = run_external_completion(block, input);
770-
assert_eq!(3, suggestions.len());
771-
assert_eq!("gh", suggestions.first().unwrap().value);
772-
assert_eq!("alias", suggestions.get(1).unwrap().value);
773-
assert_eq!("", suggestions.get(2).unwrap().value);
769+
match_suggestions(&vec!["gh", "alias", ""], &suggestions);
774770
}
775771

776772
#[test]
@@ -2640,7 +2636,6 @@ fn exact_match_case_insensitive() {
26402636
});
26412637
}
26422638

2643-
#[ignore = "was reverted, still needs fixing"]
26442639
#[rstest]
26452640
fn alias_offset_bug_7648() {
26462641
let (_, _, mut engine, mut stack) = new_engine();
@@ -2650,16 +2645,15 @@ fn alias_offset_bug_7648() {
26502645
assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack).is_ok());
26512646

26522647
let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack));
2648+
let suggestions = completer.complete("e", 1);
2649+
assert!(!suggestions.is_empty());
26532650

2654-
// Issue #7648
2655-
// Nushell crashes when an alias name is shorter than the alias command
2656-
// and the alias command is a external command
2657-
// This happens because of offset is not correct.
2658-
// This crashes before PR #7779
2659-
let _suggestions = completer.complete("e", 1);
2651+
// Make sure completion in complicated external head expression still works
2652+
let input = "^(ls | e";
2653+
let suggestions = completer.complete(input, input.len());
2654+
assert!(!suggestions.is_empty());
26602655
}
26612656

2662-
#[ignore = "was reverted, still needs fixing"]
26632657
#[rstest]
26642658
fn alias_offset_bug_7754() {
26652659
let (_, _, mut engine, mut stack) = new_engine();
@@ -2669,12 +2663,8 @@ fn alias_offset_bug_7754() {
26692663
assert!(support::merge_input(alias.as_bytes(), &mut engine, &mut stack).is_ok());
26702664

26712665
let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack));
2672-
2673-
// Issue #7754
2674-
// Nushell crashes when an alias name is shorter than the alias command
2675-
// and the alias command contains pipes.
2676-
// This crashes before PR #7756
2677-
let _suggestions = completer.complete("ll -a | c", 9);
2666+
let suggestions = completer.complete("ll -a | c", 9);
2667+
assert!(!suggestions.is_empty());
26782668
}
26792669

26802670
#[rstest]

0 commit comments

Comments
 (0)