stdlib: refactor into a multi-module library

[amtoine]

Description

this PR

• moves the tests to their own tests/ directory
• splits the old std.nu module into multiple submodules and a main mod.nu one in lib/
• fixes the mentions to the old structure
• i moved the log example to crates/nu-std/examples.nu, to be used as use crates/nu-std/examples.nu; examples log (cc/ @presidento)

🗝️ old structure

crates/nu-std/
├── Cargo.toml
├── LICENSE
├── README.md
├── src
│   └── lib.rs
├── std.nu
├── test_asserts.nu
├── test_dirs.nu
├── test_logger.nu
├── tests.nu
├── test_std.nu
└── test_xml.nu

2 directories, 11 files

🔑 new structure

crates/nu-std/
├── Cargo.toml
├── examples.nu
├── lib
│   ├── assert.nu
│   ├── dirs.nu
│   ├── help.nu
│   ├── log.nu
│   ├── mod.nu
│   └── xml.nu
├── LICENSE
├── README.md
├── src
│   └── lib.rs
└── tests
    ├── run.nu
    ├── test_asserts.nu
    ├── test_dirs.nu
    ├── test_logger.nu
    ├── test_std.nu
    └── test_xml.nu

4 directories, 17 files

⚠️ main limitations

• from crates/nu-std/lib/mod.nu, modules NEED to be exported with export use <mod> *

Note

• the * is important
• only use the name of the module, not the path or the filename (it's imported automatically since c3df973)

• with c3df973, the developer needs to modify the list of submodules in the crates/nu-std/src/lib.rs file => i think this is not so much of a limitation because, as the library is to be embedded in the nu binary, we need to abstract away from the actual files in nu-std
• commands need to be defined as "module command" in their modules such that we do not end up with std module module command when using the library
• to export environment, the following structure is to be used from inside mod.nu

export-env {
    use crates/nu-std/lib/<mod>.nu *
}

• view source std only shows the main mod.nu module
• we cannot import a whole submodule, e.g. use std help won't import all the help commands but rather only the help command

User-Facing Changes

$nothing

Tests + Formatting

• 🟢 toolkit fmt
• 🟢 toolkit clippy
• 🟢 toolkit test
• 🟢 toolkit test stdlib

After Submitting

$nothing

[bobhy]

Sorry, I don't know the best place to put this comment, but have you considered this alternative file structure? The structure you have is kind of familiar from the Rust project convention, but the one below might be easier for outside contributors to work with and maintain, since it puts all the files related to a custom command in one folder.

crates/nu-std/
├── Cargo.toml
├── lib
│   ├── assert
│   ├──── assert.nu
│   ├──── tests_assert.nu
│   ├── help
│   ├──── help.nu
│   ├──── tests_help.nu
. . .
├── LICENSE
├── README.md
├── src
│   └── lib.rs

There might be a concern that putting source and test files in the same folder might make it harder to build a pure production build distribution of the library, you'd have to strip out the test files from multiple folders. But we've already got the convention that test files are distinguished from (library) source files with the test_*, and you would still have the explicit use command to control what gets imported at runtime. And I do think this folder structure is easier to explain in CONTRIBUTING, fwiw.

[amtoine]

@bobhy

i'll come back to you shortly 😌

i would like to have a modular base which works => this is not the case right as you can see with the ❌ in the CI 😱

once we have a modular library fully working we'll be able to change the exact locations of the files as we please, almost a detail at that point 😉
but a detail that matters 💯 to write things like a clear CONTRIBUTING.md 😏

count on me not to forget about you 👍

[presidento]

I strongly suggest moving the unit testing framework (aka run.nu) to stdlib. We already have asserts, but they are not so useful without the test runner. (Let it be distributed with the assert commands.)

[amtoine]

Let it be distributed with the assert commands.

was tests.nu, now renamed tests/run.nu, meant to be a part of the library?
because it's been a script until now not a module and always only used to run the tests of the lib itself 🤔

i only moved it from tests.nu to tests.nu here, i think we can make it a true std module after this PR, not to accumulate too many changes 😌

[presidento]

was tests.nu, now renamed tests/run.nu, meant to be a part of the library?

For me, sooner or later, absolutely yes. Previously it contained only the tests, but now it is an almost complete unit testing framework. (The remaining part of supporting setup / teardown commands are in #8776). Some parts of this framework are already on standard library (the asserts), but they are not so useful without the runner.

I already created a PR to enhance Python plugin, and added tests for plugins. (Unfortunately I was not able to register a plugin in the CI, so I just closed that.)

I think in Nushell we can consider these as the recommended or the existing way of testing any code in Nushell.

If you wish, it can be done later, as well.

[amtoine]

For me, sooner or later, absolutely yes. Previously it contained only the tests, but now it is an almost complete unit testing framework. ... I think in Nushell we can consider these as the recommended or the existing way of testing any code in Nushell.

i agree with that

If you wish, it can be done later, as well.

yes, i prefer leaving the library untouched in this PR, only moving its parts 😌

[kubouch]

@amtoine You don't need to keep naming the commands with the module name. E.g., inside assert.nu you can define just def error instead of def "assert error". Then, you just call use assert.nu without the *. It's cleaner to do it that way since currently you're duplicating the assert namespace: You're creating a command named "assert error" inside the assert module.

[amtoine]

@kubouch

you are absolutely right, thanks for pointing this out to me 😮

i thought this would change the behaviour of the imports, i.e. exposing only equal for instance instead of assert equal, but it does not 👌

[amtoine]

@kubouch

mm an annoying thing is that we have some built-in command shadowing 🤔
e.g.
assert length, assert str contains, possibly xml update, xml insert, also dirs drop, ...

fixing the namespace right now would require changing some of the API 😕
i'm not sure it's the right PR to do so 😬

even though it's not perfect, at least the behaviour is the same right now as previously, wihout changing any command names 😌

[kubouch]

When you call use assert.nu, the commands will be named assert length etc., there won't be name shadowing.

But that can be done in another PR, that's OK. I'm also working in module changes which we can take advantage of in stdlib so there might be more changes in the future anyway.

[amtoine]

When you call use assert.nu, the commands will be named assert length etc., there won't be name shadowing.

not in the user space yes 👍
but i meant in the submodules themselves 😕

e.g. in the assert.nu submodule, assert length uses the builtin length command and assert str contains uses the builtin str contains command.
with use assert * all is fine.

but if we use use assert and rename the commands to length and str contains, that will break the whole library 🤔

But that can be done in another PR, that's OK. I'm also working in module changes which we can take advantage of in stdlib so there might be more changes in the future anyway.

oooh yeah definitely things we'll use in nu-std 💪

[kubouch]

I see, then you'll need to use a command with a different name internally inside the module. This is an interesting problem. A long-term solution I see is to have built-in commands in modules (imported by default). That would allow higher naming flexibility if you run into this situation, e.g., you could do use core and refer to the original length as core length inside the module.

[amtoine]

I see, then you'll need to use a command with a different name internally inside the module. This is an interesting problem. A long-term solution I see is to have built-in commands in modules (imported by default). That would allow higher naming flexibility if you run into this situation, e.g., you could do use core and refer to the original length as core length inside the module.

a core "module" to avoid built-in shadowing would be AMAZING 🤩