From 067ceedf7911cf3cdbed5954ccf6569de954988e Mon Sep 17 00:00:00 2001 From: Stefan Holderbach Date: Sun, 10 Mar 2024 17:29:02 +0100 Subject: [PATCH] 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) --- .github/workflows/ci.yml | 12 +----- .github/workflows/nightly-build.yml | 18 ++++----- .github/workflows/release.yml | 18 ++++----- Cargo.toml | 5 +-- crates/nu-cmd-extra/README.md | 37 +++++-------------- .../nu-cmd-extra/src/extra/formats/to/html.rs | 3 +- crates/nu-cmd-extra/tests/commands/mod.rs | 1 - crates/nu-cmd-lang/Cargo.toml | 1 - .../nu-cmd-lang/src/core_commands/version.rs | 8 ---- crates/nu-command/tests/commands/each.rs | 1 - crates/nu-command/tests/commands/format.rs | 10 ++--- crates/nu-command/tests/commands/mod.rs | 3 -- .../tests/format_conversions/html.rs | 5 ++- .../tests/format_conversions/mod.rs | 2 - scripts/build-all-maclin.sh | 2 +- scripts/build-all-windows.cmd | 2 +- scripts/build-all.nu | 2 +- scripts/install-all.ps1 | 2 +- scripts/install-all.sh | 2 +- src/main.rs | 1 - src/tests.rs | 1 - src/tests/test_engine.rs | 3 +- tests/plugins/custom_values.rs | 3 +- 23 files changed, 49 insertions(+), 93 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cbbf762fbd..3ae04c940c 100644 --- 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 }} diff --git a/.github/workflows/nightly-build.yml b/.github/workflows/nightly-build.yml index 723c46ba79..b8d6774948 100644 --- 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}} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 6c69506ae1..9d59c9080e 100644 --- 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}} diff --git a/Cargo.toml b/Cargo.toml index 0570bc5234..8315876ad3 100644 --- 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"] diff --git a/crates/nu-cmd-extra/README.md b/crates/nu-cmd-extra/README.md index 096d7ff711..4c61e97ef2 100644 --- 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. diff --git a/crates/nu-cmd-extra/src/extra/formats/to/html.rs b/crates/nu-cmd-extra/src/extra/formats/to/html.rs index cd879e6fa9..200ab89215 100644 --- 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( diff --git a/crates/nu-cmd-extra/tests/commands/mod.rs b/crates/nu-cmd-extra/tests/commands/mod.rs index 9195fb60dd..2354122e35 100644 --- 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; diff --git a/crates/nu-cmd-lang/Cargo.toml b/crates/nu-cmd-lang/Cargo.toml index 3665ff2787..78d324d326 100644 --- 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 = [] diff --git a/crates/nu-cmd-lang/src/core_commands/version.rs b/crates/nu-cmd-lang/src/core_commands/version.rs index fb6ff095db..0b64f879a0 100644 --- 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 { 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 { names.push("static-link-openssl".to_string()); } - #[cfg(feature = "extra")] - { - names.push("extra".to_string()); - } - #[cfg(feature = "wasi")] { names.push("wasi".to_string()); diff --git a/crates/nu-command/tests/commands/each.rs b/crates/nu-command/tests/commands/each.rs index 68d8fd4ac0..599597e66c 100644 --- 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"); diff --git a/crates/nu-command/tests/commands/format.rs b/crates/nu-command/tests/commands/format.rs index 1c7c827672..75567a9a30 100644 --- 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" "# )); diff --git a/crates/nu-command/tests/commands/mod.rs b/crates/nu-command/tests/commands/mod.rs index 94873132dd..464187a4d9 100644 --- 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; diff --git a/crates/nu-command/tests/format_conversions/html.rs b/crates/nu-command/tests/format_conversions/html.rs index 6dad5357a5..9f6644a812 100644 --- 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"Change directory.

Usage:
> cd (path)

Flags:
-h, --help - Display the help message for this command

Signatures:
<nothing> | cd <string?> -> <nothing>
<string> | cd <string?> -> <nothing>

Parameters:
path <directory>: the path to change to (optional)

Examples:
Change to your home directory
> cd ~

Change to the previous working directory ($OLDPWD)
> cd -

" + r"Change directory.

Usage:
> cd (path)

Flags:
-h, --help - Display the help message for this command

Parameters:
path <directory>: The path to change to. (optional)

Input/output types:
╭─#─┬──input──┬─output──╮
│ 0 │ nothing │ nothing │
│ 1 │ string │ nothing │
╰───┴─────────┴─────────╯

Examples:
Change to your home directory
> cd ~

Change to the previous working directory ($OLDPWD)
> cd -

" ) } diff --git a/crates/nu-command/tests/format_conversions/mod.rs b/crates/nu-command/tests/format_conversions/mod.rs index aaa256c154..7ce11f4a93 100644 --- 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; diff --git a/scripts/build-all-maclin.sh b/scripts/build-all-maclin.sh index 22a8c246dd..5490695573 100755 --- 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[@]}" diff --git a/scripts/build-all-windows.cmd b/scripts/build-all-windows.cmd index 49cb18fd8c..2619a294d0 100644 --- 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 diff --git a/scripts/build-all.nu b/scripts/build-all.nu index 3f1d302f59..2ad7ec467e 100644 --- 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 [] { diff --git a/scripts/install-all.ps1 b/scripts/install-all.ps1 index 63f2b5365f..0569de4ad3 100644 --- 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', diff --git a/scripts/install-all.sh b/scripts/install-all.sh index 94d54dac74..b53d53aa3b 100755 --- 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' diff --git a/src/main.rs b/src/main.rs index a97b2042be..4732058ad6 100644 --- 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); diff --git a/src/tests.rs b/src/tests.rs index ac7c3fa4f3..32b47bae19 100644 --- 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; diff --git a/src/tests/test_engine.rs b/src/tests/test_engine.rs index aa558236dc..9f5be84763 100644 --- 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) } diff --git a/tests/plugins/custom_values.rs b/tests/plugins/custom_values.rs index 6db4883921..a7923c03fe 100644 --- 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() {