Refactor shell listing related code (#6262)

* Refactor shell listing related code

Signed-off-by: nibon7 <nibon7@163.com>

* add test

Signed-off-by: nibon7 <nibon7@163.com>

crates/nu-command/src/shells/g.rs

@@ -1,11 +1,8 @@
-use super::{get_current_shell, get_shells, switch_shell, SwitchTo};
+use super::{list_shells, switch_shell, SwitchTo};
-use nu_engine::{current_dir, CallExt};
+use nu_engine::CallExt;
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
-use nu_protocol::{
+use nu_protocol::{Category, Example, PipelineData, ShellError, Signature, Spanned, SyntaxShape};
-    Category, Example, IntoInterruptiblePipelineData, PipelineData, ShellError, Signature, Spanned,
-    SyntaxShape, Value,
-};
 
 /// Source a file for environment variables.
 #[derive(Clone)]
@@ -37,17 +34,8 @@ impl Command for GotoShell {
         call: &Call,
         _input: PipelineData,
     ) -> Result<PipelineData, ShellError> {
-        let span = call.head;
         let new_shell: Option<Spanned<String>> = call.opt(engine_state, stack, 0)?;
 
-        let cwd = current_dir(engine_state, stack)?;
-        let cwd = Value::String {
-            val: cwd.to_string_lossy().to_string(),
-            span: call.head,
-        };
-
-        let shells = get_shells(engine_state, stack, cwd);
-
         match new_shell {
             Some(shell_span) => {
                 if shell_span.item == "-" {
@@ -61,25 +49,7 @@ impl Command for GotoShell {
                     switch_shell(engine_state, stack, call, shell_span.span, SwitchTo::Nth(n))
                 }
             }
-            None => {
+            None => list_shells(engine_state, stack, call.head),
-                let current_shell = get_current_shell(engine_state, stack);
-
-                Ok(shells
-                    .into_iter()
-                    .enumerate()
-                    .map(move |(idx, val)| Value::Record {
-                        cols: vec!["active".to_string(), "path".to_string()],
-                        vals: vec![
-                            Value::Bool {
-                                val: idx == current_shell,
-                                span,
-                            },
-                            val,
-                        ],
-                        span,
-                    })
-                    .into_pipeline_data(None))
-            }
         }
     }
 

crates/nu-command/src/shells/mod.rs

@@ -12,7 +12,7 @@ pub use n::NextShell;
 use nu_engine::current_dir;
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{EngineState, Stack};
-use nu_protocol::{PipelineData, ShellError, Span, Value};
+use nu_protocol::{IntoInterruptiblePipelineData, PipelineData, ShellError, Span, Value};
 pub use p::PrevShell;
 pub use shells_::Shells;
 
@@ -123,3 +123,34 @@ fn switch_shell(
 
     Ok(PipelineData::new(call.head))
 }
+
+fn list_shells(
+    engine_state: &EngineState,
+    stack: &mut Stack,
+    span: Span,
+) -> Result<PipelineData, ShellError> {
+    let cwd = current_dir(engine_state, stack)?;
+    let cwd = Value::String {
+        val: cwd.to_string_lossy().to_string(),
+        span,
+    };
+
+    let shells = get_shells(engine_state, stack, cwd);
+    let current_shell = get_current_shell(engine_state, stack);
+
+    Ok(shells
+        .into_iter()
+        .enumerate()
+        .map(move |(idx, val)| Value::Record {
+            cols: vec!["active".to_string(), "path".to_string()],
+            vals: vec![
+                Value::Bool {
+                    val: idx == current_shell,
+                    span,
+                },
+                val,
+            ],
+            span,
+        })
+        .into_pipeline_data(None))
+}

crates/nu-command/src/shells/shells_.rs

@@ -1,10 +1,7 @@
-use super::{get_current_shell, get_shells};
+use super::list_shells;
-use nu_engine::current_dir;
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
-use nu_protocol::{
+use nu_protocol::{Category, Example, PipelineData, ShellError, Signature};
-    Category, Example, IntoInterruptiblePipelineData, PipelineData, ShellError, Signature, Value,
-};
 
 /// Source a file for environment variables.
 #[derive(Clone)]
@@ -30,32 +27,7 @@ impl Command for Shells {
         call: &Call,
         _input: PipelineData,
     ) -> Result<PipelineData, ShellError> {
-        let span = call.head;
+        list_shells(engine_state, stack, call.head)
-        let cwd = current_dir(engine_state, stack)?;
-        let cwd = Value::String {
-            val: cwd.to_string_lossy().to_string(),
-            span,
-        };
-
-        let shells = get_shells(engine_state, stack, cwd);
-        let current_shell = get_current_shell(engine_state, stack);
-
-        let output = shells
-            .into_iter()
-            .enumerate()
-            .map(move |(idx, val)| Value::Record {
-                cols: vec!["active".to_string(), "path".to_string()],
-                vals: vec![
-                    Value::Bool {
-                        val: idx == current_shell,
-                        span,
-                    },
-                    val,
-                ],
-                span,
-            });
-
-        Ok(output.into_pipeline_data(None))
     }
 
     fn examples(&self) -> Vec<Example> {

crates/nu-command/tests/commands/mod.rs

@@ -65,6 +65,7 @@ mod run_external;
 mod save;
 mod select;
 mod semicolon;
+mod shells;
 mod skip;
 mod sort_by;
 mod source;

crates/nu-command/tests/commands/shells.rs

@@ -0,0 +1,31 @@
+use nu_test_support::{nu, pipeline, playground::Playground};
+
+#[test]
+fn list_shells_1() {
+    Playground::setup("list_shells_1", |dirs, sandbox| {
+        sandbox.mkdir("foo").mkdir("bar");
+
+        let actual = nu!(
+            cwd: dirs.test(),
+            pipeline(
+            r#"enter foo; enter ../bar; g| get active.2"#
+        ));
+
+        assert_eq!(actual.out, "true");
+    })
+}
+
+#[test]
+fn list_shells_2() {
+    Playground::setup("list_shells_2", |dirs, sandbox| {
+        sandbox.mkdir("foo").mkdir("bar");
+
+        let actual = nu!(
+            cwd: dirs.test(),
+            pipeline(
+            r#"enter foo; enter ../bar; shells| get active.2"#
+        ));
+
+        assert_eq!(actual.out, "true");
+    })
+}