REFACTOR: move the 0% commands to `nu-cmd-extra` by amtoine · Pull Request #9404 · nushell/nushell

amtoine · 2023-06-10T17:42:50Z

requires

move common tools from nu-command to nu-cmd-base #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

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 https://github.com/nushell/nushell/pull/9241
        ($it.name | str starts-with "dfr")      # see https://github.com/nushell/nushell/pull/9327
    )}
)

to preprocess them and then

$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

$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

Commands used: ```nu mkdir crates/nu-cmd-extra/src/extra/filters/ crates/nu-cmd-extra/src/extra/filters/roll/ mv crates/nu-command/src/filters/each_while.rs crates/nu-cmd-extra/src/extra/filters/each_while.rs mv crates/nu-command/src/filters/update_cells.rs crates/nu-cmd-extra/src/extra/filters/update_cells.rs mv crates/nu-command/src/filters/rotate.rs crates/nu-cmd-extra/src/extra/filters/rotate.rs mv crates/nu-command/src/filters/roll/ crates/nu-cmd-extra/src/extra/filters/roll/ mkdir crates/nu-cmd-extra/src/extra/strings/encode_decode/ mv crates/nu-command/src/strings/encode_decode/hex.rs crates/nu-cmd-extra/src/extra/strings/encode_decode/_hex.rs mv crates/nu-command/src/strings/encode_decode/encode_hex.rs crates/nu-cmd-extra/src/extra/strings/encode_decode/encode_hex.rs mv crates/nu-command/src/strings/encode_decode/decode_hex.rs crates/nu-cmd-extra/src/extra/strings/encode_decode/decode_hex.rs mv crates/nu-command/src/strings/format/ crates/nu-cmd-extra/src/extra/strings/format/ mkdir crates/nu-cmd-extra/src/extra/formats/to/ crates/nu-cmd-extra/src/extra/formats/from/ mv crates/nu-command/src/formats/to/html.rs crates/nu-cmd-extra/src/extra/formats/to/html.rs mv crates/nu-command/src/formats/from/url.rs crates/nu-cmd-extra/src/extra/formats/from/url.rs mkdir crates/nu-cmd-extra/src/extra/platform/ansi/ mv crates/nu-command/src/platform/ansi/gradient.rs crates/nu-cmd-extra/src/extra/platform/ansi/gradient.rs ```

stormasm · 2023-06-10T18:25:33Z

@amtoine I thought we were going to move the math commands and the bytes commands first prior to considering doing these ?

That is why I started with bits...

In our core team meeting a few weeks back we all agreed to do those first...

So I will let others comment on which commands they want to move across besides
math and bytes which everyone has already signed off on 😄

stormasm · 2023-06-10T18:29:24Z

However, I am OK with moving all of these commands across to nu-cmd-extra as well...

stormasm

I am fine with moving these commands across to nu-cmd-extra...
However we should get feedback from some other folks as well...
Just to make sure they are OK with it too. 😄

amtoine · 2023-06-11T07:36:34Z

@stormasm
i've asked this in the core team channel 😌

amtoine · 2023-06-11T07:36:53Z

i'll continue working on this in the meantime, this won't be lost work 👌

there was an extra level of depth

related to - #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 ```

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 ```

this should solve the merge conflict in nushell#9404.

this solves the following error ``` error: proc-macro derive panicked --> crates/nu-cmd-extra/src/extra/formats/to/html.rs:79:10 | 79 | #[derive(RustEmbed)] | ^^^^^^^^^ | = help: message: #[derive(RustEmbed)] folder '/home/amtoine/.local/share/git/store/github.com/amtoine/nushell/crates/nu-cmd-extra/assets/' does not exist. cwd: '/home/amtoine/.local/share/git/store/github.com/amtoine/nushell' error[E0599]: no function or associated item named `get` found for struct `Assets` in the current scope --> crates/nu-cmd-extra/src/extra/formats/to/html.rs:228:19 | 81 | struct Assets; | ------------- function or associated item `get` not found for this struct ... 228 | match Assets::get(json_name) { | ^^^ function or associated item not found in `Assets` | = help: items from traits can only be used if the trait is implemented and in scope = note: the following traits define an item `get`, perhaps you need to implement one of them: candidate #1: `SliceIndex` candidate #2: `RustEmbed` For more information about this error, try `rustc --explain E0599`. error: could not compile `nu-cmd-extra` due to 2 previous errors warning: build failed, waiting for other jobs to finish... ```

amtoine · 2023-07-01T11:07:43Z

@stormasm
some news and improvements here 😌

i've solved the conflicts
moved some functions and assets
moved the missing ansi link command

now i'm trying to fix the tests but i have some trouble...
i was able to solve help_works_with_missing_requirements but now the examples of each while and update cells are failing and i do not understand what is the problem here 😕

