Restructure nu-protocol in more meaningful units (#11917)

This is partially "feng-shui programming" of moving things to new
separate places.

The later commits include "`git blame` tollbooths" by moving out chunks
of code into new files, which requires an extra step to track things
with `git blame`. We can negiotiate if you want to keep particular
things in their original place.

If egregious I tried to add a bit of documentation. If I see something
that is unused/unnecessarily `pub` I will try to remove that.


- Move `nu_protocol::Exportable` to `nu-parser`
- Guess doccomment for `Exportable`
- Move `Unit` enum from `value` to `AST`
- Move engine state `Variable` def into its folder
- Move error-related files in `nu-protocol` subdir
- Move `pipeline_data` module into its own folder
- Move `stream.rs` over into the `pipeline_data` mod
- Move `PipelineMetadata` into its own file
- Doccomment `PipelineMetadata`
- Remove unused `is_leap_year` in `value/mod`
- Note about criminal `type_compatible` helper
- Move duration fmting into new `value/duration.rs`
- Move filesize fmting logic to new `value/filesize`
- Split reexports from standard imports in `value/mod`
- Doccomment trait `CustomValue`
- Polish doccomments and intradoc links

crates/nu-color-config/src/style_computer.rs

@@ -3,8 +3,9 @@ use crate::{color_record_to_nustyle, lookup_ansi_color_style, TextStyle};
 use nu_ansi_term::{Color, Style};
 use nu_engine::{env::get_config, eval_block};
 use nu_protocol::{
+    cli_error::CliError,
     engine::{EngineState, Stack, StateWorkingSet},
-    CliError, IntoPipelineData, Value,
+    IntoPipelineData, Value,
 };
 use std::collections::HashMap;
 

crates/nu-parser/src/exportable.rs

@@ -1,5 +1,6 @@
-use crate::{DeclId, ModuleId, VarId};
+use nu_protocol::{DeclId, ModuleId, VarId};
 
+/// Symbol that can be exported with its associated name and ID
 pub enum Exportable {
     Decl { name: Vec<u8>, id: DeclId },
     Module { name: Vec<u8>, id: ModuleId },

crates/nu-parser/src/lib.rs

@@ -1,4 +1,5 @@
 mod deparse;
+mod exportable;
 mod flatten;
 mod known_external;
 mod lex;

crates/nu-parser/src/parse_keywords.rs

@@ -1,4 +1,5 @@
 use crate::{
+    exportable::Exportable,
     parse_block,
     parser_path::ParserPath,
     type_check::{check_block_input_output, type_compatible},
@@ -13,7 +14,7 @@ use nu_protocol::{
     },
     engine::{StateWorkingSet, DEFAULT_OVERLAY_NAME},
     eval_const::eval_constant,
-    span, Alias, BlockId, DeclId, Exportable, Module, ModuleId, ParseError, PositionalArg,
+    span, Alias, BlockId, DeclId, Module, ModuleId, ParseError, PositionalArg,
     ResolvedImportPattern, Span, Spanned, SyntaxShape, Type, Value, VarId,
 };
 use std::collections::{HashMap, HashSet};

crates/nu-protocol/src/ast/expr.rs

@@ -5,7 +5,7 @@ use super::{
     Call, CellPath, Expression, ExternalArgument, FullCellPath, MatchPattern, Operator,
     RangeOperator,
 };
-use crate::{ast::ImportPattern, BlockId, Signature, Span, Spanned, Unit, VarId};
+use crate::{ast::ImportPattern, ast::Unit, BlockId, Signature, Span, Spanned, VarId};
 
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
 pub enum Expr {

crates/nu-protocol/src/ast/mod.rs

@@ -7,6 +7,7 @@ mod import_pattern;
 mod match_pattern;
 mod operator;
 mod pipeline;
+mod unit;
 
 pub use block::*;
 pub use call::*;
@@ -17,3 +18,4 @@ pub use import_pattern::*;
 pub use match_pattern::*;
 pub use operator::*;
 pub use pipeline::*;
+pub use unit::*;

crates/nu-protocol/src/engine/engine_state.rs

@@ -2,12 +2,14 @@ use fancy_regex::Regex;
 use lru::LruCache;
 
 use super::{usage::build_usage, usage::Usage, StateDelta};
-use super::{Command, EnvVars, OverlayFrame, ScopeFrame, Stack, Visibility, DEFAULT_OVERLAY_NAME};
+use super::{
+    Command, EnvVars, OverlayFrame, ScopeFrame, Stack, Variable, Visibility, DEFAULT_OVERLAY_NAME,
+};
 use crate::ast::Block;
 use crate::debugger::{Debugger, NoopDebugger};
 use crate::{
     BlockId, Config, DeclId, Example, FileId, HistoryConfig, Module, ModuleId, OverlayId,
-    ShellError, Signature, Span, Type, VarId, Variable, VirtualPathId,
+    ShellError, Signature, Span, Type, VarId, VirtualPathId,
 };
 use crate::{Category, Value};
 use std::borrow::Borrow;

crates/nu-protocol/src/engine/mod.rs

@@ -8,6 +8,7 @@ mod stack;
 mod state_delta;
 mod state_working_set;
 mod usage;
+mod variable;
 
 pub use call_info::*;
 pub use capture_block::*;
@@ -18,3 +19,4 @@ pub use pattern_match::*;
 pub use stack::*;
 pub use state_delta::*;
 pub use state_working_set::*;
+pub use variable::*;

crates/nu-protocol/src/engine/state_delta.rs

@@ -1,6 +1,6 @@
-use super::{usage::Usage, Command, EngineState, OverlayFrame, ScopeFrame, VirtualPath};
+use super::{usage::Usage, Command, EngineState, OverlayFrame, ScopeFrame, Variable, VirtualPath};
 use crate::ast::Block;
-use crate::{Module, Variable};
+use crate::Module;
 
 #[cfg(feature = "plugin")]
 use std::sync::Arc;

crates/nu-protocol/src/engine/state_working_set.rs

@@ -1,11 +1,9 @@
 use super::{
-    usage::build_usage, Command, EngineState, OverlayFrame, StateDelta, VirtualPath, Visibility,
+    usage::build_usage, Command, EngineState, OverlayFrame, StateDelta, Variable, VirtualPath,
-    PWD_ENV,
+    Visibility, PWD_ENV,
 };
 use crate::ast::Block;
-use crate::{
+use crate::{BlockId, Config, DeclId, FileId, Module, ModuleId, Span, Type, VarId, VirtualPathId};
-    BlockId, Config, DeclId, FileId, Module, ModuleId, Span, Type, VarId, Variable, VirtualPathId,
-};
 use crate::{Category, ParseError, ParseWarning, Value};
 use core::panic;
 use std::collections::{HashMap, HashSet};

crates/nu-protocol/src/errors/mod.rs

@@ -0,0 +1,9 @@
+pub mod cli_error;
+mod parse_error;
+mod parse_warning;
+mod shell_error;
+
+pub use cli_error::{format_error, report_error, report_error_new};
+pub use parse_error::{DidYouMean, ParseError};
+pub use parse_warning::ParseWarning;
+pub use shell_error::*;

crates/nu-protocol/src/lib.rs

@@ -1,51 +1,42 @@
 mod alias;
 pub mod ast;
-pub mod cli_error;
 pub mod config;
 pub mod debugger;
 mod did_you_mean;
 pub mod engine;
+mod errors;
 pub mod eval_base;
 pub mod eval_const;
 mod example;
-mod exportable;
 mod id;
 mod lev_distance;
 mod module;
-mod parse_error;
-mod parse_warning;
 mod pipeline_data;
 #[cfg(feature = "plugin")]
 mod plugin;
-mod shell_error;
 mod signature;
 pub mod span;
 mod syntax_shape;
 mod ty;
 pub mod util;
 mod value;
-mod variable;
 
 pub use alias::*;
-pub use cli_error::*;
+pub use ast::Unit;
 pub use config::*;
 pub use did_you_mean::did_you_mean;
 pub use engine::{ENV_VARIABLE_ID, IN_VARIABLE_ID, NU_VARIABLE_ID};
+pub use errors::*;
 pub use example::*;
-pub use exportable::*;
 pub use id::*;
 pub use lev_distance::levenshtein_distance;
 pub use module::*;
-pub use parse_error::{DidYouMean, ParseError};
-pub use parse_warning::ParseWarning;
 pub use pipeline_data::*;
 #[cfg(feature = "plugin")]
 pub use plugin::*;
-pub use shell_error::*;
 pub use signature::*;
 pub use span::*;
 pub use syntax_shape::*;
 pub use ty::*;
 pub use util::BufferedReader;
 pub use value::*;
-pub use variable::*;

crates/nu-protocol/src/pipeline_data/metadata.rs

@@ -0,0 +1,18 @@
+use std::path::PathBuf;
+
+/// Metadata that is valid for the whole [`PipelineData`](crate::PipelineData)
+#[derive(Debug, Clone)]
+pub struct PipelineMetadata {
+    pub data_source: DataSource,
+}
+
+/// Describes where the particular [`PipelineMetadata`] originates.
+///
+/// This can either be a particular family of commands (useful so downstream commands can adjust
+/// the presentation e.g. `Ls`) or the opened file to protect against overwrite-attempts properly.
+#[derive(Debug, Clone)]
+pub enum DataSource {
+    Ls,
+    HtmlThemes,
+    FilePath(PathBuf),
+}

crates/nu-protocol/src/pipeline_data/mod.rs

@@ -1,12 +1,18 @@
+mod metadata;
+mod stream;
+
+pub use metadata::*;
+pub use stream::*;
+
 use crate::{
     ast::{Call, PathMember},
     engine::{EngineState, Stack, StateWorkingSet},
-    format_error, Config, ListStream, RawStream, ShellError, Span, Value,
+    format_error, Config, ShellError, Span, Value,
 };
 use nu_utils::{stderr_write_all_and_flush, stdout_write_all_and_flush};
+use std::io::Write;
 use std::sync::{atomic::AtomicBool, Arc};
 use std::thread;
-use std::{io::Write, path::PathBuf};
 
 const LINE_ENDING_PATTERN: &[char] = &['\r', '\n'];
 
@@ -54,18 +60,6 @@ pub enum PipelineData {
     Empty,
 }
 
-#[derive(Debug, Clone)]
-pub struct PipelineMetadata {
-    pub data_source: DataSource,
-}
-
-#[derive(Debug, Clone)]
-pub enum DataSource {
-    Ls,
-    HtmlThemes,
-    FilePath(PathBuf),
-}
-
 impl PipelineData {
     pub fn new_with_metadata(metadata: Option<PipelineMetadata>, span: Span) -> PipelineData {
         PipelineData::Value(Value::nothing(span), metadata)

crates/nu-protocol/src/value/custom_value.rs

@@ -2,25 +2,30 @@ use std::{cmp::Ordering, fmt};
 
 use crate::{ast::Operator, ShellError, Span, Value};
 
-// Trait definition for a custom value
+/// Trait definition for a custom [`Value`](crate::Value) type
 #[typetag::serde(tag = "type")]
 pub trait CustomValue: fmt::Debug + Send + Sync {
+    /// Custom `Clone` implementation
+    ///
+    /// This can reemit a `Value::CustomValue(Self, span)` or materialize another representation
+    /// if necessary.
     fn clone_value(&self, span: Span) -> Value;
 
     //fn category(&self) -> Category;
 
-    // Define string representation of the custom value
+    /// Define string representation of the custom value
     fn value_string(&self) -> String;
 
-    // Converts the custom value to a base nushell value
+    /// Converts the custom value to a base nushell value.
-    // This is used to represent the custom value using the table representations
+    ///
-    // That already exist in nushell
+    /// This imposes the requirement that you can represent the custom value in some form using the
+    /// Value representations that already exist in nushell
     fn to_base_value(&self, span: Span) -> Result<Value, ShellError>;
 
-    // Any representation used to downcast object to its original type
+    /// Any representation used to downcast object to its original type
     fn as_any(&self) -> &dyn std::any::Any;
 
-    // Follow cell path functions
+    /// Follow cell path by numeric index (e.g. rows)
     fn follow_path_int(&self, _count: usize, span: Span) -> Result<Value, ShellError> {
         Err(ShellError::IncompatiblePathAccess {
             type_name: self.value_string(),
@@ -28,6 +33,7 @@ pub trait CustomValue: fmt::Debug + Send + Sync {
         })
     }
 
+    /// Follow cell path by string key (e.g. columns)
     fn follow_path_string(&self, _column_name: String, span: Span) -> Result<Value, ShellError> {
         Err(ShellError::IncompatiblePathAccess {
             type_name: self.value_string(),
@@ -35,14 +41,17 @@ pub trait CustomValue: fmt::Debug + Send + Sync {
         })
     }
 
-    // ordering with other value
+    /// ordering with other value (see [`std::cmp::PartialOrd`])
     fn partial_cmp(&self, _other: &Value) -> Option<Ordering> {
         None
     }
 
-    // Definition of an operation between the object that implements the trait
+    /// Definition of an operation between the object that implements the trait
-    // and another Value.
+    /// and another Value.
-    // The Operator enum is used to indicate the expected operation
+    ///
+    /// The Operator enum is used to indicate the expected operation.
+    ///
+    /// Default impl raises [`ShellError::UnsupportedOperator`].
     fn operation(
         &self,
         _lhs_span: Span,

crates/nu-protocol/src/value/duration.rs

@@ -0,0 +1,181 @@
+use chrono::Duration;
+use std::{
+    borrow::Cow,
+    fmt::{Display, Formatter},
+};
+
+#[derive(Clone, Copy)]
+pub enum TimePeriod {
+    Nanos(i64),
+    Micros(i64),
+    Millis(i64),
+    Seconds(i64),
+    Minutes(i64),
+    Hours(i64),
+    Days(i64),
+    Weeks(i64),
+    Months(i64),
+    Years(i64),
+}
+
+impl TimePeriod {
+    pub fn to_text(self) -> Cow<'static, str> {
+        match self {
+            Self::Nanos(n) => format!("{n} ns").into(),
+            Self::Micros(n) => format!("{n} µs").into(),
+            Self::Millis(n) => format!("{n} ms").into(),
+            Self::Seconds(n) => format!("{n} sec").into(),
+            Self::Minutes(n) => format!("{n} min").into(),
+            Self::Hours(n) => format!("{n} hr").into(),
+            Self::Days(n) => format!("{n} day").into(),
+            Self::Weeks(n) => format!("{n} wk").into(),
+            Self::Months(n) => format!("{n} month").into(),
+            Self::Years(n) => format!("{n} yr").into(),
+        }
+    }
+}
+
+impl Display for TimePeriod {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.to_text())
+    }
+}
+
+pub fn format_duration(duration: i64) -> String {
+    let (sign, periods) = format_duration_as_timeperiod(duration);
+
+    let text = periods
+        .into_iter()
+        .map(|p| p.to_text().to_string().replace(' ', ""))
+        .collect::<Vec<String>>();
+
+    format!(
+        "{}{}",
+        if sign == -1 { "-" } else { "" },
+        text.join(" ").trim()
+    )
+}
+
+pub fn format_duration_as_timeperiod(duration: i64) -> (i32, Vec<TimePeriod>) {
+    // Attribution: most of this is taken from chrono-humanize-rs. Thanks!
+    // https://gitlab.com/imp/chrono-humanize-rs/-/blob/master/src/humantime.rs
+    // Current duration doesn't know a date it's based on, weeks is the max time unit it can normalize into.
+    // Don't guess or estimate how many years or months it might contain.
+
+    let (sign, duration) = if duration >= 0 {
+        (1, duration)
+    } else {
+        (-1, -duration)
+    };
+
+    let dur = Duration::nanoseconds(duration);
+
+    /// Split this a duration into number of whole weeks and the remainder
+    fn split_weeks(duration: Duration) -> (Option<i64>, Duration) {
+        let weeks = duration.num_weeks();
+        normalize_split(weeks, Duration::try_weeks(weeks), duration)
+    }
+
+    /// Split this a duration into number of whole days and the remainder
+    fn split_days(duration: Duration) -> (Option<i64>, Duration) {
+        let days = duration.num_days();
+        normalize_split(days, Duration::try_days(days), duration)
+    }
+
+    /// Split this a duration into number of whole hours and the remainder
+    fn split_hours(duration: Duration) -> (Option<i64>, Duration) {
+        let hours = duration.num_hours();
+        normalize_split(hours, Duration::try_hours(hours), duration)
+    }
+
+    /// Split this a duration into number of whole minutes and the remainder
+    fn split_minutes(duration: Duration) -> (Option<i64>, Duration) {
+        let minutes = duration.num_minutes();
+        normalize_split(minutes, Duration::try_minutes(minutes), duration)
+    }
+
+    /// Split this a duration into number of whole seconds and the remainder
+    fn split_seconds(duration: Duration) -> (Option<i64>, Duration) {
+        let seconds = duration.num_seconds();
+        normalize_split(seconds, Duration::try_seconds(seconds), duration)
+    }
+
+    /// Split this a duration into number of whole milliseconds and the remainder
+    fn split_milliseconds(duration: Duration) -> (Option<i64>, Duration) {
+        let millis = duration.num_milliseconds();
+        normalize_split(millis, Duration::try_milliseconds(millis), duration)
+    }
+
+    /// Split this a duration into number of whole seconds and the remainder
+    fn split_microseconds(duration: Duration) -> (Option<i64>, Duration) {
+        let micros = duration.num_microseconds().unwrap_or_default();
+        normalize_split(micros, Duration::microseconds(micros), duration)
+    }
+
+    /// Split this a duration into number of whole seconds and the remainder
+    fn split_nanoseconds(duration: Duration) -> (Option<i64>, Duration) {
+        let nanos = duration.num_nanoseconds().unwrap_or_default();
+        normalize_split(nanos, Duration::nanoseconds(nanos), duration)
+    }
+
+    fn normalize_split(
+        wholes: i64,
+        wholes_duration: impl Into<Option<Duration>>,
+        total_duration: Duration,
+    ) -> (Option<i64>, Duration) {
+        match wholes_duration.into() {
+            Some(wholes_duration) if wholes != 0 => {
+                (Some(wholes), total_duration - wholes_duration)
+            }
+            _ => (None, total_duration),
+        }
+    }
+
+    let mut periods = vec![];
+
+    let (weeks, remainder) = split_weeks(dur);
+    if let Some(weeks) = weeks {
+        periods.push(TimePeriod::Weeks(weeks));
+    }
+
+    let (days, remainder) = split_days(remainder);
+    if let Some(days) = days {
+        periods.push(TimePeriod::Days(days));
+    }
+
+    let (hours, remainder) = split_hours(remainder);
+    if let Some(hours) = hours {
+        periods.push(TimePeriod::Hours(hours));
+    }
+
+    let (minutes, remainder) = split_minutes(remainder);
+    if let Some(minutes) = minutes {
+        periods.push(TimePeriod::Minutes(minutes));
+    }
+
+    let (seconds, remainder) = split_seconds(remainder);
+    if let Some(seconds) = seconds {
+        periods.push(TimePeriod::Seconds(seconds));
+    }
+
+    let (millis, remainder) = split_milliseconds(remainder);
+    if let Some(millis) = millis {
+        periods.push(TimePeriod::Millis(millis));
+    }
+
+    let (micros, remainder) = split_microseconds(remainder);
+    if let Some(micros) = micros {
+        periods.push(TimePeriod::Micros(micros));
+    }
+
+    let (nanos, _remainder) = split_nanoseconds(remainder);
+    if let Some(nanos) = nanos {
+        periods.push(TimePeriod::Nanos(nanos));
+    }
+
+    if periods.is_empty() {
+        periods.push(TimePeriod::Seconds(0));
+    }
+
+    (sign, periods)
+}

crates/nu-protocol/src/value/filesize.rs

@@ -0,0 +1,116 @@
+use crate::Config;
+use byte_unit::UnitType;
+use nu_utils::get_system_locale;
+use num_format::ToFormattedString;
+
+pub fn format_filesize_from_conf(num_bytes: i64, config: &Config) -> String {
+    // We need to take into account config.filesize_metric so, if someone asks for KB
+    // and filesize_metric is false, return KiB
+    format_filesize(
+        num_bytes,
+        config.filesize_format.as_str(),
+        Some(config.filesize_metric),
+    )
+}
+
+// filesize_metric is explicit when printed a value according to user config;
+// other places (such as `format filesize`) don't.
+pub fn format_filesize(
+    num_bytes: i64,
+    format_value: &str,
+    filesize_metric: Option<bool>,
+) -> String {
+    // Allow the user to specify how they want their numbers formatted
+
+    // When format_value is "auto" or an invalid value, the returned ByteUnit doesn't matter
+    // and is always B.
+    let filesize_unit = get_filesize_format(format_value, filesize_metric);
+    let byte = byte_unit::Byte::from_u64(num_bytes.unsigned_abs());
+    let adj_byte = if let Some(unit) = filesize_unit {
+        byte.get_adjusted_unit(unit)
+    } else {
+        // When filesize_metric is None, format_value should never be "auto", so this
+        // unwrap_or() should always work.
+        byte.get_appropriate_unit(if filesize_metric.unwrap_or(false) {
+            UnitType::Decimal
+        } else {
+            UnitType::Binary
+        })
+    };
+
+    match adj_byte.get_unit() {
+        byte_unit::Unit::B => {
+            let locale = get_system_locale();
+            let locale_byte = adj_byte.get_value() as u64;
+            let locale_byte_string = locale_byte.to_formatted_string(&locale);
+            let locale_signed_byte_string = if num_bytes.is_negative() {
+                format!("-{locale_byte_string}")
+            } else {
+                locale_byte_string
+            };
+
+            if filesize_unit.is_none() {
+                format!("{locale_signed_byte_string} B")
+            } else {
+                locale_signed_byte_string
+            }
+        }
+        _ => {
+            if num_bytes.is_negative() {
+                format!("-{:.1}", adj_byte)
+            } else {
+                format!("{:.1}", adj_byte)
+            }
+        }
+    }
+}
+
+/// Get the filesize unit, or None if format is "auto"
+fn get_filesize_format(
+    format_value: &str,
+    filesize_metric: Option<bool>,
+) -> Option<byte_unit::Unit> {
+    // filesize_metric always overrides the unit of filesize_format.
+    let metric = filesize_metric.unwrap_or(!format_value.ends_with("ib"));
+    macro_rules! either {
+        ($metric:ident, $binary:ident) => {
+            Some(if metric {
+                byte_unit::Unit::$metric
+            } else {
+                byte_unit::Unit::$binary
+            })
+        };
+    }
+    match format_value {
+        "b" => Some(byte_unit::Unit::B),
+        "kb" | "kib" => either!(KB, KiB),
+        "mb" | "mib" => either!(MB, MiB),
+        "gb" | "gib" => either!(GB, GiB),
+        "tb" | "tib" => either!(TB, TiB),
+        "pb" | "pib" => either!(TB, TiB),
+        "eb" | "eib" => either!(EB, EiB),
+        _ => None,
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use rstest::rstest;
+
+    #[rstest]
+    #[case(1000, Some(true), "auto", "1.0 KB")]
+    #[case(1000, Some(false), "auto", "1,000 B")]
+    #[case(1000, Some(false), "kb", "1.0 KiB")]
+    #[case(3000, Some(false), "auto", "2.9 KiB")]
+    #[case(3_000_000, None, "auto", "2.9 MiB")]
+    #[case(3_000_000, None, "kib", "2929.7 KiB")]
+    fn test_filesize(
+        #[case] val: i64,
+        #[case] filesize_metric: Option<bool>,
+        #[case] filesize_format: String,
+        #[case] exp: &str,
+    ) {
+        assert_eq!(exp, format_filesize(val, &filesize_format, filesize_metric));
+    }
+}

crates/nu-protocol/src/value/mod.rs

@@ -1,44 +1,40 @@
 mod custom_value;
+mod duration;
+mod filesize;
 mod from;
 mod from_value;
 mod glob;
 mod lazy_record;
 mod range;
-mod stream;
-mod unit;
 
 pub mod record;
+pub use custom_value::CustomValue;
+pub use duration::*;
+pub use filesize::*;
+pub use from_value::FromValue;
+pub use glob::*;
+pub use lazy_record::LazyRecord;
+pub use range::*;
+pub use record::Record;
 
 use crate::ast::{Bits, Boolean, CellPath, Comparison, Math, Operator, PathMember, RangeInclusion};
 use crate::engine::{Closure, EngineState};
 use crate::{did_you_mean, BlockId, Config, ShellError, Span, Type};
 
-use byte_unit::UnitType;
+use chrono::{DateTime, Datelike, FixedOffset, Locale, TimeZone};
-use chrono::{DateTime, Datelike, Duration, FixedOffset, Locale, TimeZone};
 use chrono_humanize::HumanTime;
-pub use custom_value::CustomValue;
 use fancy_regex::Regex;
-pub use from_value::FromValue;
-pub use glob::*;
-pub use lazy_record::LazyRecord;
 use nu_utils::locale::LOCALE_OVERRIDE_ENV_VAR;
-use nu_utils::{
+use nu_utils::{contains_emoji, locale::get_system_locale_string, IgnoreCaseExt};
-    contains_emoji, get_system_locale, locale::get_system_locale_string, IgnoreCaseExt,
-};
-use num_format::ToFormattedString;
-pub use range::*;
-pub use record::Record;
 use serde::{Deserialize, Serialize};
 use std::fmt::Write;
 
 use std::{
     borrow::Cow,
-    fmt::{Display, Formatter, Result as FmtResult},
+    fmt::Display,
     path::PathBuf,
     {cmp::Ordering, fmt::Debug},
 };
-pub use stream::*;
-pub use unit::*;
 
 /// Core structured values that pass through the pipeline in Nushell.
 // NOTE: Please do not reorder these enum cases without thinking through the
@@ -3696,6 +3692,8 @@ fn reorder_record_inner(record: &Record) -> (Vec<&String>, Vec<&Value>) {
     kv_pairs.into_iter().unzip()
 }
 
+// TODO: The name of this function is overly broad with partial compatibility
+// Should be replaced by an explicitly named helper on `Type` (take `Any` into account)
 fn type_compatible(a: Type, b: Type) -> bool {
     if a == b {
         return true;
@@ -3704,278 +3702,6 @@ fn type_compatible(a: Type, b: Type) -> bool {
     matches!((a, b), (Type::Int, Type::Float) | (Type::Float, Type::Int))
 }
 
-/// Is the given year a leap year?
-#[allow(clippy::nonminimal_bool)]
-pub fn is_leap_year(year: i32) -> bool {
-    (year % 4 == 0) && (year % 100 != 0 || (year % 100 == 0 && year % 400 == 0))
-}
-
-#[derive(Clone, Copy)]
-pub enum TimePeriod {
-    Nanos(i64),
-    Micros(i64),
-    Millis(i64),
-    Seconds(i64),
-    Minutes(i64),
-    Hours(i64),
-    Days(i64),
-    Weeks(i64),
-    Months(i64),
-    Years(i64),
-}
-
-impl TimePeriod {
-    pub fn to_text(self) -> Cow<'static, str> {
-        match self {
-            Self::Nanos(n) => format!("{n} ns").into(),
-            Self::Micros(n) => format!("{n} µs").into(),
-            Self::Millis(n) => format!("{n} ms").into(),
-            Self::Seconds(n) => format!("{n} sec").into(),
-            Self::Minutes(n) => format!("{n} min").into(),
-            Self::Hours(n) => format!("{n} hr").into(),
-            Self::Days(n) => format!("{n} day").into(),
-            Self::Weeks(n) => format!("{n} wk").into(),
-            Self::Months(n) => format!("{n} month").into(),
-            Self::Years(n) => format!("{n} yr").into(),
-        }
-    }
-}
-
-impl Display for TimePeriod {
-    fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult {
-        write!(f, "{}", self.to_text())
-    }
-}
-
-pub fn format_duration(duration: i64) -> String {
-    let (sign, periods) = format_duration_as_timeperiod(duration);
-
-    let text = periods
-        .into_iter()
-        .map(|p| p.to_text().to_string().replace(' ', ""))
-        .collect::<Vec<String>>();
-
-    format!(
-        "{}{}",
-        if sign == -1 { "-" } else { "" },
-        text.join(" ").trim()
-    )
-}
-
-pub fn format_duration_as_timeperiod(duration: i64) -> (i32, Vec<TimePeriod>) {
-    // Attribution: most of this is taken from chrono-humanize-rs. Thanks!
-    // https://gitlab.com/imp/chrono-humanize-rs/-/blob/master/src/humantime.rs
-    // Current duration doesn't know a date it's based on, weeks is the max time unit it can normalize into.
-    // Don't guess or estimate how many years or months it might contain.
-
-    let (sign, duration) = if duration >= 0 {
-        (1, duration)
-    } else {
-        (-1, -duration)
-    };
-
-    let dur = Duration::nanoseconds(duration);
-
-    /// Split this a duration into number of whole weeks and the remainder
-    fn split_weeks(duration: Duration) -> (Option<i64>, Duration) {
-        let weeks = duration.num_weeks();
-        normalize_split(weeks, Duration::try_weeks(weeks), duration)
-    }
-
-    /// Split this a duration into number of whole days and the remainder
-    fn split_days(duration: Duration) -> (Option<i64>, Duration) {
-        let days = duration.num_days();
-        normalize_split(days, Duration::try_days(days), duration)
-    }
-
-    /// Split this a duration into number of whole hours and the remainder
-    fn split_hours(duration: Duration) -> (Option<i64>, Duration) {
-        let hours = duration.num_hours();
-        normalize_split(hours, Duration::try_hours(hours), duration)
-    }
-
-    /// Split this a duration into number of whole minutes and the remainder
-    fn split_minutes(duration: Duration) -> (Option<i64>, Duration) {
-        let minutes = duration.num_minutes();
-        normalize_split(minutes, Duration::try_minutes(minutes), duration)
-    }
-
-    /// Split this a duration into number of whole seconds and the remainder
-    fn split_seconds(duration: Duration) -> (Option<i64>, Duration) {
-        let seconds = duration.num_seconds();
-        normalize_split(seconds, Duration::try_seconds(seconds), duration)
-    }
-
-    /// Split this a duration into number of whole milliseconds and the remainder
-    fn split_milliseconds(duration: Duration) -> (Option<i64>, Duration) {
-        let millis = duration.num_milliseconds();
-        normalize_split(millis, Duration::try_milliseconds(millis), duration)
-    }
-
-    /// Split this a duration into number of whole seconds and the remainder
-    fn split_microseconds(duration: Duration) -> (Option<i64>, Duration) {
-        let micros = duration.num_microseconds().unwrap_or_default();
-        normalize_split(micros, Duration::microseconds(micros), duration)
-    }
-
-    /// Split this a duration into number of whole seconds and the remainder
-    fn split_nanoseconds(duration: Duration) -> (Option<i64>, Duration) {
-        let nanos = duration.num_nanoseconds().unwrap_or_default();
-        normalize_split(nanos, Duration::nanoseconds(nanos), duration)
-    }
-
-    fn normalize_split(
-        wholes: i64,
-        wholes_duration: impl Into<Option<Duration>>,
-        total_duration: Duration,
-    ) -> (Option<i64>, Duration) {
-        match wholes_duration.into() {
-            Some(wholes_duration) if wholes != 0 => {
-                (Some(wholes), total_duration - wholes_duration)
-            }
-            _ => (None, total_duration),
-        }
-    }
-
-    let mut periods = vec![];
-
-    let (weeks, remainder) = split_weeks(dur);
-    if let Some(weeks) = weeks {
-        periods.push(TimePeriod::Weeks(weeks));
-    }
-
-    let (days, remainder) = split_days(remainder);
-    if let Some(days) = days {
-        periods.push(TimePeriod::Days(days));
-    }
-
-    let (hours, remainder) = split_hours(remainder);
-    if let Some(hours) = hours {
-        periods.push(TimePeriod::Hours(hours));
-    }
-
-    let (minutes, remainder) = split_minutes(remainder);
-    if let Some(minutes) = minutes {
-        periods.push(TimePeriod::Minutes(minutes));
-    }
-
-    let (seconds, remainder) = split_seconds(remainder);
-    if let Some(seconds) = seconds {
-        periods.push(TimePeriod::Seconds(seconds));
-    }
-
-    let (millis, remainder) = split_milliseconds(remainder);
-    if let Some(millis) = millis {
-        periods.push(TimePeriod::Millis(millis));
-    }
-
-    let (micros, remainder) = split_microseconds(remainder);
-    if let Some(micros) = micros {
-        periods.push(TimePeriod::Micros(micros));
-    }
-
-    let (nanos, _remainder) = split_nanoseconds(remainder);
-    if let Some(nanos) = nanos {
-        periods.push(TimePeriod::Nanos(nanos));
-    }
-
-    if periods.is_empty() {
-        periods.push(TimePeriod::Seconds(0));
-    }
-
-    (sign, periods)
-}
-
-pub fn format_filesize_from_conf(num_bytes: i64, config: &Config) -> String {
-    // We need to take into account config.filesize_metric so, if someone asks for KB
-    // and filesize_metric is false, return KiB
-    format_filesize(
-        num_bytes,
-        config.filesize_format.as_str(),
-        Some(config.filesize_metric),
-    )
-}
-
-// filesize_metric is explicit when printed a value according to user config;
-// other places (such as `format filesize`) don't.
-pub fn format_filesize(
-    num_bytes: i64,
-    format_value: &str,
-    filesize_metric: Option<bool>,
-) -> String {
-    // Allow the user to specify how they want their numbers formatted
-
-    // When format_value is "auto" or an invalid value, the returned ByteUnit doesn't matter
-    // and is always B.
-    let filesize_unit = get_filesize_format(format_value, filesize_metric);
-    let byte = byte_unit::Byte::from_u64(num_bytes.unsigned_abs());
-    let adj_byte = if let Some(unit) = filesize_unit {
-        byte.get_adjusted_unit(unit)
-    } else {
-        // When filesize_metric is None, format_value should never be "auto", so this
-        // unwrap_or() should always work.
-        byte.get_appropriate_unit(if filesize_metric.unwrap_or(false) {
-            UnitType::Decimal
-        } else {
-            UnitType::Binary
-        })
-    };
-
-    match adj_byte.get_unit() {
-        byte_unit::Unit::B => {
-            let locale = get_system_locale();
-            let locale_byte = adj_byte.get_value() as u64;
-            let locale_byte_string = locale_byte.to_formatted_string(&locale);
-            let locale_signed_byte_string = if num_bytes.is_negative() {
-                format!("-{locale_byte_string}")
-            } else {
-                locale_byte_string
-            };
-
-            if filesize_unit.is_none() {
-                format!("{locale_signed_byte_string} B")
-            } else {
-                locale_signed_byte_string
-            }
-        }
-        _ => {
-            if num_bytes.is_negative() {
-                format!("-{:.1}", adj_byte)
-            } else {
-                format!("{:.1}", adj_byte)
-            }
-        }
-    }
-}
-
-/// Get the filesize unit, or None if format is "auto"
-fn get_filesize_format(
-    format_value: &str,
-    filesize_metric: Option<bool>,
-) -> Option<byte_unit::Unit> {
-    // filesize_metric always overrides the unit of filesize_format.
-    let metric = filesize_metric.unwrap_or(!format_value.ends_with("ib"));
-    macro_rules! either {
-        ($metric:ident, $binary:ident) => {
-            Some(if metric {
-                byte_unit::Unit::$metric
-            } else {
-                byte_unit::Unit::$binary
-            })
-        };
-    }
-    match format_value {
-        "b" => Some(byte_unit::Unit::B),
-        "kb" | "kib" => either!(KB, KiB),
-        "mb" | "mib" => either!(MB, MiB),
-        "gb" | "gib" => either!(GB, GiB),
-        "tb" | "tib" => either!(TB, TiB),
-        "pb" | "pib" => either!(TB, TiB),
-        "eb" | "eib" => either!(EB, EiB),
-        _ => None,
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::{Record, Value};
@@ -4055,10 +3781,8 @@ mod tests {
 
     mod into_string {
         use chrono::{DateTime, FixedOffset};
-        use rstest::rstest;
 
         use super::*;
-        use crate::format_filesize;
 
         #[test]
         fn test_datetime() {
@@ -4087,21 +3811,5 @@ mod tests {
             let formatted = string.split(' ').next().unwrap();
             assert_eq!("-0316-02-11T06:13:20+00:00", formatted);
         }
-
-        #[rstest]
-        #[case(1000, Some(true), "auto", "1.0 KB")]
-        #[case(1000, Some(false), "auto", "1,000 B")]
-        #[case(1000, Some(false), "kb", "1.0 KiB")]
-        #[case(3000, Some(false), "auto", "2.9 KiB")]
-        #[case(3_000_000, None, "auto", "2.9 MiB")]
-        #[case(3_000_000, None, "kib", "2929.7 KiB")]
-        fn test_filesize(
-            #[case] val: i64,
-            #[case] filesize_metric: Option<bool>,
-            #[case] filesize_format: String,
-            #[case] exp: &str,
-        ) {
-            assert_eq!(exp, format_filesize(val, &filesize_format, filesize_metric));
-        }
     }
 }

src/test_bins.rs

@@ -3,7 +3,7 @@ use nu_engine::eval_block;
 use nu_parser::parse;
 use nu_protocol::debugger::WithoutDebug;
 use nu_protocol::engine::{EngineState, Stack, StateWorkingSet};
-use nu_protocol::{CliError, PipelineData, Value};
+use nu_protocol::{cli_error::CliError, PipelineData, Value};
 use nu_std::load_standard_library;
 use std::io::{self, BufRead, Read, Write};