Revert "Add the history import command" (#14077)

crates/nu-cli/src/commands/default_context.rs

@@ -17,7 +17,6 @@ pub fn add_cli_context(mut engine_state: EngineState) -> EngineState {
             CommandlineGetCursor,
             CommandlineSetCursor,
             History,
-            HistoryImport,
             HistorySession,
             Keybindings,
             KeybindingsDefault,

crates/nu-cli/src/commands/history/fields.rs

@@ -1,10 +0,0 @@
-// Each const is named after a HistoryItem field, and the value is the field name to be displayed to
-// the user (or accept during import).
-pub const COMMAND_LINE: &str = "command";
-pub const ID: &str = "item_id";
-pub const START_TIMESTAMP: &str = "start_timestamp";
-pub const HOSTNAME: &str = "hostname";
-pub const CWD: &str = "cwd";
-pub const EXIT_STATUS: &str = "exit_status";
-pub const DURATION: &str = "duration";
-pub const SESSION_ID: &str = "session_id";

crates/nu-cli/src/commands/history/history_.rs

@@ -5,8 +5,6 @@ use reedline::{
     SqliteBackedHistory,
 };
 
-use super::fields;
-
 #[derive(Clone)]
 pub struct History;
 
@@ -85,8 +83,7 @@ impl Command for History {
                     entries.into_iter().enumerate().map(move |(idx, entry)| {
                         Value::record(
                             record! {
-                                fields::COMMAND_LINE => Value::string(entry.command_line, head),
+                                "command" => Value::string(entry.command_line, head),
-                                // TODO: This name is inconsistent with create_history_record.
                                 "index" => Value::int(idx as i64, head),
                             },
                             head,
@@ -178,14 +175,14 @@ fn create_history_record(idx: usize, entry: HistoryItem, long: bool, head: Span)
     if long {
         Value::record(
             record! {
-                fields::ID => item_id_value,
+                "item_id" => item_id_value,
-                fields::START_TIMESTAMP => start_timestamp_value,
+                "start_timestamp" => start_timestamp_value,
-                fields::COMMAND_LINE => command_value,
+                "command" => command_value,
-                fields::SESSION_ID => session_id_value,
+                "session_id" => session_id_value,
-                fields::HOSTNAME => hostname_value,
+                "hostname" => hostname_value,
-                fields::CWD => cwd_value,
+                "cwd" => cwd_value,
-                fields::DURATION => duration_value,
+                "duration" => duration_value,
-                fields::EXIT_STATUS => exit_status_value,
+                "exit_status" => exit_status_value,
                 "idx" => index_value,
             },
             head,
@@ -193,11 +190,11 @@ fn create_history_record(idx: usize, entry: HistoryItem, long: bool, head: Span)
     } else {
         Value::record(
             record! {
-                fields::START_TIMESTAMP => start_timestamp_value,
+                "start_timestamp" => start_timestamp_value,
-                fields::COMMAND_LINE => command_value,
+                "command" => command_value,
-                fields::CWD => cwd_value,
+                "cwd" => cwd_value,
-                fields::DURATION => duration_value,
+                "duration" => duration_value,
-                fields::EXIT_STATUS => exit_status_value,
+                "exit_status" => exit_status_value,
             },
             head,
         )

crates/nu-cli/src/commands/history/history_import.rs

@@ -1,332 +0,0 @@
-use nu_engine::command_prelude::*;
-use nu_protocol::HistoryFileFormat;
-
-use reedline::{
-    FileBackedHistory, History, HistoryItem, HistoryItemId, ReedlineError, SearchQuery,
-    SqliteBackedHistory,
-};
-
-use super::fields;
-
-#[derive(Clone)]
-pub struct HistoryImport;
-
-impl Command for HistoryImport {
-    fn name(&self) -> &str {
-        "history import"
-    }
-
-    fn description(&self) -> &str {
-        "Import command line history"
-    }
-
-    fn extra_description(&self) -> &str {
-        r#"Can import history from input, either successive command lines or more detailed records. If providing records, available fields are:
-    command_line, id, start_timestamp, hostname, cwd, duration, exit_status.
-
-If no input is provided, will import all history items from existing history in the other format: if current history is stored in sqlite, it will store it in plain text and vice versa."#
-    }
-
-    fn signature(&self) -> nu_protocol::Signature {
-        Signature::build("history import")
-            .category(Category::History)
-            .input_output_types(vec![
-                (Type::Nothing, Type::Nothing),
-                (Type::List(Box::new(Type::String)), Type::Nothing),
-                (Type::table(), Type::Nothing),
-            ])
-    }
-
-    fn examples(&self) -> Vec<Example> {
-        vec![
-            Example {
-                example: "history import",
-                description:
-                    "Append all items from history in the other format to the current history",
-                result: None,
-            },
-            Example {
-                example: "echo foo | history import",
-                description: "Append `foo` to the current history",
-                result: None,
-            },
-            Example {
-                example: "[[ command_line cwd ]; [ foo /home ]] | history import",
-                description: "Append `foo` ran from `/home` to the current history",
-                result: None,
-            },
-        ]
-    }
-
-    fn run(
-        &self,
-        engine_state: &EngineState,
-        _stack: &mut Stack,
-        call: &Call,
-        input: PipelineData,
-    ) -> Result<PipelineData, ShellError> {
-        let ok = Ok(Value::nothing(call.head).into_pipeline_data());
-
-        let Some(history) = engine_state.history_config() else {
-            return ok;
-        };
-        let Some(current_history_path) = history.file_path() else {
-            return Err(ShellError::ConfigDirNotFound {
-                span: Some(call.head),
-            });
-        };
-
-        match input {
-            PipelineData::Empty => {
-                let other_format = match history.file_format {
-                    HistoryFileFormat::Sqlite => HistoryFileFormat::Plaintext,
-                    HistoryFileFormat::Plaintext => HistoryFileFormat::Sqlite,
-                };
-                let src = new_backend(other_format, None)?;
-                let mut dst = new_backend(history.file_format, Some(current_history_path))?;
-                let items = src
-                    .search(SearchQuery::everything(
-                        reedline::SearchDirection::Forward,
-                        None,
-                    ))
-                    .map_err(error_from_reedline)?
-                    .into_iter()
-                    .map(Ok);
-                import(dst.as_mut(), items)
-            }
-            _ => {
-                let input = input.into_iter().map(item_from_value);
-                import(
-                    new_backend(history.file_format, Some(current_history_path))?.as_mut(),
-                    input,
-                )
-            }
-        }?;
-
-        ok
-    }
-}
-
-fn new_backend(
-    format: HistoryFileFormat,
-    path: Option<std::path::PathBuf>,
-) -> Result<Box<dyn History>, ShellError> {
-    let path = match path {
-        Some(path) => path,
-        None => {
-            let Some(mut path) = nu_path::nu_config_dir() else {
-                return Err(ShellError::ConfigDirNotFound { span: None });
-            };
-            path.push(format.default_file_name());
-            path.into_std_path_buf()
-        }
-    };
-
-    fn map(
-        result: Result<impl History + 'static, ReedlineError>,
-    ) -> Result<Box<dyn History>, ShellError> {
-        result
-            .map(|x| Box::new(x) as Box<dyn History>)
-            .map_err(error_from_reedline)
-    }
-    match format {
-        // Use a reasonably large value for maximum capacity.
-        HistoryFileFormat::Plaintext => map(FileBackedHistory::with_file(0xfffffff, path)),
-        HistoryFileFormat::Sqlite => map(SqliteBackedHistory::with_file(path, None, None)),
-    }
-}
-
-fn import(
-    dst: &mut dyn History,
-    src: impl Iterator<Item = Result<HistoryItem, ShellError>>,
-) -> Result<(), ShellError> {
-    for item in src {
-        dst.save(item?).map_err(error_from_reedline)?;
-    }
-    Ok(())
-}
-
-fn error_from_reedline(e: ReedlineError) -> ShellError {
-    // TODO: Should we add a new ShellError variant?
-    ShellError::GenericError {
-        error: "Reedline error".to_owned(),
-        msg: format!("{e}"),
-        span: None,
-        help: None,
-        inner: Vec::new(),
-    }
-}
-
-fn item_from_value(v: Value) -> Result<HistoryItem, ShellError> {
-    let span = v.span();
-    match v {
-        Value::Record { val, .. } => item_from_record(val.into_owned(), span),
-        Value::String { val, .. } => Ok(HistoryItem {
-            command_line: val,
-            id: None,
-            start_timestamp: None,
-            session_id: None,
-            hostname: None,
-            cwd: None,
-            duration: None,
-            exit_status: None,
-            more_info: None,
-        }),
-        _ => Err(ShellError::UnsupportedInput {
-            msg: "Only list and record inputs are supported".to_owned(),
-            input: v.get_type().to_string(),
-            msg_span: span,
-            input_span: span,
-        }),
-    }
-}
-
-fn item_from_record(mut rec: Record, span: Span) -> Result<HistoryItem, ShellError> {
-    let cmd = match rec.remove(fields::COMMAND_LINE) {
-        Some(v) => v.as_str()?.to_owned(),
-        None => {
-            return Err(ShellError::TypeMismatch {
-                err_message: format!("missing column: {}", fields::COMMAND_LINE),
-                span,
-            })
-        }
-    };
-
-    fn get<T>(
-        rec: &mut Record,
-        field: &'static str,
-        f: impl FnOnce(Value) -> Result<T, ShellError>,
-    ) -> Result<Option<T>, ShellError> {
-        rec.remove(field).map(f).transpose()
-    }
-
-    let rec = &mut rec;
-    let item = HistoryItem {
-        command_line: cmd,
-        id: get(rec, fields::ID, |v| Ok(HistoryItemId::new(v.as_int()?)))?,
-        start_timestamp: get(rec, fields::START_TIMESTAMP, |v| Ok(v.as_date()?.to_utc()))?,
-        hostname: get(rec, fields::HOSTNAME, |v| Ok(v.as_str()?.to_owned()))?,
-        cwd: get(rec, fields::CWD, |v| Ok(v.as_str()?.to_owned()))?,
-        exit_status: get(rec, fields::EXIT_STATUS, |v| v.as_i64())?,
-        duration: get(rec, fields::DURATION, duration_from_value)?,
-        more_info: None,
-        // TODO: Currently reedline doesn't let you create session IDs.
-        session_id: None,
-    };
-
-    if !rec.is_empty() {
-        let cols = rec.columns().map(|s| s.as_str()).collect::<Vec<_>>();
-        return Err(ShellError::TypeMismatch {
-            err_message: format!("unsupported column names: {}", cols.join(", ")),
-            span,
-        });
-    }
-    Ok(item)
-}
-
-fn duration_from_value(v: Value) -> Result<std::time::Duration, ShellError> {
-    chrono::Duration::nanoseconds(v.as_duration()?)
-        .to_std()
-        .map_err(|_| ShellError::IOError {
-            msg: "negative duration not supported".to_string(),
-        })
-}
-
-#[cfg(test)]
-mod tests {
-    use chrono::DateTime;
-
-    use super::*;
-
-    #[test]
-    fn test_item_from_value_string() -> Result<(), ShellError> {
-        let item = item_from_value(Value::string("foo", Span::unknown()))?;
-        assert_eq!(
-            item,
-            HistoryItem {
-                command_line: "foo".to_string(),
-                id: None,
-                start_timestamp: None,
-                session_id: None,
-                hostname: None,
-                cwd: None,
-                duration: None,
-                exit_status: None,
-                more_info: None
-            }
-        );
-        Ok(())
-    }
-
-    #[test]
-    fn test_item_from_value_record() {
-        let span = Span::unknown();
-        let rec = new_record(&[
-            ("command", Value::string("foo", span)),
-            ("item_id", Value::int(1, span)),
-            (
-                "start_timestamp",
-                Value::date(
-                    DateTime::parse_from_rfc3339("1996-12-19T16:39:57-08:00").unwrap(),
-                    span,
-                ),
-            ),
-            ("hostname", Value::string("localhost", span)),
-            ("cwd", Value::string("/home/test", span)),
-            ("duration", Value::duration(100_000_000, span)),
-            ("exit_status", Value::int(42, span)),
-        ]);
-        let item = item_from_value(rec).unwrap();
-        assert_eq!(
-            item,
-            HistoryItem {
-                command_line: "foo".to_string(),
-                id: Some(HistoryItemId::new(1)),
-                start_timestamp: Some(
-                    DateTime::parse_from_rfc3339("1996-12-19T16:39:57-08:00")
-                        .unwrap()
-                        .to_utc()
-                ),
-                hostname: Some("localhost".to_string()),
-                cwd: Some("/home/test".to_string()),
-                duration: Some(std::time::Duration::from_nanos(100_000_000)),
-                exit_status: Some(42),
-
-                session_id: None,
-                more_info: None
-            }
-        );
-    }
-
-    #[test]
-    fn test_item_from_value_record_extra_field() {
-        let span = Span::unknown();
-        let rec = new_record(&[
-            ("command_line", Value::string("foo", span)),
-            ("id_nonexistent", Value::int(1, span)),
-        ]);
-        assert!(item_from_value(rec).is_err());
-    }
-
-    #[test]
-    fn test_item_from_value_record_bad_type() {
-        let span = Span::unknown();
-        let rec = new_record(&[
-            ("command_line", Value::string("foo", span)),
-            ("id", Value::string("one".to_string(), span)),
-        ]);
-        assert!(item_from_value(rec).is_err());
-    }
-
-    fn new_record(rec: &[(&'static str, Value)]) -> Value {
-        let span = Span::unknown();
-        let rec = Record::from_raw_cols_vals(
-            rec.iter().map(|(col, _)| col.to_string()).collect(),
-            rec.iter().map(|(_, val)| val.clone()).collect(),
-            span,
-            span,
-        )
-        .unwrap();
-        Value::record(rec, span)
-    }
-}

crates/nu-cli/src/commands/history/mod.rs

@@ -1,8 +1,5 @@
-mod fields;
 mod history_;
-mod history_import;
 mod history_session;
 
 pub use history_::History;
-pub use history_import::HistoryImport;
 pub use history_session::HistorySession;

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

@@ -7,7 +7,7 @@ mod keybindings_list;
 mod keybindings_listen;
 
 pub use commandline::{Commandline, CommandlineEdit, CommandlineGetCursor, CommandlineSetCursor};
-pub use history::{History, HistoryImport, HistorySession};
+pub use history::{History, HistorySession};
 pub use keybindings::Keybindings;
 pub use keybindings_default::KeybindingsDefault;
 pub use keybindings_list::KeybindingsList;

crates/nu-cli/tests/commands/history_import.rs

@@ -1,186 +0,0 @@
-use nu_test_support::{nu, Outcome};
-use reedline::{
-    FileBackedHistory, History, HistoryItem, HistoryItemId, ReedlineError, SearchQuery,
-    SqliteBackedHistory,
-};
-use tempfile::TempDir;
-
-struct Test {
-    cfg_dir: TempDir,
-}
-
-impl Test {
-    fn new(history_format: &'static str) -> Self {
-        let cfg_dir = tempfile::Builder::new()
-            .prefix("history_import_test")
-            .tempdir()
-            .unwrap();
-        // Assigning to $env.config.history.file_format seems to work only in startup
-        // configuration.
-        std::fs::write(
-            cfg_dir.path().join("env.nu"),
-            format!("$env.config.history.file_format = {history_format:?}"),
-        )
-        .unwrap();
-        Self { cfg_dir }
-    }
-
-    fn nu(&self, cmd: &'static str) -> Outcome {
-        let env = [(
-            "XDG_CONFIG_HOME".to_string(),
-            self.cfg_dir.path().to_str().unwrap().to_string(),
-        )];
-        let env_config = self.cfg_dir.path().join("env.nu");
-        nu!(envs: env, env_config: env_config, cmd)
-    }
-
-    fn open_plaintext(&self) -> Result<FileBackedHistory, ReedlineError> {
-        FileBackedHistory::with_file(100, self.cfg_dir.path().join("nushell").join("history.txt"))
-    }
-
-    fn open_sqlite(&self) -> Result<SqliteBackedHistory, ReedlineError> {
-        SqliteBackedHistory::with_file(
-            self.cfg_dir.path().join("nushell").join("history.sqlite3"),
-            None,
-            None,
-        )
-    }
-}
-
-fn query_all(history: impl History) -> Result<Vec<HistoryItem>, ReedlineError> {
-    history.search(SearchQuery::everything(
-        reedline::SearchDirection::Forward,
-        None,
-    ))
-}
-
-fn save_all(mut history: impl History, items: Vec<HistoryItem>) -> Result<(), ReedlineError> {
-    for item in items {
-        history.save(item)?;
-    }
-    Ok(())
-}
-
-const EMPTY_ITEM: HistoryItem = HistoryItem {
-    command_line: String::new(),
-    id: None,
-    start_timestamp: None,
-    session_id: None,
-    hostname: None,
-    cwd: None,
-    duration: None,
-    exit_status: None,
-    more_info: None,
-};
-
-#[test]
-fn history_import_pipe_string() {
-    let test = Test::new("plaintext");
-    let outcome = test.nu("echo bar | history import");
-
-    assert!(outcome.status.success());
-    assert_eq!(
-        query_all(test.open_plaintext().unwrap()).unwrap(),
-        vec![HistoryItem {
-            id: Some(HistoryItemId::new(0)),
-            command_line: "bar".to_string(),
-            ..EMPTY_ITEM
-        }]
-    );
-}
-
-#[test]
-fn history_import_pipe_record() {
-    let test = Test::new("sqlite");
-    let outcome = test.nu("[[item_id command]; [42 some_command]] | history import");
-
-    assert!(outcome.status.success());
-    assert_eq!(
-        query_all(test.open_sqlite().unwrap()).unwrap(),
-        vec![HistoryItem {
-            id: Some(HistoryItemId::new(42)),
-            command_line: "some_command".to_string(),
-            ..EMPTY_ITEM
-        }]
-    );
-}
-
-#[test]
-fn history_import_plain_to_sqlite() {
-    let test = Test::new("sqlite");
-    save_all(
-        test.open_plaintext().unwrap(),
-        vec![
-            HistoryItem {
-                id: Some(HistoryItemId::new(0)),
-                command_line: "foo".to_string(),
-                ..EMPTY_ITEM
-            },
-            HistoryItem {
-                id: Some(HistoryItemId::new(1)),
-                command_line: "bar".to_string(),
-                ..EMPTY_ITEM
-            },
-        ],
-    )
-    .unwrap();
-
-    let outcome = test.nu("history import");
-    assert!(outcome.status.success());
-    assert_eq!(
-        query_all(test.open_sqlite().unwrap()).unwrap(),
-        vec![
-            HistoryItem {
-                id: Some(HistoryItemId::new(0)),
-                command_line: "foo".to_string(),
-                ..EMPTY_ITEM
-            },
-            HistoryItem {
-                id: Some(HistoryItemId::new(1)),
-                command_line: "bar".to_string(),
-                ..EMPTY_ITEM
-            }
-        ]
-    );
-}
-
-#[test]
-fn history_import_sqlite_to_plain() {
-    let test = Test::new("plaintext");
-    save_all(
-        test.open_sqlite().unwrap(),
-        vec![
-            HistoryItem {
-                id: Some(HistoryItemId::new(0)),
-                command_line: "foo".to_string(),
-                hostname: Some("host".to_string()),
-                ..EMPTY_ITEM
-            },
-            HistoryItem {
-                id: Some(HistoryItemId::new(1)),
-                command_line: "bar".to_string(),
-                cwd: Some("/home/test".to_string()),
-                ..EMPTY_ITEM
-            },
-        ],
-    )
-    .unwrap();
-
-    let outcome = test.nu("history import");
-    assert!(outcome.status.success());
-    assert_eq!(
-        query_all(test.open_plaintext().unwrap()).unwrap(),
-        vec![
-            HistoryItem {
-                id: Some(HistoryItemId::new(0)),
-                command_line: "foo".to_string(),
-                ..EMPTY_ITEM
-            },
-            HistoryItem {
-                id: Some(HistoryItemId::new(1)),
-                command_line: "bar".to_string(),
-                ..EMPTY_ITEM
-            },
-        ]
-    );
-}

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

@@ -1,3 +1,2 @@
-mod history_import;
 mod keybindings_list;
 mod nu_highlight;

crates/nu-test-support/src/macros.rs

@@ -234,7 +234,7 @@ macro_rules! nu_with_plugins {
 }
 
 use crate::{Outcome, NATIVE_PATH_ENV_VAR};
-use nu_path::{AbsolutePath, AbsolutePathBuf, Path, PathBuf};
+use nu_path::{AbsolutePath, AbsolutePathBuf, Path};
 use std::{
     ffi::OsStr,
     process::{Command, Stdio},
@@ -248,10 +248,6 @@ pub struct NuOpts {
     pub envs: Option<Vec<(String, String)>>,
     pub collapse_output: Option<bool>,
     pub use_ir: Option<bool>,
-    // Note: At the time this was added, passing in a file path was more convenient. However,
-    // passing in file contents seems like a better API - consider this when adding new uses of
-    // this field.
-    pub env_config: Option<PathBuf>,
 }
 
 pub fn nu_run_test(opts: NuOpts, commands: impl AsRef<str>, with_std: bool) -> Outcome {
@@ -282,14 +278,8 @@ pub fn nu_run_test(opts: NuOpts, commands: impl AsRef<str>, with_std: bool) -> O
         command.envs(envs);
     }
 
-    match opts.env_config {
+    // Ensure that the user's config doesn't interfere with the tests
-        Some(path) => command.arg("--env-config").arg(path),
+    command.arg("--no-config-file");
-        // TODO: This seems unnecessary: the code that runs for integration tests
-        // (run_commands) loads startup configs only if they are specified via flags explicitly or
-        // the shell is started as logging shell (which it is not in this case).
-        None => command.arg("--no-config-file"),
-    };
-
     if !with_std {
         command.arg("--no-std-lib");
     }