Commit Graph

170 Commits

Author SHA1 Message Date
Piepmatz
66bc0542e0
Refactor I/O Errors (#14927)
<!--
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
<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->

As mentioned in #10698, we have too many `ShellError` variants, with
some even overlapping in meaning. This PR simplifies and improves I/O
error handling by restructuring `ShellError` related to I/O issues.
Previously, `ShellError::IOError` only contained a message string,
making it convenient but overly generic. It was widely used without
providing spans (#4323).

This PR introduces a new `ShellError::Io` variant that consolidates
multiple I/O-related errors (except for `ShellError::NetworkFailure`,
which remains distinct for now). The new `ShellError::Io` variant
replaces the following:

- `FileNotFound`
- `FileNotFoundCustom`
- `IOInterrupted`
- `IOError`
- `IOErrorSpanned`
- `NotADirectory`
- `DirectoryNotFound`
- `MoveNotPossible`
- `CreateNotPossible`
- `ChangeAccessTimeNotPossible`
- `ChangeModifiedTimeNotPossible`
- `RemoveNotPossible`
- `ReadingFile`

## The `IoError`
`IoError` includes the following fields:

1. **`kind`**: Extends `std::io::ErrorKind` to specify the type of I/O
error without needing new `ShellError` variants. This aligns with the
approach used in `std::io::Error`. This adds a second dimension to error
reporting by combining the `kind` field with `ShellError` variants,
making it easier to describe errors in more detail. As proposed by
@kubouch in [#design-discussion on
Discord](https://discord.com/channels/601130461678272522/615329862395101194/1323699197165178930),
this helps reduce the number of `ShellError` variants. In the error
report, the `kind` field is displayed as the "source" of the error,
e.g., "I/O error," followed by the specific kind of I/O error.
2. **`span`**: A non-optional field to encourage providing spans for
better error reporting (#4323).
3. **`path`**: Optional `PathBuf` to give context about the file or
directory involved in the error (#7695). If provided, it’s shown as a
help entry in error reports.
4. **`additional_context`**: Allows adding custom messages when the
span, kind, and path are insufficient. This is rendered in the error
report at the labeled span.
5. **`location`**: Sometimes, I/O errors occur in the engine itself and
are not caused directly by user input. In such cases, if we don’t have a
span and must set it to `Span::unknown()`, we need another way to
reference the error. For this, the `location` field uses the new
`Location` struct, which records the Rust file and line number where the
error occurred. This ensures that we at least know the Rust code
location that failed, helping with debugging. To make this work, a new
`location!` macro was added, which retrieves `file!`, `line!`, and
`column!` values accurately. If `Location::new` is used directly, it
issues a warning to remind developers to use the macro instead, ensuring
consistent and correct usage.

### Constructor Behavior
`IoError` provides five constructor methods:
- `new` and `new_with_additional_context`: Used for errors caused by
user input and require a valid (non-unknown) span to ensure precise
error reporting.
- `new_internal` and `new_internal_with_path`: Used for internal errors
where a span is not available. These methods require additional context
and the `Location` struct to pinpoint the source of the error in the
engine code.
- `factory`: Returns a closure that maps an `std::io::Error` to an
`IoError`. This is useful for handling multiple I/O errors that share
the same span and path, streamlining error handling in such cases.

## New Report Look
This is simulation how the I/O errors look like (the `open crates` is
simulated to show how internal errors are referenced now):
![Screenshot 2025-01-25
190426](https://github.com/user-attachments/assets/a41b6aa6-a440-497d-bbcc-3ac0121c9226)

## `Span::test_data()`
To enable better testing, `Span::test_data()` now returns a value
distinct from `Span::unknown()`. Both `Span::test_data()` and
`Span::unknown()` refer to invalid source code, but having a separate
value for test data helps identify issues during testing while keeping
spans unique.

## Cursed Sneaky Error Transfers
I removed the conversions between `std::io::Error` and `ShellError` as
they often removed important information and were used too broadly to
handle I/O errors. This also removed the problematic implementation
found here:

7ea4895513/crates/nu-protocol/src/errors/shell_error.rs (L1534-L1583)

which hid some downcasting from I/O errors and made it hard to trace
where `ShellError` was converted into `std::io::Error`. To address this,
I introduced a new struct called `ShellErrorBridge`, which explicitly
defines this transfer behavior. With `ShellErrorBridge`, we can now
easily grep the codebase to locate and manage such conversions.

## Miscellaneous
- Removed the OS error added in #14640, as it’s no longer needed.
- Improved error messages in `glob_from` (#14679).
- Trying to open a directory with `open` caused a permissions denied
error (it's just what the OS provides). I added a `is_dir` check to
provide a better error in that case.

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

- Error outputs now include more detailed information and are formatted
differently, including updated error codes.
- The structure of `ShellError` has changed, requiring plugin authors
and embedders to update their implementations.

# 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` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use toolkit.nu; toolkit test stdlib"` 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
> ```
-->

I updated tests to account for the new I/O error structure and
formatting changes.

# 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.
-->

This PR closes #7695 and closes #14892 and partially addresses #4323 and
#10698.

---------

Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com>
2025-01-28 16:03:31 -06:00
Ian Manske
93e121782c
Improve and fix filesize formatting/display (#14397)
# Description

This PR cleans up the code surrounding formatting and displaying file
sizes.
- The `byte_unit` crate we use for file size units displays kilobytes as
`KB`, which is not the SI or ISO/IEC standard. Rather it should be `kB`,
so this fixes #8872. On some systems, `KB` actually means `KiB`, so this
avoids any potential confusion.
- The `byte_unit` crate, when displaying file sizes, casts integers to
floats which will lose precision for large file sizes. This PR adds a
custom `Display` implementation for `Filesize` that can give an exact
string representation of a `Filesize` for metric/SI units.
- This PR also removes the dependency on the `byte_unit` crate which
brought in several other dependencies.

Additionally, this PR makes some changes to the config for filesize
formatting (`$env.config.filesize`).
- The previous filesize config had the `metric` and `format` options. If
a metric (SI) unit was set in `format`, but `metric` was set to false,
then the `metric` option would take precedence and convert `format` to
the corresponding binary unit (or vice versa). E.g., `{ format: kB,
metric: false }` => `KiB`. Instead, this PR adds the `unit` option to
replace the `format` and `metric` options. `unit` can be set to a fixed
file size unit like `kB` or `KiB`, or it can be set to one of the
special options: `binary` or `metric`. These options tells nushell to
format file sizes using an appropriately scaled metric or binary unit
(examples below).
  ```nushell
  # precision = null

  # unit = kB
  1kB  # 1 kB
  1KiB # 1.024 kB
  
  # unit = KiB
  1kB  # 0.9765625 KiB
  1KiB # 1 KiB
  
  # unit = metric
  1000B     # 1 kB
  1024B     # 1.024 kB
  10_000MB  # 10 GB
  10_240MiB # 10.73741824 GB

  # unit = binary
  1000B     # 1000 B
  1024B     # 1 KiB
  10_000MB  # 9.313225746154785 GiB
  10_240MiB # 10 GiB
  ```
- In addition, this PR also adds the `precision` option to the filesize
config. It determines how many digits to show after the decimal point.
If set to null, then everything after the decimal point is shown.
- The default filesize config is `{ unit: metric, precision: 1 }`.

# User-Facing Changes

- Commands that use the config to format file sizes will follow the
changes described above (e.g., `table`, `into string`, `to text`, etc.).
- The file size unit/format passed to `format filesize` is now case
sensitive. An error with the valid units is shown if the case does not
match.
- `$env.config.filesize.format` and `$env.config.filesize.metric` are
deprecated and replaced by `$env.config.filesize.unit`.
- A new `$env.config.filesize.precision` option was added.

# Tests + Formatting

Mostly updated test expected outputs.

# After Submitting

This PR does not change the way NUON serializes file sizes, because that
would require changing the nu parser to be able to losslessly decode the
new, exact string representation introduced in this PR.

Similarly, this PR also does not change the file size parsing in any
way. Although the file size units provided to `format filesize` or the
filesize config are now case-sensitive, the same is not yet true for
file size literals in nushell code.
2025-01-22 22:24:51 -08:00
Darren Schroeder
dad956b2ee
more closure serialization (#14698)
# Description

This PR introduces a switch `--serialize` that allows serializing of
types that cannot be deserialized. Right now it only serializes closures
as strings in `to toml`, `to json`, `to nuon`, `to text`, some indirect
`to html` and `to yaml`.

A lot of the changes are just weaving the engine_state through calling
functions and the rest is just repetitive way of getting the closure
block span and grabbing the span's text.

In places where it has to report `<Closure 123>` I changed it to
`closure_123`. It always seemed like the `<>` were not very nushell-y.
This is still a breaking change.

I think this could also help with systematic translation of old config
to new config file.


# 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` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use toolkit.nu; toolkit test stdlib"` 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.
-->
2025-01-07 11:51:22 -06:00
Rikuki IX
6ebc0fc3ff
Switch from serde_yaml to serde_yml (#14630)
# Description
This PR fixes #14339.

Since [serde_yaml](https://docs.rs/serde_yaml/latest/serde_yaml/) is
already deprecated, replaced it with
[serde_yml](https://doc.serdeyml.com/serde_yml/).

After this change, the `to yaml` boolean parsing issue in #14339 is also
fixed.
Now the command
```
['y' 'Y' 'yes' 'Yes' 'YES' 'n' 'N' 'no' 'No' 'No' 'on' 'On' 'ON' 'off' 'Off' 'OFF'] | to yaml
```
will return
```
- 'y'
- 'Y'
- 'yes'
- 'Yes'
- 'YES'
- 'n'
- 'N'
- 'no'
- 'No'
- 'No'
- 'on'
- 'On'
- 'ON'
- 'off'
- 'Off'
- 'OFF'
```

# User-Facing Changes

I'm not sure if the yaml spec change is a user-facing change.
2024-12-25 21:35:49 +08:00
Ian Manske
7f61cbbfd6
Add Filesize type (#14369)
# Description
Adds a new `Filesize` type so that `FromValue` can be used to convert a
`Value::Filesize` to a `Filesize`. Currently, to extract a filesize from
a `Value` using `FromValue`, you have to extract an `i64` which coerces
`Value::Int`, `Value::Duration`, and `Value::Filesize` to an `i64`.

Having a separate type also allows us to enforce checked math to catch
overflows. Similarly, it allows us to specify other trait
implementations like `Display` in a common place.

# User-Facing Changes
Multiplication with filesizes now error on overflow. Should not be a
breaking change for plugins (i.e., serialization) since `Filesize` is
marked with `serde(transparent)`.

# Tests + Formatting
Updated some tests.
2024-11-29 21:24:17 +00:00
Michel Lind
6e84ba182e
Bump quick-xml to 0.37.0 (#14354)
# Description
Bump `quick-xml` to `0.37.0`.

This came about rebasing `nushell` in Fedora, which now has `quick-xml`
0.36.

There is one breaking change in 0.33 as far as `nu-command` is
concerned, in that `Event::PI` is now a dedicated `BytesPI` type:


https://github.com/tafia/quick-xml/blob/master/Changelog.md#misc-changes-5

I've tested compiling and testing locally with `0.33.0`, `0.36.0` and
`0.37.0` - but let's future-proof by requiring `0.37.0`.


# User-Facing Changes
N/A

# Tests + Formatting
No additional tests required, existing tests pass

# After Submitting
N/A

Signed-off-by: Michel Lind <salimma@fedoraproject.org>
2024-11-18 18:26:31 -06:00
Ian Manske
62198a29c2
Make to text line endings consistent for list (streams) (#14166)
# Description
Fixes #14151 where `to text` treats list streams and lists values
differently.

# User-Facing Changes
New line is always added after items in a list or record except for the
last item if the `--no-newline` flag is provided.
2024-11-05 09:33:54 +01:00
Darren Schroeder
abb6fca5e3
make adding newlines with to text more consistent and opt-out-able (#14158)
# Description

This PR tries to make `to text` more consistent with how it adds
newlines and also gives you an opt-out --no-newline option.

![image](https://github.com/user-attachments/assets/e4976ce6-c685-47a4-8470-4947970daf47)


I wasn't sure how to change the `PipelineData::ByteStream` match arm. I
figure something needs to be done there but I'm not sure how to do it.


# User-Facing Changes
newlines are more consistent.

# 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` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use toolkit.nu; toolkit test stdlib"` 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.
-->
2024-10-23 16:49:51 -05:00
Piepmatz
f0c83a4459
Replace raw usize IDs with new types (#13832)
# Description

In this PR I replaced most of the raw usize IDs with
[newtypes](https://doc.rust-lang.org/rust-by-example/generics/new_types.html).
Some other IDs already started using new types and in this PR I did not
want to touch them. To make the implementation less repetitive, I made
use of a generic `Id<T>` with marker structs. If this lands I would try
to move make other IDs also in this pattern.

Also at some places I needed to use `cast`, I'm not sure if the type was
incorrect and therefore casting not needed or if actually different ID
types intermingle sometimes.

# User-Facing Changes

Probably few, if you got a `DeclId` via a function and placed it later
again it will still work.
2024-09-30 13:20:15 +02:00
Devyn Cairns
9ca0fb772d
Make IR the default evaluator (#13718)
# Description

Makes IR the default evaluator, in preparation to remove the non-IR
evaluator in a future release.

# User-Facing Changes

* Remove `NU_USE_IR` option
* Add `NU_DISABLE_IR` option
* IR is enabled unless `NU_DISABLE_IR` is set

# After Submitting
- [ ] release notes
2024-09-15 14:54:38 -07:00
Stefan Holderbach
95b78eee25
Change the usage misnomer to "description" (#13598)
# Description
    
The meaning of the word usage is specific to describing how a command
function is *used* and not a synonym for general description. Usage can
be used to describe the SYNOPSIS or EXAMPLES sections of a man page
where the permitted argument combinations are shown or example *uses*
are given.
Let's not confuse people and call it what it is a description.

Our `help` command already creates its own *Usage* section based on the
available arguments and doesn't refer to the description with usage.

# User-Facing Changes

`help commands` and `scope commands` will now use `description` or
`extra_description`
`usage`-> `description`
`extra_usage` -> `extra_description`

Breaking change in the plugin protocol:

In the signature record communicated with the engine.
`usage`-> `description`
`extra_usage` -> `extra_description`

The same rename also takes place for the methods on
`SimplePluginCommand` and `PluginCommand`

# Tests + Formatting
- Updated plugin protocol specific changes
# After Submitting
- [ ] update plugin protocol doc
2024-08-22 12:02:08 +02:00
Jack Wright
73e8de9753
Attempt to guess the content type of a file when opening with --raw (#13521)
# Description
Attempt to guess the content type of a file when opening with --raw and
set it in the pipeline metadata.

<img width="644" alt="Screenshot 2024-08-02 at 11 30 10"
src="https://github.com/user-attachments/assets/071f0967-c4dd-405a-b8c8-f7aa073efa98">


# User-Facing Changes
- Content of files can be directly piped into commands like `http post`
with the content type set appropriately when using `--raw`.
2024-08-06 11:36:24 +02:00
Jack Wright
d34a24db33
setting content type metadata on all core to * commands (#13506)
# Description

All core `to *` set content type pipeline metadata. 

# User-Facing Changes
- For consistency, `from json` no longer sets the content type metadata

# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib
2024-08-01 11:10:52 +02:00
Devyn Cairns
f65bc97a54
Update config directly at assignment (#13332)
# Description

Allows `Stack` to have a modified local `Config`, which is updated
immediately when `$env.config` is assigned to. This means that even
within a script, commands that come after `$env.config` changes will
always see those changes in `Stack::get_config()`.

Also fixed a lot of cases where `engine_state.get_config()` was used
even when `Stack` was available.

Closes #13324.

# User-Facing Changes
- Config changes apply immediately after the assignment is executed,
rather than whenever config is read by a command that needs it.
- Potentially slower performance when executing a lot of lines that
change `$env.config` one after another. Recommended to get `$env.config`
into a `mut` variable first and do modifications, then assign it back.
- Much faster performance when executing a script that made
modifications to `$env.config`, as the changes are only parsed once.

# Tests + Formatting
All passing.

# After Submitting
- [ ] release notes
2024-07-11 06:09:33 -07:00
Ian Manske
399a7c8836
Add and use new Signals struct (#13314)
# Description
This PR introduces a new `Signals` struct to replace our adhoc passing
around of `ctrlc: Option<Arc<AtomicBool>>`. Doing so has a few benefits:
- We can better enforce when/where resetting or triggering an interrupt
is allowed.
- Consolidates `nu_utils::ctrl_c::was_pressed` and other ad-hoc
re-implementations into a single place: `Signals::check`.
- This allows us to add other types of signals later if we want. E.g.,
exiting or suspension.
- Similarly, we can more easily change the underlying implementation if
we need to in the future.
- Places that used to have a `ctrlc` of `None` now use
`Signals::empty()`, so we can double check these usages for correctness
in the future.
2024-07-07 22:29:01 +00:00
Jack Wright
0d060aeae8
Use pipeline data for http post|put|patch|delete commands. (#13254)
# Description
Provides the ability to use http commands as part of a pipeline.
Additionally, this pull requests extends the pipeline metadata to add a
content_type field. The content_type metadata field allows commands such
as `to json` to set the metadata in the pipeline allowing the http
commands to use it when making requests.

This pull request also introduces the ability to directly stream http
requests from streaming pipelines.

One other small change is that Content-Type will always be set if it is
passed in to the http commands, either indirectly or throw the content
type flag. Previously it was not preserved with requests that were not
of type json or form data.

# User-Facing Changes
* `http post`, `http put`, `http patch`, `http delete` can be used as
part of a pipeline
* `to text`, `to json`, `from json` all set the content_type metadata
field and the http commands will utilize them when making requests.
2024-07-01 12:34:19 -07:00
Andrej Kolchin
2d0a60ac67
Use native toml datetime type in to toml (#13018)
# Description

Makes `to toml` use the `toml::value::Datetime` type, so that `to toml`
serializes dates properly.


# User-Facing Changes

`to toml` will now encode dates differently, in a native format instead
of a string. This could, in theory, break some workflows:

```Nushell
# Before:
~> {datetime: 2024-05-31} | to toml | from toml | get datetime | into datetime
Fri, 31 May 2024 00:00:00 +0000 (10 hours ago)
# After:
~> {datetime: 2024-05-31} | to toml | from toml | get datetime | into datetime
Error: nu:🐚:only_supports_this_input_type

  × Input type not supported.
   ╭─[entry #13:1:36]
 1 │ {datetime: 2024-05-31} | to toml | from toml | get datetime | into datetime
   ·                                    ────┬────                  ──────┬──────
   ·                                        │                            ╰── only string and int input data is supported
   ·                                        ╰── input type: date
   ╰────
```

Fix #11751
2024-06-07 07:43:30 -05:00
Devyn Cairns
28e33587d9
msgpackz: increase default compression level (#13035)
# Description
Increase default compression level for brotli on msgpackz to 3. This has
the best compression time generally. Level 0 and 1 give weird results
and sometimes cause extremely inflated outputs rather than being
compressed. So far this hasn't really been a problem for the plugin
registry file, but has been for other data.

The `$example` is the web-app example from https://json.org/example.html

Benchmarked with:

```nushell
seq 0 11 | each { |level|
  let compressed = ($example | to msgpackz --quality $level)
  let time = (timeit { $example | to msgpackz --quality $level })
  {
    level: $level
    time: $time
    length: ($compressed | bytes length)
    ratio: (($uncompressed_length | into float) / ($compressed | bytes length))
  }
}
```

```
╭────┬───────┬─────────────────┬────────┬───────╮
│  # │ level │      time       │ length │ ratio │
├────┼───────┼─────────────────┼────────┼───────┤
│  0 │     0 │ 4ms 611µs 875ns │   3333 │  0.72 │
│  1 │     1 │ 1ms 334µs 500ns │   3333 │  0.72 │
│  2 │     2 │     190µs 333ns │   1185 │  2.02 │
│  3 │     3 │      184µs 42ns │   1128 │  2.12 │
│  4 │     4 │      245µs 83ns │   1098 │  2.18 │
│  5 │     5 │     265µs 584ns │   1040 │  2.30 │
│  6 │     6 │     270µs 792ns │   1040 │  2.30 │
│  7 │     7 │     444µs 708ns │   1040 │  2.30 │
│  8 │     8 │       1ms 801µs │   1040 │  2.30 │
│  9 │     9 │     843µs 875ns │   1037 │  2.31 │
│ 10 │    10 │ 4ms 128µs 375ns │    984 │  2.43 │
│ 11 │    11 │ 6ms 352µs 834ns │    986 │  2.43 │
╰────┴───────┴─────────────────┴────────┴───────╯
```

cc @maxim-uvarov
2024-06-04 17:19:10 -07:00
Antoine Büsch
65911c125c
Try to preserve the ordering of elements in from toml (#13045)
# Description

Enable the `preserve_order` feature of the `toml` crate to preserve the
ordering of elements when converting from/to toml.

Additionally, use `to_string_pretty()` instead of `to_string()` in `to
toml`. This displays arrays on multiple lines instead of one big single
line. I'm not sure if this one is a good idea or not... Happy to remove
this from this PR if it's not.

# User-Facing Changes
The order of elements will be different when using `from toml`. The
formatting of arrays will also be different when using `to toml`. For
example:

- before
```
❯ "foo=1\nbar=2\ndoo=3" | from toml
╭─────┬───╮
│ bar │ 2 │
│ doo │ 3 │
│ foo │ 1 │
╰─────┴───╯
❯ {a: [a b c d]} | to toml
a = ["a", "b", "c", "d"]
```
- after
```
❯ "foo=1\nbar=2\ndoo=3" | from toml
╭─────┬───╮
│ foo │ 1 │
│ bar │ 2 │
│ doo │ 3 │
╰─────┴───╯
❯ {a: [a b c d]} | to toml
a = [
    "a",
    "b",
    "c",
    "d",
]
```

# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🔴 `toolkit test`
-  `toolkit test stdlib`

# 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.
-->
2024-06-05 08:00:39 +08:00
Devyn Cairns
7de513a4e0
Implement streaming I/O for CSV and TSV commands (#12918)
# Description

Implements streaming for:

- `from csv`
- `from tsv`
- `to csv`
- `to tsv`

via the new string-typed ByteStream support.

# User-Facing Changes
Commands above. Also:

- `to csv` and `to tsv` now have `--columns <List(String)>`, to provide
the exact columns desired in the output. This is required for them to
have streaming output, because otherwise collecting the entire list is
necessary to determine the output columns. If we introduce
`TableStream`, this may become less necessary.

# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`

# After Submitting
- [ ] release notes

---------

Co-authored-by: Ian Manske <ian.manske@pm.me>
2024-05-22 16:55:24 +00:00
Devyn Cairns
c61075e20e
Add string/binary type color to ByteStream (#12897)
# Description

This PR allows byte streams to optionally be colored as being
specifically binary or string data, which guarantees that they'll be
converted to `Binary` or `String` appropriately on `into_value()`,
making them compatible with `Type` guarantees. This makes them
significantly more broadly usable for command input and output.

There is still an `Unknown` type for byte streams coming from external
commands, which uses the same behavior as we previously did where it's a
string if it's UTF-8.

A small number of commands were updated to take advantage of this, just
to prove the point. I will be adding more after this merges.

# User-Facing Changes
- New types in `describe`: `string (stream)`, `binary (stream)`
- These commands now return a stream if their input was a stream:
  - `into binary`
  - `into string`
  - `bytes collect`
  - `str join`
  - `first` (binary)
  - `last` (binary)
  - `take` (binary)
  - `skip` (binary)
- Streams that are explicitly binary colored will print as a streaming
hexdump
  - example:
    ```nushell
    1.. | each { into binary } | bytes collect
    ```

# Tests + Formatting
I've added some tests to cover it at a basic level, and it doesn't break
anything existing, but I do think more would be nice. Some of those will
come when I modify more commands to stream.

# After Submitting
There are a few things I'm not quite satisfied with:

- **String trimming behavior.** We automatically trim newlines from
streams from external commands, but I don't think we should do this with
internal commands. If I call a command that happens to turn my string
into a stream, I don't want the newline to suddenly disappear. I changed
this to specifically do it only on `Child` and `File`, but I don't know
if this is quite right, and maybe we should bring back the old flag for
`trim_end_newline`
- **Known binary always resulting in a hexdump.** It would be nice to
have a `print --raw`, so that we can put binary data on stdout
explicitly if we want to. This PR doesn't change how external commands
work though - they still dump straight to stdout.

Otherwise, here's the normal checklist:

- [ ] release notes
- [ ] docs update for plugin protocol changes (added `type` field)

---------

Co-authored-by: Ian Manske <ian.manske@pm.me>
2024-05-20 00:35:32 +00:00
Ian Manske
baeba19b22
Make get_full_help take &dyn Command (#12903)
# Description
Changes `get_full_help` to take a `&dyn Command` instead of multiple
arguments (`&Signature`, `&Examples` `is_parser_keyword`). All of these
arguments can be gathered from a `Command`, so there is no need to pass
the pieces to `get_full_help`.

This PR also fixes an issue where the search terms are not shown if
`--help` is used on a command.
2024-05-19 19:56:33 +02:00
Ian Manske
cc9f41e553
Use CommandType in more places (#12832)
# Description
Kind of a vague title, but this PR does two main things:
1. Rather than overriding functions like `Command::is_parser_keyword`,
this PR instead changes commands to override `Command::command_type`.
The `CommandType` returned by `Command::command_type` is then used to
automatically determine whether `Command::is_parser_keyword` and the
other `is_{type}` functions should return true. These changes allow us
to remove the `CommandType::Other` case and should also guarantee than
only one of the `is_{type}` functions on `Command` will return true.
2. Uses the new, reworked `Command::command_type` function in the `scope
commands` and `which` commands.


# User-Facing Changes
- Breaking change for `scope commands`: multiple columns (`is_builtin`,
`is_keyword`, `is_plugin`, etc.) have been merged into the `type`
column.
- Breaking change: the `which` command can now report `plugin` or
`keyword` instead of `built-in` in the `type` column. It may also now
report `external` instead of `custom` in the `type` column for known
`extern`s.
2024-05-18 23:37:31 +00:00
Ian Manske
6fd854ed9f
Replace ExternalStream with new ByteStream type (#12774)
# Description
This PR introduces a `ByteStream` type which is a `Read`-able stream of
bytes. Internally, it has an enum over three different byte stream
sources:
```rust
pub enum ByteStreamSource {
    Read(Box<dyn Read + Send + 'static>),
    File(File),
    Child(ChildProcess),
}
```

This is in comparison to the current `RawStream` type, which is an
`Iterator<Item = Vec<u8>>` and has to allocate for each read chunk.

Currently, `PipelineData::ExternalStream` serves a weird dual role where
it is either external command output or a wrapper around `RawStream`.
`ByteStream` makes this distinction more clear (via `ByteStreamSource`)
and replaces `PipelineData::ExternalStream` in this PR:
```rust
pub enum PipelineData {
    Empty,
    Value(Value, Option<PipelineMetadata>),
    ListStream(ListStream, Option<PipelineMetadata>),
    ByteStream(ByteStream, Option<PipelineMetadata>),
}
```

The PR is relatively large, but a decent amount of it is just repetitive
changes.

This PR fixes #7017, fixes #10763, and fixes #12369.

This PR also improves performance when piping external commands. Nushell
should, in most cases, have competitive pipeline throughput compared to,
e.g., bash.
| Command | Before (MB/s) | After (MB/s) | Bash (MB/s) |
| -------------------------------------------------- | -------------:|
------------:| -----------:|
| `throughput \| rg 'x'` | 3059 | 3744 | 3739 |
| `throughput \| nu --testbin relay o> /dev/null` | 3508 | 8087 | 8136 |

# User-Facing Changes
- This is a breaking change for the plugin communication protocol,
because the `ExternalStreamInfo` was replaced with `ByteStreamInfo`.
Plugins now only have to deal with a single input stream, as opposed to
the previous three streams: stdout, stderr, and exit code.
- The output of `describe` has been changed for external/byte streams.
- Temporary breaking change: `bytes starts-with` no longer works with
byte streams. This is to keep the PR smaller, and `bytes ends-with`
already does not work on byte streams.
- If a process core dumped, then instead of having a `Value::Error` in
the `exit_code` column of the output returned from `complete`, it now is
a `Value::Int` with the negation of the signal number.

# After Submitting
- Update docs and book as necessary
- Release notes (e.g., plugin protocol changes)
- Adapt/convert commands to work with byte streams (high priority is
`str length`, `bytes starts-with`, and maybe `bytes ends-with`).
- Refactor the `tee` code, Devyn has already done some work on this.

---------

Co-authored-by: Devyn Cairns <devyn.cairns@gmail.com>
2024-05-16 07:11:18 -07:00
Ian Manske
e879d4ecaf
ListStream touchup (#12524)
# Description

Does some misc changes to `ListStream`:
- Moves it into its own module/file separate from `RawStream`.
- `ListStream`s now have an associated `Span`.
- This required changes to `ListStreamInfo` in `nu-plugin`. Note sure if
this is a breaking change for the plugin protocol.
- Hides the internals of `ListStream` but also adds a few more methods.
- This includes two functions to more easily alter a stream (these take
a `ListStream` and return a `ListStream` instead of having to go through
the whole `into_pipeline_data(..)` route).
  -  `map`: takes a `FnMut(Value) -> Value`
  - `modify`: takes a function to modify the inner stream.
2024-05-05 16:00:59 +00:00
Ian Manske
847646e44e
Remove lazy records (#12682)
# Description
Removes lazy records from the language, following from the reasons
outlined in #12622. Namely, this should make semantics more clear and
will eliminate concerns regarding maintainability.

# User-Facing Changes
- Breaking change: `lazy make` is removed.
- Breaking change: `describe --collect-lazyrecords` flag is removed.
- `sys` and `debug info` now return regular records.

# After Submitting
- Update nushell book if necessary.
- Explore new `sys` and `debug info` APIs to prevent them from taking
too long (e.g., subcommands or taking an optional column/cell-path
argument).
2024-05-03 08:36:10 +08:00
Devyn Cairns
adf38c7c76
Msgpack commands (#12664)
# Description

I thought about bringing `nu_plugin_msgpack` in, but that is MPL with a
clause that prevents other licenses, so rather than adapt that code I
decided to take a crack at just doing it straight from `rmp` to `Value`
without any `rmpv` in the middle. It seems like it's probably faster,
though I can't say for sure how much with the plugin overhead.

@IanManske I started on a `Read` implementation for `RawStream` but just
specialized to `from msgpack` here, but I'm thinking after release maybe
we can polish it up and make it a real one. It works!

# User-Facing Changes
New commands:

- `from msgpack`
- `from msgpackz`
- `to msgpack`
- `to msgpackz`

# Tests + Formatting
Pretty thorough tests added for the format deserialization, with a
roundtrip for serialization. Some example tests too for both `from
msgpack` and `to msgpack`.

- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`


# After Submitting
- [ ] update release notes
2024-04-26 06:23:16 -05:00
Ian Manske
9996e4a1f8
Shrink the size of Expr (#12610)
# Description
Continuing from #12568, this PR further reduces the size of `Expr` from
64 to 40 bytes. It also reduces `Expression` from 128 to 96 bytes and
`Type` from 32 to 24 bytes.

This was accomplished by:
- for `Expr` with multiple fields (e.g., `Expr::Thing(A, B, C)`),
merging the fields into new AST struct types and then boxing this struct
(e.g. `Expr::Thing(Box<ABC>)`).
- replacing `Vec<T>` with `Box<[T]>` in multiple places. `Expr`s and
`Expression`s should rarely be mutated, if at all, so this optimization
makes sense.

By reducing the size of these types, I didn't notice a large performance
improvement (at least compared to #12568). But this PR does reduce the
memory usage of nushell. My config is somewhat light so I only noticed a
difference of 1.4MiB (38.9MiB vs 37.5MiB).

---------

Co-authored-by: Stefan Holderbach <sholderbach@users.noreply.github.com>
2024-04-24 15:46:35 +00:00
Ian Manske
3b1d405b96
Remove the Value::Block case (#12582)
# Description
`Value` describes the types of first-class values that users and scripts
can create, manipulate, pass around, and store. However, `Block`s are
not first-class values in the language, so this PR removes it from
`Value`. This removes some unnecessary code, and this change should be
invisible to the user except for the change to `scope modules` described
below.

# User-Facing Changes
Breaking change: the output of `scope modules` was changed so that
`env_block` is now `has_env_block` which is a boolean value instead of a
`Block`.

# After Submitting
Update the language guide possibly.
2024-04-21 07:03:33 +02:00
Antoine Stevan
be5ed3290c
add "to nuon" enumeration of possible styles (#12591)
# Description
in order to change the style of the _serialized_ NUON data,
`nuon::to_nuon` takes three mutually exclusive arguments, `raw: bool`,
`tabs: Option<usize>` and `indent: Option<usize>` 🤔
this begs to use an enumeration with all possible alternatives, right?

this PR changes the signature of `nuon::to_nuon` to use `nuon::ToStyle`
which has three variants
- `Raw`: no newlines
- `Tabs(n: usize)`: newlines and `n` tabulations as indent
- `Spaces(n: usize)`: newlines and `n` spaces as indent

# User-Facing Changes
the signature of `nuon::to_nuon` changes from
```rust
to_nuon(
    input: &Value,
    raw: bool,
    tabs: Option<usize>,
    indent: Option<usize>,
    span: Option<Span>,
) -> Result<String, ShellError>
```
to
```rust
to_nuon(
    input: &Value,
    style: ToStyle,
    span: Option<Span>
) -> Result<String, ShellError>
```

# Tests + Formatting

# After Submitting
2024-04-20 11:40:52 +02:00
Antoine Stevan
55edef5dda
create nuon crate from from nuon and to nuon (#12553)
# Description
playing with the NUON format in Rust code in some plugins, we agreed
with the team it was a great time to create a standalone NUON format to
allow Rust devs to use this Nushell file format.

> **Note**
> this PR almost copy-pastes the code from
`nu_commands/src/formats/from/nuon.rs` and
`nu_commands/src/formats/to/nuon.rs` to `nuon/src/from.rs` and
`nuon/src/to.rs`, with minor tweaks to make then standalone functions,
e.g. remove the rest of the command implementations

### TODO
- [x] add tests
- [x] add documentation

# User-Facing Changes
devs will have access to a new crate, `nuon`, and two functions,
`from_nuon` and `to_nuon`
```rust
from_nuon(
    input: &str,
    span: Option<Span>,
) -> Result<Value, ShellError>
```
```rust
to_nuon(
    input: &Value,
    raw: bool,
    tabs: Option<usize>,
    indent: Option<usize>,
    span: Option<Span>,
) -> Result<String, ShellError>
```

# Tests + Formatting
i've basically taken all the tests from
`crates/nu-command/tests/format_conversions/nuon.rs` and converted them
to use `from_nuon` and `to_nuon` instead of Nushell commands
- i've created a `nuon_end_to_end` to run both conversions with an
optional middle value to check that all is fine

> **Note** 
> the `nuon::tests::read_code_should_fail_rather_than_panic` test does
give different results locally and in the CI...
> i've left it ignored with comments to help future us :)

# After Submitting
mention that in the release notes for sure!!
2024-04-19 13:54:16 +02:00
Devyn Cairns
2ae9ad8676
Copy-on-write for record values (#12305)
# Description
This adds a `SharedCow` type as a transparent copy-on-write pointer that
clones to unique on mutate.

As an initial test, the `Record` within `Value::Record` is shared.

There are some pretty big wins for performance. I'll post benchmark
results in a comment. The biggest winner is nested access, as that would
have cloned the records for each cell path follow before and it doesn't
have to anymore.

The reusability of the `SharedCow` type is nice and I think it could be
used to clean up the previous work I did with `Arc` in `EngineState`.
It's meant to be a mostly transparent clone-on-write that just clones on
`.to_mut()` or `.into_owned()` if there are actually multiple
references, but avoids cloning if the reference is unique.

# User-Facing Changes
- `Value::Record` field is a different type (plugin authors)

# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`

# After Submitting
- [ ] use for `EngineState`
- [ ] use for `Value::List`
2024-04-14 01:42:03 +00:00
Ian Manske
7a7d43344e
Range refactor (#12405)
# Description
Currently, `Range` is a struct with a `from`, `to`, and `incr` field,
which are all type `Value`. This PR changes `Range` to be an enum over
`IntRange` and `FloatRange` for better type safety / stronger compile
time guarantees.

Fixes: #11778 Fixes: #11777 Fixes: #11776 Fixes: #11775 Fixes: #11774
Fixes: #11773 Fixes: #11769.

# User-Facing Changes
Hopefully none, besides bug fixes.

Although, the `serde` representation might have changed.
2024-04-06 09:04:56 -05:00
Stefan Holderbach
910e3b52a7
Avoid uses of Record internals again (#12312)
# Description
Again avoid uses of the `Record` internals, so we are free to change the
data layout

- **Don't use internals of `Record` in `into sqlite`**
- **Don't use internals of `Record` in `to xml`**

Remaining: `rename`

# User-Facing Changes
None
2024-03-28 11:36:12 +01:00
Stefan Holderbach
b19da158d5
Rename Value::CustomValue to Value::Custom (#12309)
# Description
The second `Value` is redundant and will consume five extra bytes on
each transmission of a custom value to/from a plugin.

# User-Facing Changes
This is a breaking change to the plugin protocol.

The [example in the protocol
reference](https://www.nushell.sh/contributor-book/plugin_protocol_reference.html#value)
becomes

```json
{
  "Custom": {
    "val": {
      "type": "PluginCustomValue",
      "name": "database",
      "data": [36, 190, 127, 40, 12, 3, 46, 83],
      "notify_on_drop": true
    },
    "span": {
      "start": 320,
      "end": 340
    }
  }
}
```

instead of 

```json
{
  "CustomValue": {
    ...
  }
}
```


# After Submitting
Update plugin protocol reference
2024-03-27 22:10:56 +01:00
Ian Manske
c747ec75c9
Add command_prelude module (#12291)
# Description
When implementing a `Command`, one must also import all the types
present in the function signatures for `Command`. This makes it so that
we often import the same set of types in each command implementation
file. E.g., something like this:
```rust
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{
    record, Category, Example, IntoInterruptiblePipelineData, IntoPipelineData, PipelineData,
    ShellError, Signature, Span, Type, Value,
};
```

This PR adds the `nu_engine::command_prelude` module which contains the
necessary and commonly used types to implement a `Command`:
```rust
// command_prelude.rs
pub use crate::CallExt;
pub use nu_protocol::{
    ast::{Call, CellPath},
    engine::{Command, EngineState, Stack},
    record, Category, Example, IntoInterruptiblePipelineData, IntoPipelineData, IntoSpanned,
    PipelineData, Record, ShellError, Signature, Span, Spanned, SyntaxShape, Type, Value,
};
```

This should reduce the boilerplate needed to implement a command and
also gives us a place to track the breadth of the `Command` API. I tried
to be conservative with what went into the prelude modules, since it
might be hard/annoying to remove items from the prelude in the future.
Let me know if something should be included or excluded.
2024-03-26 21:17:30 +00:00
Filip Andersson
b70766e6f5
Boxes record for smaller Value enum. (#12252)
<!--
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
<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->
Boxes `Record` inside `Value` to reduce memory usage, `Value` goes from
`72` -> `56` bytes after this change.
# 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` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` 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.
-->
2024-03-26 17:17:44 +02:00
João Fidalgo
63335e99ae
Fix usage of --tabs flag while converting to json (#12115) (#12251)
closes #12115 

# Description
This fix addresses a bug where the --tabs flag couldn't be utilized due
to improper handling of the tab quantity provided by the user.
Previously, the code mistakenly attempted to convert the tab quantity to
a boolean value, leading to a conversion error. The resolution involves
adjusting the condition clauses to properly validate the presence of the
flag's value. Now, the code checks whether the get_flag() function
returns a value or None associated with the --tabs flag. This adjustment
enables the --tabs flag to function correctly, triggering the
appropriate condition and allowing the conversion to proceed as
expected. Similarly, the fix applies to the --indent flag. Additionally,
a default case was added, and the conversion now works properly without
flags. Two tests were added to validate the corrected behavior of these
flags.

# User-Facing Changes
Now the conversion should work properly instead of displaying an error.

# Tests + Formatting
-🟢 toolkit fmt
-🟢 toolkit clippy
-🟢 toolkit test
-🟢 toolkit test stdlib

To run added tests:
- cargo test --package nu-command --test main --
format_conversions::json::test_tabs_indent_flag
- cargo test --package nu-command --test main --
format_conversions::json::test_indent_flag
2024-03-20 11:55:51 -05:00
Devyn Cairns
6795ad7e33
Make custom value type handling more consistent (#12230)
[Context on
Discord](https://discord.com/channels/601130461678272522/855947301380947968/1219425984990806207)

# Description

- Rename `CustomValue::value_string()` to `type_name()` to reflect its
usage better.
- Change print behavior to always call `to_base_value()` first, to give
the custom value better control over the output.
- Change `describe --detailed` to show the type name as the subtype,
rather than trying to describe the base value.
- Change custom `Type` to use `type_name()` rather than `typetag_name()`
to make things like `PluginCustomValue` more transparent

One question: should `describe --detailed` still include a description
of the base value somewhere? I'm torn on it, it seems possibly useful
for some things (maybe sqlite databases?), but having `describe -d` not
include the custom type name anywhere felt weird. Another option would
be to add another method to `CustomValue` for info to be displayed in
`describe`, so that it can be more type-specific?

# User-Facing Changes
Everything above has implications for printing and `describe` on custom
values

# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`
2024-03-19 11:09:59 +01:00
Panagiotis Ganelis
f24877ba08
fix: process empty headers in to md command (#12012)
<!--
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!
-->
fixes #12006

# Description
<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->
Process empty headers as well in `to md` command.

# 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` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` 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.
-->
2024-02-28 20:44:11 -06:00
Wind
f7d647ac3c
open, rm, umv, cp, rm and du: Don't globs if inputs are variables or string interpolation (#11886)
# Description
This is a follow up to
https://github.com/nushell/nushell/pull/11621#issuecomment-1937484322

Also Fixes: #11838 

## About the code change
It applys the same logic when we pass variables to external commands:


0487e9ffcb/crates/nu-command/src/system/run_external.rs (L162-L170)

That is: if user input dynamic things(like variables, sub-expression, or
string interpolation), it returns a quoted `NuPath`, then user input
won't be globbed
 
# User-Facing Changes
Given two input files: `a*c.txt`, `abc.txt`

* `let f = "a*c.txt"; rm $f` will remove one file: `a*c.txt`. 
~* `let f = "a*c.txt"; rm --glob $f` will remove `a*c.txt` and
`abc.txt`~
* `let f: glob = "a*c.txt"; rm $f` will remove `a*c.txt` and `abc.txt`

## Rules about globbing with *variable*
Given two files: `a*c.txt`, `abc.txt`
| Cmd Type | example | Result |
| ----- | ------------------ | ------ |
| builtin | let f = "a*c.txt"; rm $f | remove `a*c.txt` |
| builtin | let f: glob = "a*c.txt"; rm $f | remove `a*c.txt` and
`abc.txt`
| builtin | let f = "a*c.txt"; rm ($f \| into glob) | remove `a*c.txt`
and `abc.txt`
| custom | def crm [f: glob] { rm $f }; let f = "a*c.txt"; crm $f |
remove `a*c.txt` and `abc.txt`
| custom | def crm [f: glob] { rm ($f \| into string) }; let f =
"a*c.txt"; crm $f | remove `a*c.txt`
| custom | def crm [f: string] { rm $f }; let f = "a*c.txt"; crm $f |
remove `a*c.txt`
| custom | def crm [f: string] { rm $f }; let f = "a*c.txt"; crm ($f \|
into glob) | remove `a*c.txt` and `abc.txt`

In general, if a variable is annotated with `glob` type, nushell will
expand glob pattern. Or else, we need to use `into | glob` to expand
glob pattern

# Tests + Formatting
Done

# After Submitting
I think `str glob-escape` command will be no-longer required. We can
remove it.
2024-02-23 09:17:09 +08:00
Ian Manske
1c49ca503a
Name the Value conversion functions more clearly (#11851)
# Description
This PR renames the conversion functions on `Value` to be more consistent.
It follows the Rust [API guidelines](https://rust-lang.github.io/api-guidelines/naming.html#ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv) for ad-hoc conversions.
The conversion functions on `Value` now come in a few forms:
- `coerce_{type}` takes a `&Value` and attempts to convert the value to
`type` (e.g., `i64` are converted to `f64`). This is the old behavior of
some of the `as_{type}` functions -- these functions have simply been
renamed to better reflect what they do.
- The new `as_{type}` functions take a `&Value` and returns an `Ok`
result only if the value is of `type` (no conversion is attempted). The
returned value will be borrowed if `type` is non-`Copy`, otherwise an
owned value is returned.
- `into_{type}` exists for non-`Copy` types, but otherwise does not
attempt conversion just like `as_type`. It takes an owned `Value` and
always returns an owned result.
- `coerce_into_{type}` has the same relationship with `coerce_{type}` as
`into_{type}` does with `as_{type}`.
- `to_{kind}_string`: conversion to different string formats (debug,
abbreviated, etc.). Only two of the old string conversion functions were
removed, the rest have been renamed only.
- `to_{type}`: other conversion functions. Currently, only `to_path`
exists. (And `to_string` through `Display`.)

This table summaries the above:
| Form | Cost | Input Ownership | Output Ownership | Converts `Value`
case/`type` |
| ---------------------------- | ----- | --------------- |
---------------- | -------- |
| `as_{type}` | Cheap | Borrowed | Borrowed/Owned | No |
| `into_{type}` | Cheap | Owned | Owned | No |
| `coerce_{type}` | Cheap | Borrowed | Borrowed/Owned | Yes |
| `coerce_into_{type}` | Cheap | Owned | Owned | Yes |
| `to_{kind}_string` | Expensive | Borrowed | Owned | Yes |
| `to_{type}` | Expensive | Borrowed | Owned | Yes |

# User-Facing Changes
Breaking API change for `Value` in `nu-protocol` which is exposed as
part of the plugin API.
2024-02-17 18:14:16 +00:00
dannou812
b0600449e5
Fixed to/from toml date format (#11846)
With this PR i try to resolve #11751 

# Description
I am rather new to Rust so if anything is not the way it should be
please let me know.

As described in the title I just fixed the date conversion in the to and
from toml commands as i thought it would be a good first issue. The
example of the original issue will now work as follows:

```
~> {date: 2024-02-02} | to toml
date = "2024-02-02T00:00:00+00:00"
~> "dob = 1979-05-27T07:32:00-08:00" | from toml
╭─────┬───────────────────────────╮
│ dob │ 44 years ago              |
╰─────┴───────────────────────────╯
```

The `from toml` command now returns a nushell date which is displayed as
`44 years ago` in this case.

# User-Facing Changes
none

# Tests + Formatting
all tests pass and formatting has been applied

---------

Co-authored-by: dannou812 <dannou281@gmail.com>
Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com>
2024-02-14 12:13:33 -06:00
WindSoilder
c59d6d31bc
do not attempt to glob expand if the file path is wrapped in quotes (#11569)
# Description
Fixes: #11455

### For arguments which is annotated with `:path/:directory/:glob`
To fix the issue, we need to have a way to know if a path is originally
quoted during runtime. So the information needed to be added at several
levels:
* parse time (from user input to expression)
We need to add quoted information into `Expr::Filepath`,
`Expr::Directory`, `Expr::GlobPattern`
* eval time
When convert from `Expr::Filepath`, `Expr::Directory`,
`Expr::GlobPattern` to `Value::String` during runtime, we won't auto
expanded the path if it's quoted

### For `ls`
It's really special, because it accepts a `String` as a pattern, and it
generates `glob` expression inside the command itself.

So the idea behind the change is introducing a special SyntaxShape to
ls: `SyntaxShape::LsGlobPattern`. So we can track if the pattern is
originally quoted easier, and we don't auto expand the path either.

Then when constructing a glob pattern inside ls, we check if input
pattern is quoted, if so: we escape the input pattern, so we can run `ls
a[123]b`, because it's already escaped.
Finally, to accomplish the checking process, we also need to introduce a
new value type called `Value::QuotedString` to differ from
`Value::String`, it's used to generate an enum called `NuPath`, which is
finally used in `ls` function. `ls` learned from `NuPath` to know if
user input is quoted.

# User-Facing Changes
Actually it contains several changes
### For arguments which is annotated with `:path/:directory/:glob`
#### Before
```nushell
> def foo [p: path] { echo $p }; print (foo "~/a"); print (foo '~/a')
/home/windsoilder/a
/home/windsoilder/a
> def foo [p: directory] { echo $p }; print (foo "~/a"); print (foo '~/a')
/home/windsoilder/a
/home/windsoilder/a
> def foo [p: glob] { echo $p }; print (foo "~/a"); print (foo '~/a')
/home/windsoilder/a
/home/windsoilder/a
```
#### After
```nushell
> def foo [p: path] { echo $p }; print (foo "~/a"); print (foo '~/a')
~/a
~/a
> def foo [p: directory] { echo $p }; print (foo "~/a"); print (foo '~/a')
~/a
~/a
> def foo [p: glob] { echo $p }; print (foo "~/a"); print (foo '~/a')
~/a
~/a
```
### For ls command
`touch '[uwu]'`
#### Before
```
❯ ls -D "[uwu]"
Error:   × No matches found for [uwu]
   ╭─[entry #6:1:1]
 1 │ ls -D "[uwu]"
   ·       ───┬───
   ·          ╰── Pattern, file or folder not found
   ╰────
  help: no matches found
```

#### After
```
❯ ls -D "[uwu]"
╭───┬───────┬──────┬──────┬──────────╮
│ # │ name  │ type │ size │ modified │
├───┼───────┼──────┼──────┼──────────┤
│ 0 │ [uwu] │ file │  0 B │ now      │
╰───┴───────┴──────┴──────┴──────────╯
```

# Tests + Formatting
Done

# After Submitting
NaN
2024-01-21 23:22:25 +08:00
Artemiy
ff290a5c3d
Add self-closed tag support for to xml (#11577)
# Description
This PR closes #11524
Add `to xml --self-closed` flag to output empty tags as self close.
For example:

![image](https://github.com/nushell/nushell/assets/17511668/bdf040f7-8ac1-4e8b-80bb-0043d7cec7f9)


# User-Facing Changes
New `to xml` flag `--self-closed`.

# Tests + Formatting
Added new example for `to xml` command and new test for self-closed
tags.
2024-01-19 05:35:29 -06:00
Michel Lind (né Salim)
5d63f47c85
Replace htmlescape with v_htmlescape (#11572)
# Description

`htmlescape` is unmaintained: https://crates.io/crates/htmlescape

while `v_htmlescape` is: https://crates.io/crates/v_htmlescape

and is used by two popular crates (`actix-files` and `minijinja`)

Let's use this instead - I'm packaging `nu` in Fedora and there is
understandable reluctance in bringing in an unmaintained crate if we can
avoid it.

# User-Facing Changes
Should not be any; drop-in replacement

# Tests + Formatting
Tested using:
- `cargo build` in the root folder (needed by some `nu-command` tests)
- `cargo test --features sqlite` in `crates/nu-command`
(`tests/commands/database/into_sqlite.rs` needs `rusqlite`)
- `cargo test` in `crates/nu-cmd-extra`

# 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.
-->
N/A

Signed-off-by: Michel Lind <salimma@fedoraproject.org>
2024-01-18 12:58:35 -06:00
Artemiy
e4c2c123ab
Support for disabling automatic escaping in to xml (#11536)
# Description
This PR addresses #11525 by adding `--partial-escape` which makes `to
xml` only escape `<>&` in text and `<>&"` in comments. This PR also
fixes issue where comment and PI content was escaped even though [it
should not be](https://stackoverflow.com/a/46637835)

# User-Facing Changes
Correct comments and PIs
 `to xml --partial-escape` flag to emit less escaped characters

# Tests + Formatting
Added tests for specified issues
2024-01-14 07:36:53 -06:00
Artemiy
1867bb1a88
Fix incorrect handling of boolean flags for builtin commands (#11492)
# Description
Possible fix of #11456
This PR fixes a bug where builtin commands did not respect the logic of
dynamically passed boolean flags. The reason is
[has_flag](6f59abaf43/crates/nu-protocol/src/ast/call.rs (L204C5-L212C6))
method did not evaluate and take into consideration expression used with
flag.

To address this issue a solution is proposed:
1. `has_flag` method is moved to `CallExt` and new logic to evaluate
expression and check if it is a boolean value is added
2. `has_flag_const` method is added to `CallExt` which is a constant
version of `has_flag`
3. `has_named` method is added to `Call` which is basically the old
logic of `has_flag`
4. All usages of `has_flag` in code are updated, mostly to pass
`engine_state` and `stack` to new `has_flag`. In `run_const` commands it
is replaced with `has_flag_const`. And in a few select places: parser,
`to nuon` and `into string` old logic via `has_named` is used.

# User-Facing Changes
Explicit values of boolean flags are now respected in builtin commands.
Before:

![image](https://github.com/nushell/nushell/assets/17511668/f9fbabb2-3cfd-43f9-ba9e-ece76d80043c)
After:

![image](https://github.com/nushell/nushell/assets/17511668/21867596-2075-437f-9c85-45563ac70083)

Another example:
Before:

![image](https://github.com/nushell/nushell/assets/17511668/efdbc5ca-5227-45a4-ac5b-532cdc2bbf5f)
After:

![image](https://github.com/nushell/nushell/assets/17511668/2907d5c5-aa93-404d-af1c-21cdc3d44646)


# Tests + Formatting
Added test reproducing some variants of original issue.
2024-01-11 17:19:48 +02:00
Artemiy
5f7425a7b4
Xml errors fix (#11487)
# Description
Fixes #11264
This PR adds checks in `to xml` to output error for malformed xml
entries:
* With columns that are not one of `tag`, `attributes` or `content`
* With no `tag` when entry is not a string
* With `tag` that is not a string
This PR also replaces `attrs` with `attributes` in example and
extra_usage of `to xml` (column was originally named attrs and renamed
to attributes, but this was missed in docs)

# User-Facing Changes
`to xml` will produce error for conditions described above instead of
silently returning nothing

# Tests + Formatting
Added tests for `to xml` to check handling of malformed xml entries
2024-01-05 15:56:13 -06:00
Ian Manske
c9c93f5b4d
Remove Value::MatchPattern (#11356)
# Description
`Value::MatchPattern` implies that `MatchPattern`s are first-class
values. This PR removes this case, and commands must now instead use
`Expr::MatchPattern` to extract `MatchPattern`s just like how the
`match` command does using `Expr::MatchBlock`.

# User-Facing Changes
Breaking API change for `nu_protocol` crate.
2023-12-18 07:25:34 +13:00