Updated summary for commit
[612e0e2](612e0e2160)
- While folks are welcome to read through the entire comments, the core
information is summarized here.
# Description
This PR drastically improves startup times of Nushell by only parsing a
single submodule of the Standard Library that provides the `banner` and
`pwd` commands. All other Standard Library commands and submodules are
parsed when imported by the user. This cuts startup times by more than
60%.
At the moment, we have stopped adding to `std-lib` because every
addition adds a small amount to the Nushell startup time.
With this change, we should once again be able to allow new
functionality to be added to the Standard Library without it impacting
`nu` startup times.
# User-Facing Changes
* Nushell now starts about 60% faster
* Breaking change: The `dirs` (Shells) aliases will return a warning
message that it will not be auto-loaded in the following release, along
with instructions on how to restore it (and disable the message)
* The `use std <submodule> *` syntax is available for convenience, but
should be avoided in scripts as it parses the entire `std` module and
all other submodules and places it in scope. The correct syntax to
*just* load a submodule is `use std/<submodule> *` (asterisk optional).
The slash is important. This will be documented.
* `use std *` can be used for convenience to load all of the library but
still incurs the full loading-time.
* `std/dirs`: Semi-breaking change. The `dirs` command replaces the
`show` command. This is more in line with the directory-stack
functionality found in other shells. Existing users will not be impacted
by this as the alias (`shells`) remains the same.
* Breaking-change: Technically a breaking change, but probably only
impacts maintainers of `std`. The virtual path for the standard library
has changed. It could previously be imported using its virtual path (and
technically, this would have been the correct way to do it):
```nu
use NU_STDLIB_VIRTUAL_DIR/std
```
The path is now simply `std/`:
```nu
use std
```
All submodules have moved accordingly.
# Timings
Comparisons below were made:
* In a temporary, clean config directory using `$env.XDG_CONFIG_HOME =
(mktemp -d)`.
* `nu` was run with a release build
* `nu` was run one time to generate the default `config.nu` (etc.) files
- Otherwise timings would include the user-prompt
* The shell was exited and then restarted several times to get timing
samples
(Note: Old timings based on 0.97 rather than 0.98, but in the range of
being accurate)
| Scenario | `$nu.startup-time` |
| --- | --- |
| 0.97.2
([aaaab8e](aaaab8e070))
Without this PR | 23ms - 24ms |
| This PR with deprecated commands | 9ms - <11ms |
| This PR after deprecated commands are removed in following release |
8ms - <10ms |
| Final PR (remove deprecated), using `--no-std-lib` | 6.1ms to 6.4ms |
| Final PR (remove deprecated), using `--no-config-file` | 3.1ms - 3.6ms
|
| Final PR (remove deprecated), using `--no-config-file --no-std-lib` |
1ms - 1.5ms |
*These last two timings point to the opportunity for further
optimization (see comment in thread below (will link once I write it).*
# Implementation details for future maintenance
* `use std banner` is a ridiculously deceptive call. That call parses
and imports *all* of `std` into scope. Simply replacing it with `use
std/core *` is essentially what saves ~14-15ms. This *only* imports the
submodule with the `banner` and `pwd` commands.
* From the code-comments, the reason that `NU_STDLIB_VIRTUAL_DIR` was
used as a prefix was so that there wouldn't be an issue if a user had a
`./std/mod.nu` in the current directory. This does **not** appear to be
an issue. After removing the prefix, I tested with both a relative
module as well as one in the `$env.NU_LIB_DIRS` path, and in all cases
the *internal* `std` still took precedence.
* By removing the prefix, users can now `use std` (and variants) without
requiring that it already be parsed and in scope.
* In the next release, we'll stop autoloading the `dirs` (shells)
functionality. While this only costs an additional 1-1.5ms, I think it's
better moved to the `config.nu` where the user can optionally remove it.
The main reason is its use of aliases (which have also caused issues) -
The `n`, `p`, and `g` short-commands are valuable real-estate, and users
may want to map these to something else.
For this release, there's an `deprecated_dirs` module that is still
autoloaded. As with the top-level commands, use of these will give a
deprecation warning with instructions on how to handle going forward.
To help with this, moved the aliases to their own submodule inside the
`dirs` module.
* Also sneaks in a small change where the top-level `dirs` command is
now the replacement for `dirs show`
* Fixed a double-import of `assert` in `dirs.nu`
* The `show_banner` step is replaced with simply `banner` rather than
re-importing it.
* A `virtual_path` may now be referenced with either a forward-slash or
a backward-slash on Windows. This allows `use std/<submodule>` to work
on all platforms.
# Performance side-notes:
* Future parsing and/or IR improvements should improve performance even
further.
* While the existing load time penalty of `std-lib` was not noticeable
on many systems, Nushell runs on a wide-variety of hardware and OS
platforms. Slower platforms will naturally see a bigger jump in
performance here. For users starting multiple Nushell sessions
frequently (e.g., `tmux`, Zellij, `screen`, et. al.) it is recommended
to keep total startup time (including user configuration) under ~250ms.
# Tests + Formatting
* All tests are green
* Updated tests:
- Removed the test that confirmed that `std` was loaded (since we
don't).
- Removed the `shells` test since it is not autoloaded. Main `dirs.nu`
functionality is tested through `stdlib-test`.
- Many tests assumed that the library was fully loaded, because it was
(even though we didn't intend for it to be). Fixed those tests.
- Tests now import only the necessary submodules (e.g., `use
std/assert`, rather than `use std assert`)
- Some tests *thought* they were loading `std/log`, but were doing so
improperly. This was masked by the now-fixed "load-everything-into-scope
bug". Local CI would pass due the `$env.NU_LOG_<...>` variables being
inherited from the calling process, but would fail in the "clean" GitHub
CI environment. These tests have also been fixed.
* Added additional tests for the changes
# After Submitting
Will update the Standard Library doc page
Recommend holding until after #13125 is fully digested and *possibly*
until 0.96.
# Description
Fixes one of the issues described in #13125
The `do` signature included a `SyntaxShape:Any` as one of the possible
first-positional types. This is incorrect. `do` only takes a closure as
a positional. This had the result of:
1. Moving what should have been a parser error to evaluation-time
## Before
```nu
> do 1
Error: nu:🐚:cant_convert
× Can't convert to Closure.
╭─[entry #26:1:4]
1 │ do 1
· ┬
· ╰── can't convert int to Closure
╰────
```
## After
```nu
> do 1
Error: nu::parser::parse_mismatch
× Parse mismatch during operation.
╭─[entry #5:1:4]
1 │ do 1
· ┬
· ╰── expected block, closure or record
╰────
```
2. Masking a bad test in `std assert`
This is a bit convoluted, but `std assert` tests included testing
`assert error` to make sure it:
* Asserts on bad code
* Doesn't assert on good code
The good-code test was broken, and was essentially bad-code (really
bad-code) that wasn't getting caught due to the bad signature.
Fixing this resulted in *parse time* failures on every call to
`test_asserts` (not something that particular test was designed to
handle.
This PR also fixes the test case to properly evaluate `std assert error`
against a good code path.
# User-Facing Changes
* Error-type returned (possible breaking change?)
# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`
# After Submitting
N/A
# Description
We keep "into decimal" for a release and warn through a message that it
will be removed in 0.86.
All tests are updated to use `into float`
# User-Facing Changes
`into decimal` raises a deprecation warning, will be removed soon.
Use `into float` as the new functionally identical command instead.
```
~/nushell> 2 | into decimal
Error: × Deprecated command
╭─[entry #1:1:1]
1 │ 2 | into decimal
· ──────┬─────
· ╰── `into decimal` is deprecated and will be removed in 0.86.
╰────
help: Use `into float` instead
2
```
# Tests + Formatting
Updated
---------
Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com>
<!--
if this PR closes one or more issues, you can automatically link the PR
with
them by using one of the [*linking
keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword),
e.g.
- this PR should close #xxxx
- fixes #xxxx
you can also mention related issues, PRs or discussions!
-->
# Description
Test runner now uses annotations instead of magic function names to pick
up code to run. Additionally skipping tests is now done on annotation
level so skipping and unskipping a test no longer requires changes to
the test code
In order for a function to be picked up by the test runner it needs to
meet following criteria:
* Needs to be private (all exported functions are ignored)
* Needs to contain one of valid annotations (and only the annotation)
directly above the definition, all other comments are ignored
Following are considered valid annotations:
* \# test
* \# test-skip
* \# before-all
* \# before-each
* \# after-each
* \# after-all
# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->
# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.
Make sure you've run and fixed any issues with these commands:
- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect -A clippy::result_large_err` to check that
you're using the standard code style
- `cargo test --workspace` to check that all tests pass
- `cargo run -- crates/nu-std/tests/run.nu` to run the tests for the
standard library
> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->
# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
# Description
Test runner now performs following actions in order to run tests:
* Module file is opened
* Public function with random name is added to the source code, this
function calls user-specified private function
* Modified module file is saved under random name in $nu.temp-path
* Modified module file is imported in subprocess, injected function is
called by the test runner
# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->
* Test functions no longer need to be exported
* test functions no longer need to reside in separate test_ files
* setup and teardown renamed to before-each and after-each respectively
* before-all and after-all functions added that run before all tests in
given module. This matches the behavior of test runners used by other
languages such as JUnit/TestNG or Mocha
# Tests + Formatting
# After Submitting
---------
Co-authored-by: Kamil <skelly37@protonmail.com>
Co-authored-by: amtoine <stevan.antoine@gmail.com>
# Description
A new command to simplify assertions for `false`.
The name is just a draft, though I could not come up with a better name.
I have rejected `assert false`, because I would have to rename `assert`
to `assert true` which would break the compatibility and worsen the
shell experience of the good old `assert`.
Another idea I have rejected was something like `assert_false` to keep
it consistent with the naming convention of our stdlib.
I am open to suggestions :)
# User-Facing Changes
Just a new command
# Tests + Formatting
- [x] Done
# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
