move common tools from `nu-command` to `nu-cmd-base` by amtoine · Pull Request #9455 · nushell/nushell

amtoine · 2023-06-16T10:04:23Z

related to

REFACTOR: move the 0% commands to nu-cmd-extra #9404

Description

to support our cratification effort and moving non-1.0 commands outside of the main focus, this PR

creates a new nu-cmd-base crate to hold the common structs, traits and functions used by all command-related crates
to start the transition, moves the input_handler module from nu-command to nu-cmd-base

User-Facing Changes

$nothing

Tests + Formatting

🟢 toolkit fmt
🟢 toolkit clippy
⚫ toolkit test
⚫ toolkit test stdlib

After Submitting

$nothing

amtoine · 2023-06-16T10:16:07Z

cc/ @stormasm

i've used the following command to isolate the tool items used throughout the nu-command crate:

rg "^use crate" crates/nu-command/src/math/ | lines | parse "{file}:{match}" | get match

which gives, after removing the use crate from the default_context and from the help command which obviously need these imports, we get the following table of items

╭────┬───────────────────────────────────────────────────────────────────────────────────────────────╮
│  1 │ use crate::math::reducers::{reducer_for, Reduce};                                             │
│  2 │ use crate::math::utils::run_with_function;                                                    │
│  3 │ use crate::input_handler::{operate, CmdArgument};                                             │
│  4 │ use crate::input_handler::{operate, CellPathOnlyArgs};                                        │
│  5 │ use crate::formats::to::delimited::merge_descriptors;                                         │
│  6 │ use crate::formats::nu_xml_format::{COLUMN_ATTRS_NAME, COLUMN_CONTENT_NAME, COLUMN_TAG_NAME}; │
│  7 │ use crate::formats::to::delimited::to_delimited_data;                                         │
│  8 │ use crate::math::avg::average;                                                                │
│ 13 │ use crate::input_handler::{operate as general_operate, CmdArgument};                          │
│ 14 │ use crate::util::get_guaranteed_cwd;                                                          │
│ 15 │ use crate::date::utils::parse_date_from_string;                                               │
│ 16 │ use crate::{generate_strftime_list, parse_date_from_string};                                  │
│ 17 │ use crate::formats::value_to_string;                                                          │
│ 18 │ use crate::help::highlight_search_string;                                                     │
│ 19 │ use crate::hook::eval_hook;                                                                   │
│ 20 │ use crate::database::values::definitions::{db_row::DbRow, db_table::DbTable};                 │
│ 21 │ use crate::database::values::sqlite::open_sqlite_db;                                          │
│ 22 │ use crate::ExternalCommand;                                                                   │
│ 23 │ use crate::network::http::client::{                                                           │
│ 24 │ use crate::formats::value_to_json_value;                                                      │
│ 25 │ use crate::database::values::definitions::db_row::DbRow;                                      │
│ 26 │ use crate::filesystem::util::FileStructure;                                                   │
│ 27 │ use crate::progress_bar;                                                                      │
│ 28 │ use crate::database::SQLiteDatabase;                                                          │
│ 29 │ use crate::DirBuilder;                                                                        │
│ 30 │ use crate::DirInfo;                                                                           │
│ 31 │ use crate::{DirBuilder, DirInfo, FileInfo};                                                   │
│ 32 │ use crate::filesystem::cd_query::query;                                                       │
│ 33 │ use crate::debug::inspect_table::{                                                            │
│ 34 │ use crate::{grapheme_flags, util};                                                            │
│ 35 │ use crate::grapheme_flags;                                                                    │
│ 36 │ use crate::operate;                                                                           │
╰────┴───────────────────────────────────────────────────────────────────────────────────────────────╯

❓ do we move them all to the new `nu-cmd-base` crate? 😮

stormasm · 2023-06-21T16:01:01Z

@amtoine

I think to be on the safe side of things we should move those helper functions over as we need them...

In other words the first one I would move in this PR is input_handler because the bytes command will need it...

So input_handler will be our first command in nu-cmd-base

and it will be up and "running" / landed

then I will do the bytes command

and then as we want to move more commands

we move the helper commands first and then the actual commands...

What do you think of this ?

amtoine · 2023-06-21T16:31:25Z

sounds good to me 👍

so let's move input_handler 😏

…ands-to-nu-cmd-base this should solve the `benches/` issue fixed in nushell#9454

crates/nu-command/Cargo.toml

stormasm · 2023-06-21T21:15:22Z

@kubouch @fdncred you all can go ahead and land this PR if you think its ok...

or I will check in with you all tomorrow morning after I wake up and we can figure out what to do....

thanks !

stormasm · 2023-06-21T21:17:33Z

all this PR is doing is moving input_handler to a new crate called nu-cmd-base

amtoine · 2023-06-22T16:57:23Z

or I will check in with you all tomorrow morning after I wake up and we can figure out what to do....

have you had the time to gain some insights into this? 😋

stormasm · 2023-06-22T21:48:41Z

@amtoine I went ahead and merged this as it was blocking me making forward progress on moving the bytes command to nu-cmd-extra...

Now I should be able to land that PR as well....

stormasm · 2023-06-22T21:49:32Z

And now that we have seen some other PRs fail in the exact same spot with Python virtualenv I was assured the issue is NOT related to this PR 😄

related to - nushell#9404 # Description to support our cratification effort and moving non-1.0 commands outside of the main focus, this PR - creates a new `nu-cmd-base` crate to hold the common structs, traits and functions used by all command-related crates - to start the transition, moves the `input_handler` module from `nu-command` to `nu-cmd-base` # User-Facing Changes ``` $nothing ``` # Tests + Formatting - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - ⚫ `toolkit test` - ⚫ `toolkit test stdlib` # After Submitting ``` $nothing ```

now that #9455 has landed we can move the bytes command to nu-cmd-extra in concert with moving nu_command::util to nu-cmd-base

requires - #9455 # ⚙️ Description in this PR i move the commands we've all agreed, in the core team, to move out of the core Nushell to the `extra` feature. > **Warning** > in the first commits here, i've > - moved the implementations to `nu-cmd-extra` > - removed the declaration of all the commands below from `nu-command` > - made sure the commands were not available anymore with `cargo run -- -n` ## the list of commands to move with the current command table downloaded as `commands.csv`, i've run ```bash let commands = ( open commands.csv | where is_plugin == "FALSE" and category != "deprecated" | select name category "approv. %" | rename name category approval | insert treated {|it| ( ($it.approval == 100) or # all the core team agreed on them ($it.name | str starts-with "bits") or # see #9241 ($it.name | str starts-with "dfr") # see #9327 )} ) ``` to preprocess them and then ```bash $commands | where {|it| (not $it.treated) and ($it.approval == 0)} ``` to get all untreated commands with no approval, which gives ``` ╭────┬───────────────┬─────────┬─────────────┬──────────╮ │ # │ name │ treated │ category │ approval │ ├────┼───────────────┼─────────┼─────────────┼──────────┤ │ 0 │ fmt │ false │ conversions │ 0 │ │ 1 │ each while │ false │ filters │ 0 │ │ 2 │ roll │ false │ filters │ 0 │ │ 3 │ roll down │ false │ filters │ 0 │ │ 4 │ roll left │ false │ filters │ 0 │ │ 5 │ roll right │ false │ filters │ 0 │ │ 6 │ roll up │ false │ filters │ 0 │ │ 7 │ rotate │ false │ filters │ 0 │ │ 8 │ update cells │ false │ filters │ 0 │ │ 9 │ decode hex │ false │ formats │ 0 │ │ 10 │ encode hex │ false │ formats │ 0 │ │ 11 │ from url │ false │ formats │ 0 │ │ 12 │ to html │ false │ formats │ 0 │ │ 13 │ ansi gradient │ false │ platform │ 0 │ │ 14 │ ansi link │ false │ platform │ 0 │ │ 15 │ format │ false │ strings │ 0 │ ╰────┴───────────────┴─────────┴─────────────┴──────────╯ ``` # 🖌️ User-Facing Changes ``` $nothing ``` # 🧪 Tests + Formatting - ⚫ `toolkit fmt` - ⚫ `toolkit clippy` - ⚫ `toolkit test` - ⚫ `toolkit test stdlib` # 📖 After Submitting ``` $nothing ``` # 🔍 For reviewers ```bash $commands | where {|it| (not $it.treated) and ($it.approval == 0)} | each {|command| try { help $command.name | ignore } catch {|e| $"($command.name): ($e.msg)" } } ``` should give no output in `cargo run --features extra -- -n` and a table with 16 lines in `cargo run -- -n`

add a base skeleton for the nu-cmd-base crate

563f4ee

amtoine added cratification category:refactor labels Jun 16, 2023

amtoine self-assigned this Jun 16, 2023

amtoine mentioned this pull request Jun 16, 2023

REFACTOR: move the 0% commands to nu-cmd-extra #9404

Merged

amtoine added 2 commits June 21, 2023 19:18

Merge commit '7d301c76d' into refactor/move-common-tools-from-nu-comm…

45db6b7

…ands-to-nu-cmd-base this should solve the `benches/` issue fixed in nushell#9454

move the input_handler module from nu-command to nu-cmd-base

06260e9

amtoine marked this pull request as ready for review June 21, 2023 17:22

sholderbach reviewed Jun 21, 2023

View reviewed changes

crates/nu-command/Cargo.toml Outdated Show resolved Hide resolved

bump the nu-cmd-base crate to 0.81.1

4de6bf7

stormasm merged commit 78697bb into nushell:main Jun 22, 2023

stormasm mentioned this pull request Jun 23, 2023

cratification: move the bytes command to nu-cmd-extra #9509

Merged

amtoine mentioned this pull request Jun 23, 2023

add the new crate nu-cmd-base to the release nushell/nu_scripts#533

Merged

amtoine deleted the refactor/move-common-tools-from-nu-commands-to-nu-cmd-base branch June 23, 2023 17:07

stormasm added a commit that referenced this pull request Jun 23, 2023

cratification: move the bytes command to nu-cmd-extra (#9509)

d1449c4

now that #9455 has landed we can move the bytes command to nu-cmd-extra in concert with moving nu_command::util to nu-cmd-base

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

move common tools from `nu-command` to `nu-cmd-base`#9455

move common tools from `nu-command` to `nu-cmd-base`#9455
stormasm merged 4 commits intonushell:mainfrom
amtoine:refactor/move-common-tools-from-nu-commands-to-nu-cmd-base

amtoine commented Jun 16, 2023 •

edited

Loading

Uh oh!

amtoine commented Jun 16, 2023

Uh oh!

stormasm commented Jun 21, 2023

Uh oh!

amtoine commented Jun 21, 2023

Uh oh!

Uh oh!

stormasm commented Jun 21, 2023

Uh oh!

stormasm commented Jun 21, 2023

Uh oh!

amtoine commented Jun 22, 2023

Uh oh!

stormasm commented Jun 22, 2023

Uh oh!

stormasm commented Jun 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

amtoine commented Jun 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

User-Facing Changes

Tests + Formatting

After Submitting

Uh oh!

amtoine commented Jun 16, 2023

❓ do we move them all to the new nu-cmd-base crate? 😮

Uh oh!

stormasm commented Jun 21, 2023

Uh oh!

amtoine commented Jun 21, 2023

Uh oh!

Uh oh!

stormasm commented Jun 21, 2023

Uh oh!

stormasm commented Jun 21, 2023

Uh oh!

amtoine commented Jun 22, 2023

Uh oh!

stormasm commented Jun 22, 2023

Uh oh!

stormasm commented Jun 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

amtoine commented Jun 16, 2023 •

edited

Loading

❓ do we move them all to the new `nu-cmd-base` crate? 😮