explore: pass config to views at creation time (#13312)

cc: @zhiburt

This is an internal refactoring for `explore`.

Previously, views inside `explore` were created with default/incorrect
configuration and then the correct configuration was passed to them
using a function called `setup()`. I believe this was because
configuration was dynamic and could change while `explore` was running.

After https://github.com/nushell/nushell/pull/10259, configuration can
no longer be changed on the fly. So we can clean this up by removing
`setup()` and passing configuration to views when they are created.

crates/nu-explore/src/commands/expand.rs

@@ -1,7 +1,7 @@
 use super::ViewCommand;
 use crate::{
     nu_common::{self, collect_input},
-    views::Preview,
+    views::{Preview, ViewConfig},
 };
 use anyhow::Result;
 use nu_color_config::StyleComputer;
@@ -43,6 +43,7 @@ impl ViewCommand for ExpandCmd {
         engine_state: &EngineState,
         stack: &mut Stack,
         value: Option<Value>,
+        _: &ViewConfig,
     ) -> Result<Self::View> {
         if let Some(value) = value {
             let value_as_string = convert_value_to_string(value, engine_state, stack)?;

crates/nu-explore/src/commands/help.rs

@@ -1,5 +1,5 @@
 use super::ViewCommand;
-use crate::views::Preview;
+use crate::views::{Preview, ViewConfig};
 use anyhow::Result;
 use nu_ansi_term::Color;
 use nu_protocol::{
@@ -99,7 +99,13 @@ impl ViewCommand for HelpCmd {
         Ok(())
     }
 
-    fn spawn(&mut self, _: &EngineState, _: &mut Stack, _: Option<Value>) -> Result<Self::View> {
+    fn spawn(
+        &mut self,
+        _: &EngineState,
+        _: &mut Stack,
+        _: Option<Value>,
+        _: &ViewConfig,
+    ) -> Result<Self::View> {
         Ok(HelpCmd::view())
     }
 }

crates/nu-explore/src/commands/mod.rs

@@ -1,3 +1,5 @@
+use crate::views::ViewConfig;
+
 use super::pager::{Pager, Transition};
 use anyhow::Result;
 use nu_protocol::{
@@ -49,5 +51,6 @@ pub trait ViewCommand {
         engine_state: &EngineState,
         stack: &mut Stack,
         value: Option<Value>,
+        config: &ViewConfig,
     ) -> Result<Self::View>;
 }

crates/nu-explore/src/commands/nu.rs

@@ -48,6 +48,7 @@ impl ViewCommand for NuCmd {
         engine_state: &EngineState,
         stack: &mut Stack,
         value: Option<Value>,
+        config: &ViewConfig,
     ) -> Result<Self::View> {
         let value = value.unwrap_or_default();
 
@@ -62,7 +63,7 @@ impl ViewCommand for NuCmd {
             return Ok(NuView::Preview(Preview::new(&text)));
         }
 
-        let mut view = RecordView::new(columns, values);
+        let mut view = RecordView::new(columns, values, config.explore_config.clone());
 
         if is_record {
             view.set_top_layer_orientation(Orientation::Left);
@@ -119,11 +120,4 @@ impl View for NuView {
             NuView::Preview(v) => v.exit(),
         }
     }
-
-    fn setup(&mut self, config: ViewConfig<'_>) {
-        match self {
-            NuView::Records(v) => v.setup(config),
-            NuView::Preview(v) => v.setup(config),
-        }
-    }
 }

crates/nu-explore/src/commands/table.rs

@@ -1,7 +1,7 @@
 use super::ViewCommand;
 use crate::{
     nu_common::collect_input,
-    views::{Orientation, RecordView},
+    views::{Orientation, RecordView, ViewConfig},
 };
 use anyhow::Result;
 use nu_protocol::{
@@ -49,13 +49,14 @@ impl ViewCommand for TableCmd {
         _: &EngineState,
         _: &mut Stack,
         value: Option<Value>,
+        config: &ViewConfig,
     ) -> Result<Self::View> {
         let value = value.unwrap_or_default();
         let is_record = matches!(value, Value::Record { .. });
 
         let (columns, data) = collect_input(value)?;
 
-        let mut view = RecordView::new(columns, data);
+        let mut view = RecordView::new(columns, data, config.explore_config.clone());
 
         if is_record {
             view.set_top_layer_orientation(Orientation::Left);

crates/nu-explore/src/commands/try.rs

@@ -1,5 +1,5 @@
 use super::ViewCommand;
-use crate::views::TryView;
+use crate::views::{TryView, ViewConfig};
 use anyhow::Result;
 use nu_protocol::{
     engine::{EngineState, Stack},
@@ -43,9 +43,10 @@ impl ViewCommand for TryCmd {
         engine_state: &EngineState,
         stack: &mut Stack,
         value: Option<Value>,
+        config: &ViewConfig,
     ) -> Result<Self::View> {
         let value = value.unwrap_or_default();
-        let mut view = TryView::new(value);
+        let mut view = TryView::new(value, config.explore_config.clone());
         view.init(self.command.clone());
         view.try_run(engine_state, stack)?;
 

crates/nu-explore/src/lib.rs

@@ -10,6 +10,7 @@ use anyhow::Result;
 use commands::{ExpandCmd, HelpCmd, NuCmd, QuitCmd, TableCmd, TryCmd};
 pub use default_context::add_explore_context;
 pub use explore::Explore;
+use explore::ExploreConfig;
 use nu_common::{collect_pipeline, has_simple_value, CtrlC};
 use nu_protocol::{
     engine::{EngineState, Stack},
@@ -43,7 +44,7 @@ fn run_pager(
     if is_binary {
         p.show_message("For help type :help");
 
-        let view = binary_view(input)?;
+        let view = binary_view(input, config.explore_config)?;
         return p.run(engine_state, stack, ctrlc, Some(view), commands);
     }
 
@@ -73,7 +74,7 @@ fn create_record_view(
     is_record: bool,
     config: PagerConfig,
 ) -> Option<Page> {
-    let mut view = RecordView::new(columns, data);
+    let mut view = RecordView::new(columns, data, config.explore_config.clone());
     if is_record {
         view.set_top_layer_orientation(Orientation::Left);
     }
@@ -91,14 +92,14 @@ fn help_view() -> Option<Page> {
     Some(Page::new(HelpCmd::view(), false))
 }
 
-fn binary_view(input: PipelineData) -> Result<Page> {
+fn binary_view(input: PipelineData, config: &ExploreConfig) -> Result<Page> {
     let data = match input {
         PipelineData::Value(Value::Binary { val, .. }, _) => val,
         PipelineData::ByteStream(bs, _) => bs.into_bytes()?,
         _ => unreachable!("checked beforehand"),
     };
 
-    let view = BinaryView::new(data);
+    let view = BinaryView::new(data, config);
 
     Ok(Page::new(view, true))
 }

crates/nu-explore/src/pager/mod.rs

@@ -90,19 +90,42 @@ impl<'a> Pager<'a> {
         engine_state: &EngineState,
         stack: &mut Stack,
         ctrlc: CtrlC,
-        mut view: Option<Page>,
+        view: Option<Page>,
         commands: CommandRegistry,
     ) -> Result<Option<Value>> {
-        if let Some(page) = &mut view {
+        // setup terminal
-            page.view.setup(ViewConfig::new(
+        enable_raw_mode()?;
-                self.config.nu_config,
+        let mut stdout = io::stdout();
-                self.config.explore_config,
+        execute!(stdout, EnterAlternateScreen, Clear(ClearType::All))?;
-                self.config.style_computer,
+
-                self.config.lscolors,
+        let backend = CrosstermBackend::new(stdout);
-            ))
+        let mut terminal = Terminal::new(backend)?;
+
+        let mut info = ViewInfo {
+            status: Some(Report::default()),
+            ..Default::default()
+        };
+
+        if let Some(text) = self.message.take() {
+            info.status = Some(Report::message(text, Severity::Info));
         }
 
-        run_pager(engine_state, stack, ctrlc, self, view, commands)
+        let result = render_ui(
+            &mut terminal,
+            engine_state,
+            stack,
+            ctrlc,
+            self,
+            &mut info,
+            view,
+            commands,
+        )?;
+
+        // restore terminal
+        disable_raw_mode()?;
+        execute!(io::stdout(), LeaveAlternateScreen)?;
+
+        Ok(result)
     }
 }
 
@@ -145,49 +168,6 @@ impl<'a> PagerConfig<'a> {
     }
 }
 
-fn run_pager(
-    engine_state: &EngineState,
-    stack: &mut Stack,
-    ctrlc: CtrlC,
-    pager: &mut Pager,
-    view: Option<Page>,
-    commands: CommandRegistry,
-) -> Result<Option<Value>> {
-    // setup terminal
-    enable_raw_mode()?;
-    let mut stdout = io::stdout();
-    execute!(stdout, EnterAlternateScreen, Clear(ClearType::All))?;
-
-    let backend = CrosstermBackend::new(stdout);
-    let mut terminal = Terminal::new(backend)?;
-
-    let mut info = ViewInfo {
-        status: Some(Report::default()),
-        ..Default::default()
-    };
-
-    if let Some(text) = pager.message.take() {
-        info.status = Some(Report::message(text, Severity::Info));
-    }
-
-    let result = render_ui(
-        &mut terminal,
-        engine_state,
-        stack,
-        ctrlc,
-        pager,
-        &mut info,
-        view,
-        commands,
-    )?;
-
-    // restore terminal
-    disable_raw_mode()?;
-    execute!(io::stdout(), LeaveAlternateScreen)?;
-
-    Ok(result)
-}
-
 #[allow(clippy::too_many_arguments)]
 fn render_ui(
     term: &mut Terminal,
@@ -440,15 +420,20 @@ fn run_command(
         Command::View { mut cmd, stackable } => {
             // what we do we just replace the view.
             let value = view_stack.curr_view.as_mut().and_then(|p| p.view.exit());
-            let mut new_view = cmd.spawn(engine_state, stack, value)?;
+            let view_cfg = ViewConfig::new(
+                pager.config.nu_config,
+                pager.config.explore_config,
+                pager.config.style_computer,
+                pager.config.lscolors,
+            );
+
+            let new_view = cmd.spawn(engine_state, stack, value, &view_cfg)?;
             if let Some(view) = view_stack.curr_view.take() {
                 if !view.stackable {
                     view_stack.stack.push(view);
                 }
             }
 
-            setup_view(&mut new_view, &pager.config);
-
             view_stack.curr_view = Some(Page::raw(new_view, stackable));
 
             Ok(CmdResult::new(false, true, cmd.name().to_owned()))
@@ -456,16 +441,6 @@ fn run_command(
     }
 }
 
-fn setup_view(view: &mut Box<dyn View>, cfg: &PagerConfig<'_>) {
-    let cfg = ViewConfig::new(
-        cfg.nu_config,
-        cfg.explore_config,
-        cfg.style_computer,
-        cfg.lscolors,
-    );
-    view.setup(cfg);
-}
-
 fn set_cursor_cmd_bar(f: &mut Frame, area: Rect, pager: &Pager) {
     if pager.cmd_buf.is_cmd_input {
         // todo: deal with a situation where we exceed the bar width

crates/nu-explore/src/registry/command.rs

@@ -1,6 +1,6 @@
 use crate::{
     commands::{SimpleCommand, ViewCommand},
-    views::View,
+    views::{View, ViewConfig},
 };
 use anyhow::Result;
 
@@ -78,8 +78,9 @@ where
         engine_state: &nu_protocol::engine::EngineState,
         stack: &mut nu_protocol::engine::Stack,
         value: Option<nu_protocol::Value>,
+        cfg: &ViewConfig,
     ) -> Result<Self::View> {
-        let view = self.0.spawn(engine_state, stack, value)?;
+        let view = self.0.spawn(engine_state, stack, value, cfg)?;
         Ok(Box::new(view) as Box<dyn View>)
     }
 }

crates/nu-explore/src/views/binary/mod.rs

@@ -40,11 +40,15 @@ struct Settings {
 }
 
 impl BinaryView {
-    pub fn new(data: Vec<u8>) -> Self {
+    pub fn new(data: Vec<u8>, cfg: &ExploreConfig) -> Self {
+        let settings = settings_from_config(cfg);
+        // There's gotta be a nicer way of doing this than creating a widget just to count lines
+        let count_rows = BinaryWidget::new(&data, settings.opts, Default::default()).count_lines();
+
         Self {
             data,
-            cursor: WindowCursor2D::default(),
+            cursor: WindowCursor2D::new(count_rows, 1).expect("Failed to create XYCursor"),
-            settings: Settings::default(),
+            settings,
         }
     }
 }
@@ -87,15 +91,6 @@ impl View for BinaryView {
         // todo: impl Cursor + peek of a value
         None
     }
-
-    fn setup(&mut self, cfg: ViewConfig<'_>) {
-        self.settings = settings_from_config(cfg.explore_config);
-
-        let count_rows =
-            BinaryWidget::new(&self.data, self.settings.opts, Default::default()).count_lines();
-        // TODO: refactor View so setup() is fallible and we don't have to panic here
-        self.cursor = WindowCursor2D::new(count_rows, 1).expect("Failed to create XYCursor");
-    }
 }
 
 fn create_binary_widget(v: &BinaryView) -> BinaryWidget<'_> {

crates/nu-explore/src/views/mod.rs

@@ -99,8 +99,6 @@ pub trait View {
     fn exit(&mut self) -> Option<Value> {
         None
     }
-
-    fn setup(&mut self, _: ViewConfig<'_>) {}
 }
 
 impl View for Box<dyn View> {
@@ -131,8 +129,4 @@ impl View for Box<dyn View> {
     fn show_data(&mut self, i: usize) -> bool {
         self.as_mut().show_data(i)
     }
-
-    fn setup(&mut self, cfg: ViewConfig<'_>) {
-        self.as_mut().setup(cfg)
-    }
 }

crates/nu-explore/src/views/record/mod.rs

@@ -36,14 +36,12 @@ pub struct RecordView {
 }
 
 impl RecordView {
-    pub fn new(columns: Vec<String>, records: Vec<Vec<Value>>) -> Self {
+    pub fn new(columns: Vec<String>, records: Vec<Vec<Value>>, cfg: ExploreConfig) -> Self {
         Self {
             layer_stack: vec![RecordLayer::new(columns, records)],
             mode: UIMode::View,
             orientation: Orientation::Top,
-            // TODO: It's kind of gross how this temporarily has an incorrect/default config.
+            cfg,
-            // See if we can pass correct config in through the constructor
-            cfg: ExploreConfig::default(),
         }
     }
 
@@ -72,23 +70,6 @@ impl RecordView {
             .expect("we guarantee that 1 entry is always in a list")
     }
 
-    pub fn get_top_layer_orientation(&mut self) -> Orientation {
-        self.get_top_layer().orientation
-    }
-
-    pub fn set_orientation(&mut self, orientation: Orientation) {
-        self.orientation = orientation;
-
-        // we need to reset all indexes as we can't no more use them.
-        self.reset_cursors();
-    }
-
-    fn reset_cursors(&mut self) {
-        for layer in &mut self.layer_stack {
-            layer.reset_cursor();
-        }
-    }
-
     pub fn set_top_layer_orientation(&mut self, orientation: Orientation) {
         let layer = self.get_top_layer_mut();
         layer.orientation = orientation;
@@ -299,11 +280,6 @@ impl View for RecordView {
     fn exit(&mut self) -> Option<Value> {
         Some(build_last_value(self))
     }
-
-    // todo: move the method to Command?
-    fn setup(&mut self, cfg: ViewConfig<'_>) {
-        self.cfg = cfg.explore_config.clone();
-    }
 }
 
 fn get_element_info(

crates/nu-explore/src/views/try.rs

@@ -1,5 +1,6 @@
 use super::{record::RecordView, util::nu_style_to_tui, Layout, Orientation, View, ViewConfig};
 use crate::{
+    explore::ExploreConfig,
     nu_common::{collect_pipeline, run_command_with_value},
     pager::{report::Report, Frame, Transition, ViewInfo},
 };
@@ -23,17 +24,19 @@ pub struct TryView {
     table: Option<RecordView>,
     view_mode: bool,
     border_color: Style,
+    config: ExploreConfig,
 }
 
 impl TryView {
-    pub fn new(input: Value) -> Self {
+    pub fn new(input: Value, config: ExploreConfig) -> Self {
         Self {
             input,
             table: None,
-            immediate: false,
+            immediate: config.try_reactive,
-            border_color: Style::default(),
+            border_color: nu_style_to_tui(config.table.separator_style),
             view_mode: false,
             command: String::new(),
+            config,
         }
     }
 
@@ -42,7 +45,13 @@ impl TryView {
     }
 
     pub fn try_run(&mut self, engine_state: &EngineState, stack: &mut Stack) -> Result<()> {
-        let view = run_command(&self.command, &self.input, engine_state, stack)?;
+        let view = run_command(
+            &self.command,
+            &self.input,
+            engine_state,
+            stack,
+            &self.config,
+        )?;
         self.table = Some(view);
         Ok(())
     }
@@ -122,7 +131,6 @@ impl View for TryView {
         f.render_widget(table_block, table_area);
 
         if let Some(table) = &mut self.table {
-            table.setup(cfg);
             let area = Rect::new(
                 area.x + 2,
                 area.y + 4,
@@ -232,20 +240,6 @@ impl View for TryView {
     fn show_data(&mut self, i: usize) -> bool {
         self.table.as_mut().map_or(false, |v| v.show_data(i))
     }
-
-    fn setup(&mut self, config: ViewConfig<'_>) {
-        self.border_color = nu_style_to_tui(config.explore_config.table.separator_style);
-        self.immediate = config.explore_config.try_reactive;
-
-        let mut r = RecordView::new(vec![], vec![]);
-        r.setup(config);
-
-        if let Some(view) = &mut self.table {
-            view.setup(config);
-            view.set_orientation(r.get_top_layer_orientation());
-            view.set_top_layer_orientation(r.get_top_layer_orientation());
-        }
-    }
 }
 
 fn run_command(
@@ -253,6 +247,7 @@ fn run_command(
     input: &Value,
     engine_state: &EngineState,
     stack: &mut Stack,
+    config: &ExploreConfig,
 ) -> Result<RecordView> {
     let pipeline = run_command_with_value(command, input, engine_state, stack)?;
 
@@ -260,7 +255,7 @@ fn run_command(
 
     let (columns, values) = collect_pipeline(pipeline)?;
 
-    let mut view = RecordView::new(columns, values);
+    let mut view = RecordView::new(columns, values, config.clone());
     if is_record {
         view.set_top_layer_orientation(Orientation::Left);
     }