stormasm · 2023-07-01T15:14:52Z

@amtoine see my comments to you in discord 😄

* bring `enumerate` and `if` into `extra` tests * disable all failing tests when not in `extra`

this should solve the merge conflicts in nushell#9404.

amtoine · 2023-07-06T08:35:10Z

cc/ @stormasm

the CI still is suspicious but i think this PR is somewhat unrelated to the CI issue.
as the jobs should all be 🟢 as in amtoine#4, i think we can land this, if you're still ok with the changes 😌

crates/nu-cmd-extra/Cargo.toml

stormasm

Great job @amtoine

amtoine added 3 commits June 10, 2023 19:36

move fmt to nu-cmd-extra

2cd7527

remove the old exports

1b4634f

amtoine added the status:needs-core-team-attention An issue than needs the attention of other core-team members label Jun 10, 2023

amtoine requested a review from stormasm June 10, 2023 17:42

stormasm approved these changes Jun 10, 2023

View reviewed changes

amtoine changed the title ~~REFACTOR: move the 0% percent commands to nu-cmd-extra~~ REFACTOR: move the 0% commands to nu-cmd-extra Jun 11, 2023

amtoine added 4 commits June 11, 2023 09:58

move the roll commands to the right location

720880f

there was an extra level of depth

declare and export all commands out of extra

daa3593

fix the visibility of the HEX tool functions

78f1152

add missing dependencies from nu-command

84e3a55

amtoine added notes:breaking-changes This PR implies a change affecting users and has to be noted in the release notes cratification labels Jun 11, 2023

amtoine mentioned this pull request Jun 16, 2023

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

Merged

amtoine self-assigned this Jun 16, 2023

amtoine added 6 commits July 1, 2023 11:48

Merge commit '9547c106d' into refactor/move-0-percent-commands-to-extra

40d5501

this should solve the merge conflict in nushell#9404.

move merge_descriptors from nu-command to nu-cmd-base

7b353c2

move the ansi link command as well

142724a

fix the length of each --help without each while

ea27b6f

change the each --help length depending on the feature

c79ad0b

amtoine mentioned this pull request Jul 2, 2023

fix the tests of nushell/nushell#9404 amtoine/nushell#4

Merged

amtoine and others added 2 commits July 6, 2023 10:24

fix the tests of nushell#9404 (#4)

86a2e3e

* bring `enumerate` and `if` into `extra` tests * disable all failing tests when not in `extra`

Merge branch 'main' into refactor/move-0-percent-commands-to-extra

6d7b120

this should solve the merge conflicts in nushell#9404.

amtoine marked this pull request as ready for review July 6, 2023 08:33

amtoine requested a review from stormasm July 6, 2023 08:33

sholderbach reviewed Jul 6, 2023

View reviewed changes

crates/nu-cmd-extra/Cargo.toml Outdated Show resolved Hide resolved

move test dependencies to dev-dependencies in nu-cmd-extra

d589a05

stormasm merged commit 504eff7 into nushell:main Jul 6, 2023

stormasm reviewed Jul 6, 2023

View reviewed changes

amtoine deleted the refactor/move-0-percent-commands-to-extra branch July 6, 2023 15:44

amtoine mentioned this pull request Jul 7, 2023

allow tables to have annotations #9613

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

REFACTOR: move the 0% commands to `nu-cmd-extra`#9404
stormasm merged 16 commits intonushell:mainfrom
amtoine:refactor/move-0-percent-commands-to-extra

amtoine commented Jun 10, 2023 •

edited

Loading

Uh oh!

stormasm commented Jun 10, 2023

Uh oh!

stormasm commented Jun 10, 2023

Uh oh!

stormasm left a comment

Uh oh!

amtoine commented Jun 11, 2023

Uh oh!

amtoine commented Jun 11, 2023

Uh oh!

amtoine commented Jul 1, 2023

Uh oh!

stormasm commented Jul 1, 2023

Uh oh!

amtoine commented Jul 6, 2023

Uh oh!

Uh oh!

stormasm left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

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

Uh oh!

⚙️ Description

the list of commands to move

🖌️ User-Facing Changes

🧪 Tests + Formatting

📖 After Submitting

🔍 For reviewers

Uh oh!

stormasm commented Jun 10, 2023

Uh oh!

stormasm commented Jun 10, 2023

Uh oh!

stormasm left a comment

Choose a reason for hiding this comment

Uh oh!

amtoine commented Jun 11, 2023

Uh oh!

amtoine commented Jun 11, 2023

Uh oh!

amtoine commented Jul 1, 2023

Uh oh!

stormasm commented Jul 1, 2023

Uh oh!

amtoine commented Jul 6, 2023

Uh oh!

Uh oh!

stormasm left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

amtoine commented Jun 10, 2023 •

edited

Loading