Remove feat extra and include in default (#12140)

# Description The intended effect of the `extra` feature has been undermined by introducing the full builds on our release pages and having more activity on some of the extra commands. To simplify the feature matrix let's get rid of it and focus our effort on truly either refining a command to well-specified behavior or discarding it entirely from the `nu` binary and moving it into plugins. ## Details - Remove `--features extra` from CI - Don't explicitly name `extra` in full build wf - Remove feature extra from build-help scripts - Update README in `nu-cmd-extra` - Remove feature `extra` - Fix previously dead `format pattern` tests - Relax signature of `to html` - Fix/ignore `html::test_no_color_flag` - Remove dead features from `version` - Refine `to html` type signature # User-Facing Changes The commands that were previously only available when building with `--features extra` will now be available to everyone. This increases the number of dependencies slightly but has a limited impact on the overall binary size. # Tests + Formatting Some tests that were left in `nu-command` during cratification were dead because the feature was not passed to `nu-command` and only to `nu-cmd-lang` for feature-flag mention in `version`. Those tests have now been either fixed or ignored in one case. # After Submitting There may be places in the documentation where we point to `--features extra` that will now be moot (apart from the generated command help)
2024-11-21 16:03:19 +01:00 · 2024-03-10 17:29:02 +01:00 · 2024-03-10 17:29:02 +01:00 · 067ceedf79
commit 067ceedf79
parent a7b281292d
23 changed files with 49 additions and 93 deletions
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@ -21,14 +21,12 @@ jobs:
        # builds to link against a too-new-for-many-Linux-installs glibc version. Consider
        # revisiting this when 20.04 is closer to EOL (April 2025)
        platform: [windows-latest, macos-latest, ubuntu-20.04]
-        feature: [default, dataframe, extra]
+        feature: [default, dataframe]
        include:
          - feature: default
            flags: ""
          - feature: dataframe
            flags: "--features=dataframe"
-          - feature: extra
-            flags: "--features=extra"
        exclude:
          - platform: windows-latest
            feature: dataframe
@ -61,7 +59,7 @@ jobs:
      fail-fast: true
      matrix:
        platform: [windows-latest, macos-latest, ubuntu-20.04]
-        feature: [default, dataframe, extra]
+        feature: [default, dataframe]
        include:
          # linux CI cannot handle clipboard feature
          - default-flags: ""
@ -71,17 +69,11 @@ jobs:
            flags: ""
          - feature: dataframe
            flags: "--features=dataframe"
-          - feature: extra
-            flags: "--features=extra"
        exclude:
          - platform: windows-latest
            feature: dataframe
          - platform: macos-latest
            feature: dataframe
-          - platform: windows-latest
-            feature: extra
-          - platform: macos-latest
-            feature: extra

    runs-on: ${{ matrix.platform }}

--- a/.github/workflows/nightly-build.yml
+++ b/.github/workflows/nightly-build.yml
@ -202,35 +202,35 @@ jobs:
        include:
        - target: aarch64-apple-darwin
          os: macos-latest
-          target_rustflags: '--features=dataframe,extra'
+          target_rustflags: '--features=dataframe'
        - target: x86_64-apple-darwin
          os: macos-latest
-          target_rustflags: '--features=dataframe,extra'
+          target_rustflags: '--features=dataframe'
        - target: x86_64-pc-windows-msvc
          extra: 'bin'
          os: windows-latest
-          target_rustflags: '--features=dataframe,extra'
+          target_rustflags: '--features=dataframe'
        - target: x86_64-pc-windows-msvc
          extra: msi
          os: windows-latest
-          target_rustflags: '--features=dataframe,extra'
+          target_rustflags: '--features=dataframe'
        - target: aarch64-pc-windows-msvc
          extra: 'bin'
          os: windows-latest
-          target_rustflags: '--features=dataframe,extra'
+          target_rustflags: '--features=dataframe'
        - target: aarch64-pc-windows-msvc
          extra: msi
          os: windows-latest
-          target_rustflags: '--features=dataframe,extra'
+          target_rustflags: '--features=dataframe'
        - target: x86_64-unknown-linux-gnu
          os: ubuntu-20.04
-          target_rustflags: '--features=dataframe,extra'
+          target_rustflags: '--features=dataframe'
        - target: x86_64-unknown-linux-musl
          os: ubuntu-20.04
-          target_rustflags: '--features=dataframe,extra'
+          target_rustflags: '--features=dataframe'
        - target: aarch64-unknown-linux-gnu
          os: ubuntu-20.04
-          target_rustflags: '--features=dataframe,extra'
+          target_rustflags: '--features=dataframe'

    runs-on: ${{matrix.os}}

--- a/.github/workflows/release.yml
+++ b/.github/workflows/release.yml
@ -128,35 +128,35 @@ jobs:
        include:
        - target: aarch64-apple-darwin
          os: macos-latest
-          target_rustflags: '--features=dataframe,extra'
+          target_rustflags: '--features=dataframe'
        - target: x86_64-apple-darwin
          os: macos-latest
-          target_rustflags: '--features=dataframe,extra'
+          target_rustflags: '--features=dataframe'
        - target: x86_64-pc-windows-msvc
          extra: 'bin'
          os: windows-latest
-          target_rustflags: '--features=dataframe,extra'
+          target_rustflags: '--features=dataframe'
        - target: x86_64-pc-windows-msvc
          extra: msi
          os: windows-latest
-          target_rustflags: '--features=dataframe,extra'
+          target_rustflags: '--features=dataframe'
        - target: aarch64-pc-windows-msvc
          extra: 'bin'
          os: windows-latest
-          target_rustflags: '--features=dataframe,extra'
+          target_rustflags: '--features=dataframe'
        - target: aarch64-pc-windows-msvc
          extra: msi
          os: windows-latest
-          target_rustflags: '--features=dataframe,extra'
+          target_rustflags: '--features=dataframe'
        - target: x86_64-unknown-linux-gnu
          os: ubuntu-20.04
-          target_rustflags: '--features=dataframe,extra'
+          target_rustflags: '--features=dataframe'
        - target: x86_64-unknown-linux-musl
          os: ubuntu-20.04
-          target_rustflags: '--features=dataframe,extra'
+          target_rustflags: '--features=dataframe'
        - target: aarch64-unknown-linux-gnu
          os: ubuntu-20.04
-          target_rustflags: '--features=dataframe,extra'
+          target_rustflags: '--features=dataframe'

    runs-on: ${{matrix.os}}

--- a/Cargo.toml
+++ b/Cargo.toml
@ -82,7 +82,7 @@ nu-cmd-lang = { path = "./crates/nu-cmd-lang", version = "0.91.1" }
 nu-cmd-dataframe = { path = "./crates/nu-cmd-dataframe", version = "0.91.1", features = [
  "dataframe",
 ], optional = true }
-nu-cmd-extra = { path = "./crates/nu-cmd-extra", version = "0.91.1", optional = true }
+nu-cmd-extra = { path = "./crates/nu-cmd-extra", version = "0.91.1" }
 nu-command = { path = "./crates/nu-command", version = "0.91.1" }
 nu-engine = { path = "./crates/nu-engine", version = "0.91.1" }
 nu-explore = { path = "./crates/nu-explore", version = "0.91.1" }
@ -163,9 +163,6 @@ system-clipboard = ["reedline/system_clipboard"]
 which-support = ["nu-command/which-support", "nu-cmd-lang/which-support"]
 trash-support = ["nu-command/trash-support", "nu-cmd-lang/trash-support"]

-# Extra feature for nushell
-extra = ["dep:nu-cmd-extra", "nu-cmd-lang/extra"]
-
 # Dataframe feature for nushell
 dataframe = ["dep:nu-cmd-dataframe", "nu-cmd-lang/dataframe"]

--- a/crates/nu-cmd-extra/README.md
+++ b/crates/nu-cmd-extra/README.md
@ -1,30 +1,13 @@
 # nu-cmd-extra

-## the extra commands are not part of the Nushell binary
+The commands in this crate are the *extra commands* of Nushell.  These commands
+are not in a state to be guaranteed to be part of the 1.0 API; meaning that
+there is no guarantee longer term that these commands will be around into the
+future.

-The commands in this crate are the *extra commands* of Nushell.  They do not
-get built for the release and it is the responsibility of the developer to
-build these commands if they want to use them.
-
-These commands are not going to part of the 1.0 Api; meaning that there
-is no guarantee longer term that these commands will be around into the future.
-Of course since they are part of the source tree one could always incorporate
-them into their own custom release.
-
-### How to build the commands in this crate
-
-Step 1 is to
-[read the installation notes](https://www.nushell.sh/book/installation.html#build-from-source)
-for Nushell which is located in our Nushell book.
-
-Once Rust is installed you can then build Nushell with the following command.
-
-```rust
-cargo build --features=extra
-```
-
-Your Nushell binary which just got built is called *nu* and will be located here.
-
-```
-nushell/target/debug/nu
-```
+For a while we did exclude them behind the `--features extra` compile time
+flag, meaning that the default release did not contain them. As we (the Nushell
+team) shipped a full build including both `extra` and `dataframe` for some
+time, we chose to sunset the `extra` feature but keep the commands in this
+crate for now. In the future the commands may be moved to more topical crates
+or discarded into plugins.
--- a/crates/nu-cmd-extra/src/extra/formats/to/html.rs
+++ b/crates/nu-cmd-extra/src/extra/formats/to/html.rs
@ -91,7 +91,8 @@ impl Command for ToHtml {

    fn signature(&self) -> Signature {
        Signature::build("to html")
-            .input_output_types(vec![(Type::Any, Type::String)])
+            .input_output_types(vec![(Type::Nothing, Type::Any), (Type::Any, Type::String)])
+            .allow_variants_without_examples(true)
            .switch("html-color", "change ansi colors to html colors", Some('c'))
            .switch("no-color", "remove all ansi colors in output", Some('n'))
            .switch(
--- a/crates/nu-cmd-extra/tests/commands/mod.rs
+++ b/crates/nu-cmd-extra/tests/commands/mod.rs
@ -1,2 +1 @@
-#[cfg(feature = "extra")]
 mod bytes;
--- a/crates/nu-cmd-lang/Cargo.toml
+++ b/crates/nu-cmd-lang/Cargo.toml
@ -31,4 +31,3 @@ sqlite = []
 dataframe = []
 static-link-openssl = []
 wasi = []
-extra = []
--- a/crates/nu-cmd-lang/src/core_commands/version.rs
+++ b/crates/nu-cmd-lang/src/core_commands/version.rs
@ -163,9 +163,6 @@ fn features_enabled() -> Vec<String> {
        names.push("which".to_string());
    }

-    // always include it?
-    names.push("zip".to_string());
-
    #[cfg(feature = "trash-support")]
    {
        names.push("trash".to_string());
@ -186,11 +183,6 @@ fn features_enabled() -> Vec<String> {
        names.push("static-link-openssl".to_string());
    }

-    #[cfg(feature = "extra")]
-    {
-        names.push("extra".to_string());
-    }
-
    #[cfg(feature = "wasi")]
    {
        names.push("wasi".to_string());
--- a/crates/nu-command/tests/commands/each.rs
+++ b/crates/nu-command/tests/commands/each.rs
@ -52,7 +52,6 @@ fn each_uses_enumerate_index() {
 }

 #[test]
-#[cfg(feature = "extra")]
 fn each_while_uses_enumerate_index() {
    let actual = nu!("[7 8 9 10] | enumerate | each while {|el| $el.index } | to nuon");

--- a/crates/nu-command/tests/commands/format.rs
+++ b/crates/nu-command/tests/commands/format.rs
@ -9,7 +9,7 @@ fn creates_the_resulting_string_from_the_given_fields() {
        r#"
        open cargo_sample.toml
            | get package
-            | format "{name} has license {license}"
+            | format pattern "{name} has license {license}"
        "#
    ));

@ -18,7 +18,7 @@ fn creates_the_resulting_string_from_the_given_fields() {

 #[test]
 fn format_input_record_output_string() {
-    let actual = nu!(r#"{name: Downloads} | format "{name}""#);
+    let actual = nu!(r#"{name: Downloads} | format pattern "{name}""#);

    assert_eq!(actual.out, "Downloads");
 }
@ -29,7 +29,7 @@ fn given_fields_can_be_column_paths() {
        cwd: "tests/fixtures/formats", pipeline(
        r#"
        open cargo_sample.toml
-            | format "{package.name} is {package.description}"
+            | format pattern "{package.name} is {package.description}"
        "#
    ));

@ -42,7 +42,7 @@ fn can_use_variables() {
        cwd: "tests/fixtures/formats", pipeline(
        r#"
        open cargo_sample.toml
-            | format "{$it.package.name} is {$it.package.description}"
+            | format pattern "{$it.package.name} is {$it.package.description}"
        "#
    ));

@ -55,7 +55,7 @@ fn error_unmatched_brace() {
        cwd: "tests/fixtures/formats", pipeline(
        r#"
        open cargo_sample.toml
-            | format "{$it.package.name"
+            | format pattern "{$it.package.name"
        "#
    ));

--- a/crates/nu-command/tests/commands/mod.rs
+++ b/crates/nu-command/tests/commands/mod.rs
@ -35,7 +35,6 @@ mod find;
 mod first;
 mod flatten;
 mod for_;
-#[cfg(feature = "extra")]
 mod format;
 mod generate;
 mod get;
@ -85,9 +84,7 @@ mod rename;
 mod return_;
 mod reverse;
 mod rm;
-#[cfg(feature = "extra")]
 mod roll;
-#[cfg(feature = "extra")]
 mod rotate;
 mod run_external;
 mod save;
--- a/crates/nu-command/tests/format_conversions/html.rs
+++ b/crates/nu-command/tests/format_conversions/html.rs
@ -51,13 +51,16 @@ fn test_cd_html_color_flag_dark_false() {
 }

 #[test]
+#[ignore]
 fn test_no_color_flag() {
+    // TODO replace with something potentially more stable, otherwise this test needs to be
+    // manuallly updated when ever the help output changes
    let actual = nu!(r#"
                cd --help | to html --no-color
            "#);
    assert_eq!(
        actual.out,
-        r"<html><style>body { background-color:white;color:black; }</style><body>Change directory.<br><br>Usage:<br>  &gt; cd (path) <br><br>Flags:<br>  -h, --help - Display the help message for this command<br><br>Signatures:<br>  &lt;nothing&gt; | cd &lt;string?&gt; -&gt; &lt;nothing&gt;<br>  &lt;string&gt; | cd &lt;string?&gt; -&gt; &lt;nothing&gt;<br><br>Parameters:<br>  path &lt;directory&gt;: the path to change to (optional)<br><br>Examples:<br>  Change to your home directory<br>  &gt; cd ~<br><br>  Change to the previous working directory ($OLDPWD)<br>  &gt; cd -<br><br></body></html>"
+        r"<html><style>body { background-color:white;color:black; }</style><body>Change directory.<br><br>Usage:<br>  &gt; cd (path) <br><br>Flags:<br>  -h, --help - Display the help message for this command<br><br>Parameters:<br>  path &lt;directory&gt;: The path to change to. (optional)<br><br>Input&#x2f;output types:<br>  ╭─#─┬──input──┬─output──╮<br>  │ 0 │ nothing │ nothing │<br>  │ 1 │ string  │ nothing │<br>  ╰───┴─────────┴─────────╯<br><br>Examples:<br>  Change to your home directory<br>  &gt; cd ~<br><br>  Change to the previous working directory ($OLDPWD)<br>  &gt; cd -<br><br></body></html>"
    )
 }

--- a/crates/nu-command/tests/format_conversions/mod.rs
+++ b/crates/nu-command/tests/format_conversions/mod.rs
@ -1,5 +1,4 @@
 mod csv;
-#[cfg(feature = "extra")]
 mod html;
 mod json;
 mod markdown;
@ -8,7 +7,6 @@ mod ods;
 mod ssv;
 mod toml;
 mod tsv;
-#[cfg(feature = "extra")]
 mod url;
 mod xlsx;
 mod xml;
--- a/scripts/build-all-maclin.sh
+++ b/scripts/build-all-maclin.sh
@ -21,7 +21,7 @@ NU_PLUGINS=(
 echo "Building nushell"
 (
    cd $REPO_ROOT
-    cargo build --features=dataframe,extra --locked
+    cargo build --features=dataframe --locked
 )

 for plugin in "${NU_PLUGINS[@]}"
--- a/scripts/build-all-windows.cmd
+++ b/scripts/build-all-windows.cmd
@ -5,7 +5,7 @@ echo -------------------------------------------------------------------
 echo.

 echo Building nushell.exe
-cargo build --features=dataframe,extra --locked
+cargo build --features=dataframe --locked
 echo.

 call :build crates\nu_plugin_example nu_plugin_example.exe
--- a/scripts/build-all.nu
+++ b/scripts/build-all.nu
@ -13,7 +13,7 @@ def build-nushell [] {
    print '----------------------------'

    cd $repo_root
-    cargo build --features=dataframe,extra --locked
+    cargo build --features=dataframe --locked
 }

 def build-plugin [] {
--- a/scripts/install-all.ps1
+++ b/scripts/install-all.ps1
@ -8,7 +8,7 @@ Write-Output ""

 Write-Output "Install nushell from local..."
 Write-Output "----------------------------------------------"
-cargo install --force --path . --features=dataframe,extra --locked
+cargo install --force --path . --features=dataframe --locked

 $NU_PLUGINS = @(
    'nu_plugin_example',
--- a/scripts/install-all.sh
+++ b/scripts/install-all.sh
@ -12,7 +12,7 @@ echo ""

 echo "Install nushell from local..."
 echo "----------------------------------------------"
-cargo install --force --path "$REPO_ROOT" --features=dataframe,extra --locked
+cargo install --force --path "$REPO_ROOT" --features=dataframe --locked

 NU_PLUGINS=(
    'nu_plugin_inc'
--- a/src/main.rs
+++ b/src/main.rs
@ -42,7 +42,6 @@ use std::{
 fn get_engine_state() -> EngineState {
    let engine_state = nu_cmd_lang::create_default_context();
    let engine_state = nu_command::add_shell_command_context(engine_state);
-    #[cfg(feature = "extra")]
    let engine_state = nu_cmd_extra::add_extra_command_context(engine_state);
    #[cfg(feature = "dataframe")]
    let engine_state = nu_cmd_dataframe::add_dataframe_context(engine_state);
--- a/src/tests.rs
+++ b/src/tests.rs
@ -1,4 +1,3 @@
-#[cfg(feature = "extra")]
 mod test_bits;
 mod test_cell_path;
 mod test_commandline;
--- a/src/tests/test_engine.rs
+++ b/src/tests/test_engine.rs
@ -54,8 +54,7 @@ fn in_and_if_else() -> TestResult {

 #[test]
 fn help_works_with_missing_requirements() -> TestResult {
-    // `each while` is part of the *extra* feature and adds 3 lines
-    let expected_length = if cfg!(feature = "extra") { "70" } else { "67" };
+    let expected_length = "70";
    run_test(r#"each --help | lines | length"#, expected_length)
 }

--- a/tests/plugins/custom_values.rs
+++ b/tests/plugins/custom_values.rs
@ -80,8 +80,7 @@ fn can_get_describe_plugin_custom_values() {
 }

 // There are currently no custom values defined by the engine that aren't hidden behind an extra
-// feature, both database and dataframes are hidden behind --features=extra so we need to guard
-// this test
+// feature
 #[cfg(feature = "sqlite")]
 #[test]
 fn fails_if_passing_engine_custom_values_to_plugins() {