From c5e59efa4d42dc783f93ad6db656692e4e1e4e84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Thu, 17 Aug 2023 17:37:01 +0300 Subject: [PATCH] Sort entries in `scope` commands; Fix usage of externs (#10039) # Description * All output of `scope` commands is sorted by the "name" column. (`scope externs` and some other commands had entries in a weird/random order) * The output of `scope externs` does not have extra newlines (that was due to wrong usage creation of known externals) --- crates/nu-engine/src/scope.rs | 60 ++++++++++++++++---------- crates/nu-parser/src/known_external.rs | 1 + crates/nu-parser/src/parse_keywords.rs | 4 +- tests/scope/mod.rs | 26 +++++++++++ 4 files changed, 67 insertions(+), 24 deletions(-) diff --git a/crates/nu-engine/src/scope.rs b/crates/nu-engine/src/scope.rs index 512d3176b7..913ff22ec9 100644 --- a/crates/nu-engine/src/scope.rs +++ b/crates/nu-engine/src/scope.rs @@ -73,6 +73,8 @@ impl<'e, 's> ScopeData<'e, 's> { span, }) } + + sort_rows(&mut vars); vars } @@ -201,23 +203,8 @@ impl<'e, 's> ScopeData<'e, 's> { } } - commands.sort_by(|a, b| match (a, b) { - (Value::Record { vals: rec_a, .. }, Value::Record { vals: rec_b, .. }) => { - // Comparing the first value from the record - // It is expected that the first value is the name of the column - // The names of the commands should be a value string - match (rec_a.get(0), rec_b.get(0)) { - (Some(val_a), Some(val_b)) => match (val_a, val_b) { - (Value::String { val: str_a, .. }, Value::String { val: str_b, .. }) => { - str_a.cmp(str_b) - } - _ => Ordering::Equal, - }, - _ => Ordering::Equal, - } - } - _ => Ordering::Equal, - }); + sort_rows(&mut commands); + commands } @@ -474,6 +461,7 @@ impl<'e, 's> ScopeData<'e, 's> { } } + sort_rows(&mut externals); externals } @@ -518,7 +506,8 @@ impl<'e, 's> ScopeData<'e, 's> { } } - aliases.sort_by(|a, b| a.partial_cmp(b).unwrap_or(Ordering::Equal)); + sort_rows(&mut aliases); + // aliases.sort_by(|a, b| a.partial_cmp(b).unwrap_or(Ordering::Equal)); aliases } @@ -527,7 +516,7 @@ impl<'e, 's> ScopeData<'e, 's> { let all_decls = module.decls(); - let export_commands: Vec = all_decls + let mut export_commands: Vec = all_decls .iter() .filter_map(|(name_bytes, decl_id)| { let decl = self.engine_state.get_decl(*decl_id); @@ -547,7 +536,7 @@ impl<'e, 's> ScopeData<'e, 's> { }) .collect(); - let export_aliases: Vec = all_decls + let mut export_aliases: Vec = all_decls .iter() .filter_map(|(name_bytes, decl_id)| { let decl = self.engine_state.get_decl(*decl_id); @@ -567,7 +556,7 @@ impl<'e, 's> ScopeData<'e, 's> { }) .collect(); - let export_externs: Vec = all_decls + let mut export_externs: Vec = all_decls .iter() .filter_map(|(name_bytes, decl_id)| { let decl = self.engine_state.get_decl(*decl_id); @@ -587,13 +576,13 @@ impl<'e, 's> ScopeData<'e, 's> { }) .collect(); - let export_submodules: Vec = module + let mut export_submodules: Vec = module .submodules() .iter() .map(|(name_bytes, submodule_id)| self.collect_module(name_bytes, submodule_id, span)) .collect(); - let export_consts: Vec = module + let mut export_consts: Vec = module .vars() .iter() .map(|(name_bytes, var_id)| { @@ -609,6 +598,12 @@ impl<'e, 's> ScopeData<'e, 's> { }) .collect(); + sort_rows(&mut export_commands); + sort_rows(&mut export_aliases); + sort_rows(&mut export_externs); + sort_rows(&mut export_submodules); + sort_rows(&mut export_consts); + let export_env_block = module.env_block.map_or_else( || Value::nothing(span), |block_id| Value::Block { @@ -720,3 +715,22 @@ fn extract_custom_completion_from_arg(engine_state: &EngineState, shape: &Syntax _ => "".to_string(), }; } + +fn sort_rows(decls: &mut [Value]) { + decls.sort_by(|a, b| match (a, b) { + (Value::Record { vals: rec_a, .. }, Value::Record { vals: rec_b, .. }) => { + // Comparing the first value from the record + // It is expected that the first value is the name of the entry (command, module, alias, etc.) + match (rec_a.get(0), rec_b.get(0)) { + (Some(val_a), Some(val_b)) => match (val_a, val_b) { + (Value::String { val: str_a, .. }, Value::String { val: str_b, .. }) => { + str_a.cmp(str_b) + } + _ => Ordering::Equal, + }, + _ => Ordering::Equal, + } + } + _ => Ordering::Equal, + }); +} diff --git a/crates/nu-parser/src/known_external.rs b/crates/nu-parser/src/known_external.rs index 83a1769225..df0ee61222 100644 --- a/crates/nu-parser/src/known_external.rs +++ b/crates/nu-parser/src/known_external.rs @@ -11,6 +11,7 @@ pub struct KnownExternal { pub name: String, pub signature: Box, pub usage: String, + pub extra_usage: String, } impl Command for KnownExternal { diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 50c72e30f5..d3709c82ae 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -215,6 +215,7 @@ pub fn parse_def_predecl(working_set: &mut StateWorkingSet, spans: &[Span]) { let decl = KnownExternal { name, usage: "run external command".into(), + extra_usage: "".into(), signature, }; @@ -650,7 +651,8 @@ pub fn parse_extern( } else { let decl = KnownExternal { name: external_name, - usage: [usage, extra_usage].join("\n"), + usage, + extra_usage, signature, }; diff --git a/tests/scope/mod.rs b/tests/scope/mod.rs index 7e7e77e112..b8c097065c 100644 --- a/tests/scope/mod.rs +++ b/tests/scope/mod.rs @@ -238,6 +238,13 @@ fn correct_scope_externs_fields() { let actual = nu!(cwd: dirs.test(), &inp.join("; ")); assert_eq!(actual.out, "nice extern"); + let inp = &[ + "use spam.nu", + "scope externs | where name == 'spam git' | get 0.usage | str contains (char nl)", + ]; + let actual = nu!(cwd: dirs.test(), &inp.join("; ")); + assert_eq!(actual.out, "false"); + let inp = &[ "use spam.nu", "scope externs | where name == 'spam git' | get 0.decl_id | is-empty", @@ -246,3 +253,22 @@ fn correct_scope_externs_fields() { assert_eq!(actual.out, "false"); }) } + +#[test] +fn scope_externs_sorted() { + let module_setup = r#" + export extern a [] + export extern b [] + export extern c [] + "#; + + let inp = &[ + "extern a []", + "extern b []", + "extern c []", + "scope externs | get name | str join ''", + ]; + + let actual = nu!(&inp.join("; ")); + assert_eq!(actual.out, "abc"); +}