move some cmds to helpers#3
Merged
sophiajt merged 3 commits intosophiajt:move_value_to_helpersfrom Aug 27, 2023
Merged
Conversation
sophiajt
reviewed
Aug 26, 2023
sophiajt
reviewed
Aug 26, 2023
sophiajt
reviewed
Aug 26, 2023
sophiajt
pushed a commit
that referenced
this pull request
Sep 29, 2023
…ushell#10430) should close nushell#10406 # Description when writing a script, with variables you try to `ls` or `open`, you will get a "directory not found" error but the variable won't be expanded and you won't be able to see which one of the variable was the issue... this PR adds this information to the error. # User-Facing Changes let's define a variable ```nushell let does_not_exist = "i_do_not_exist_in_the_current_directory" ``` ### before ```nushell > open $does_not_exist Error: nu::shell::directory_not_found × Directory not found ╭─[entry #7:1:1] 1 │ open $does_not_exist · ───────┬─────── · ╰── directory not found ╰──── ``` ```nushell > ls $does_not_exist Error: nu:🐚:directory_not_found × Directory not found ╭─[entry nushell#8:1:1] 1 │ ls $does_not_exist · ───────┬─────── · ╰── directory not found ╰──── ``` ### after ```nushell > open $does_not_exist Error: nu:🐚:directory_not_found × Directory not found ╭─[entry #3:1:1] 1 │ open $does_not_exist · ───────┬─────── · ╰── directory not found ╰──── help: /home/amtoine/documents/repos/github.com/amtoine/nushell/i_do_not_exist_in_the_current_directory does not exist ``` ```nushell > ls $does_not_exist Error: nu:🐚:directory_not_found × Directory not found ╭─[entry #4:1:1] 1 │ ls $does_not_exist · ───────┬─────── · ╰── directory not found ╰──── help: /home/amtoine/documents/repos/github.com/amtoine/nushell/i_do_not_exist_in_the_current_directory does not exist ``` # Tests + Formatting shouldn't harm anything 🤞 # After Submitting
sophiajt
pushed a commit
that referenced
this pull request
Oct 2, 2023
should close nushell#10549 # Description this PR is twofold - uses `to nuon --raw` in the error messages to make sure nushell#10549 is solved and makes a difference between `"1"` and `1` - tries to introduce slightly better errors, i.e. by putting left / right on new lines => this should hopefully help when the values become a bit big 😋 # User-Facing Changes the original issue: ```nushell > assert equal {one:1 two:2} {one:"1" two:"2"} Error: × Assertion failed. ╭─[entry #3:1:1] 1 │ assert equal {one:1 two:2} {one:"1" two:"2"} · ───────────────┬─────────────── · ╰── These are not equal. Left : '{one: 1, two: 2}' Right : '{one: "1", two: "2"}' ╰──── ``` a sample for all the assertions and their new messages ```nushell > assert equal {one:1 two:2} {one:"1" two:"2"} Error: × Assertion failed. ╭─[entry #3:1:1] 1 │ assert equal {one:1 two:2} {one:"1" two:"2"} · ───────────────┬─────────────── · ╰── These are not equal. Left : '{one: 1, two: 2}' Right : '{one: "1", two: "2"}' ╰──── ``` ```nushell > assert equal 1 2 Error: × Assertion failed. ╭─[entry #4:1:1] 1 │ assert equal 1 2 · ─┬─ · ╰── These are not equal. Left : '1' Right : '2' ╰──── ``` ```nushell > assert less 3 1 Error: × Assertion failed. ╭─[entry #6:1:1] 1 │ assert less 3 1 · ─┬─ · ╰── The condition *left < right* is not satisfied. Left : '3' Right : '1' ╰──── ``` ```nushell > assert less or equal 3 1 Error: × Assertion failed. ╭─[entry #7:1:1] 1 │ assert less or equal 3 1 · ─┬─ · ╰── The condition *left <= right* is not satisfied. Left : '3' Right : '1' ╰──── ``` ```nushell > assert greater 1 3 Error: × Assertion failed. ╭─[entry nushell#8:1:1] 1 │ assert greater 1 3 · ─┬─ · ╰── The condition *left > right* is not satisfied. Left : '1' Right : '3' ╰──── ``` ```nushell > assert greater or equal 1 3 Error: × Assertion failed. ╭─[entry nushell#9:1:1] 1 │ assert greater or equal 1 3 · ─┬─ · ╰── The condition *left < right* is not satisfied. Left : '1' Right : '3' ╰──── ``` ```nushell > assert length [1 2 3] 2 Error: × Assertion failed. ╭─[entry nushell#10:1:1] 1 │ assert length [1 2 3] 2 · ────┬──── · ╰── This does not have the correct length: value : [1, 2, 3] length : 3 expected : 2 ╰──── ``` ```nushell > assert length [1 "2" 3] 2 Error: × Assertion failed. ╭─[entry nushell#11:1:1] 1 │ assert length [1 "2" 3] 2 · ─────┬───── · ╰── This does not have the correct length: value : [1, "2", 3] length : 3 expected : 2 ╰──── ``` ```nushell > assert str contains "foo" "bar" Error: × Assertion failed. ╭─[entry nushell#13:1:1] 1 │ assert str contains "foo" "bar" · ─────┬───── · ╰── This does not contain '($right)'. value: "foo" ╰──── ``` # Tests + Formatting # After Submitting
WindSoilder
pushed a commit
that referenced
this pull request
Dec 5, 2023
# Description
## ❌ before this PR
```
>_ std help modules euwioq
Error: nu:🐚:external_command
× External command failed
╭─[help:259:1]
259 │ if ($found_module | is-empty) {
260 │ module_not_found_error (metadata $module | get span)
· ──────────────┬──────────────
· ╰── Cannot convert record<start: int, end: int> to a string
261 │ }
╰────
help: All arguments to an external command need to be string-compatible
```
```
>_ std help externs euwioq
Error:
× std::help::extern_not_found
╭─[help:401:1]
401 │
402 │ let extern = ($extern | str join " ")
· ─┬─
· ╰── extern not found
403 │
╰────
```
> **Note**
> same kind of error with all the others
## ✔️ after this PR
```
> std help modules euwioq 04/28/2023 05:45:50 PM
Error:
× std::help::module_not_found
╭─[entry #2:1:1]
1 │ std help modules euwioq
· ───┬──
· ╰── module not found
╰────
```
```
> std help externs euwioq 04/28/2023 05:45:53 PM
Error:
× std::help::extern_not_found
╭─[entry #3:1:1]
1 │ std help externs euwioq
· ───┬──
· ╰── extern not found
╰────
```
> **Note**
> same with the others
# User-Facing Changes
fixes the errors to have proper messages
# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- ⚫ `toolkit test`
- ⚫ `toolkit test stdlib`
# After Submitting
```
$nothing
```
WindSoilder
pushed a commit
that referenced
this pull request
Dec 5, 2023
Closes nushell#9003. This PR changes `group-by` so that its optional argument is interpreted as a cell path. In turn, this lets users use `?` to ignore rows that are missing the column they wish to group on. For example: ``` > [{foo: 123}, {foo: 234}, {bar: 345}] | group-by foo Error: nu::shell::column_not_found × Cannot find column ╭─[entry #3:1:1] 1 │ [{foo: 123}, {foo: 234}, {bar: 345}] | group-by foo · ─────┬──── ─┬─ · │ ╰── cannot find column 'foo' · ╰── value originates here ╰──── > [{foo: 123}, {foo: 234}, {bar: 345}] | group-by foo? ╭─────┬───────────────╮ │ 123 │ [table 1 row] │ │ 234 │ [table 1 row] │ ╰─────┴───────────────╯ ``` ~~This removes the ability to pass `group-by` a closure or block (I wasn't able to figure out how to make the 2 features coexist), and so it is a breaking change. I think this is OK; I didn't even know `group-by` could accept a closure or block because there was no example for that functionality.~~
WindSoilder
added a commit
that referenced
this pull request
Dec 5, 2023
… file (nushell#9368) # Description It's not a good idea to save `stdout` and `stderr` to the same file from `save` command directly. Because it saves `stdout` and `stderr` in different thread, which leads to in-consistent output. As replace, we can use `o+e` redirection to fix the issue # User-Facing Changes ``` ❯ do -i { "aa" } | save foo.txt -e foo.txt Error: × input and stderr input to same file ╭─[entry #3:1:1] 1 │ do -i { "aa" } | save foo.txt -e foo.txt · ───┬─── · ╰── can't save both input and stderr input to the same file ╰──── help: you should use `o+e> file` instead ``` # 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. -->
WindSoilder
pushed a commit
that referenced
this pull request
Dec 5, 2023
# Description Fixes a small bug with `rm` where names of files which couldn't be deleted due to error were not printed. Fixes nushell#9004 # User-Facing Changes Slightly different error message than previously. Nothing significant, though. The new error message looks like this ``` ~/Projects/rust/nushell> rm /proc/1/mem 05/06/2023 01:13:23 PM Error: nu::shell::remove_not_possible × Remove not possible ╭─[entry #3:1:1] 1 │ rm /proc/1/mem · ─────┬───── · ╰── Could not delete /proc/1/mem: Operation not permitted (os error 1) ╰──── ``` or when using a glob (only showing a single entry for brevity) ``` Error: nu:🐚:remove_not_possible × Remove not possible ╭─[entry #2:1:1] 1 │ rm --recursive --force --verbose /proc/1/* · ────┬──── · ╰── Could not delete /proc/1/comm: Operation not permitted (os error 1) ╰──── ``` # Tests + Formatting No new unit tests were added for this change as it is pretty difficult to test this particular case. However, manual testing was run with the following commands ``` rm /proc/1/mem rm --recursive --force --verbose /proc/1/* ``` # After Submitting N/A
sophiajt
pushed a commit
that referenced
this pull request
Jan 29, 2024
# Description
Currently, when writing a record, if you don't give the value for a
field, the syntax error highlights the entire record instead of
pinpointing the issue. Here's some examples:
```nushell
> { a: 2, 3 } # Missing colon (and value)
Error: nu::parser::parse_mismatch
× Parse mismatch during operation.
╭─[entry #2:1:1]
1 │ { a: 2, 3 }
· ─────┬─────
· ╰── expected record
╰────
> { a: 2, 3: } # Missing value
Error: nu::parser::parse_mismatch
× Parse mismatch during operation.
╭─[entry #3:1:1]
1 │ { a: 2, 3: }
· ──────┬─────
· ╰── expected record
╰────
> { a: 2, 3 4 } # Missing colon
Error: nu::parser::parse_mismatch
× Parse mismatch during operation.
╭─[entry #4:1:1]
1 │ { a: 2, 3 4 }
· ──────┬──────
· ╰── expected record
╰────
```
In all of them, the entire record is highlighted red because an
`Expr::Garbage` is returned covering that whole span:

This PR is for highlighting only the part inside the record that could
not be parsed. If the record literal is big, an error message pointing
to the start of where the parser thinks things went wrong should help
people fix their code.
# User-Facing Changes
Below are screenshots of the new errors:
If there's a stray record key right before the record ends, it
highlights only that key and tells the user it expected a colon after
it:

If the record ends before the value for the last field was given, it
highlights the key and colon of that field and tells the user it
expected a value after the colon:

If there are two consecutive expressions without a colon between them,
it highlights everything from the second expression to the end of the
record and tells the user it expected a colon. I was tempted to add a
help message suggesting adding a colon in between, but that may not
always be the right thing to do.

# Tests + Formatting
# After Submitting
sophiajt
pushed a commit
that referenced
this pull request
Jan 29, 2024
<!-- 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! --> Closes nushell#11561 # 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. --> This PR will allow string interpolation at parse time. Since the actual config hasn't been loaded at parse time, this uses the `get_config()` method on `StateWorkingSet`. So file sizes and datetimes (I think those are the only things whose string representations depend on the config) may be formatted differently from how users have configured things, which may come as a surprise to some. It does seem unlikely that anyone would be formatting file sizes or date times at parse time. Still, something to think about if/before this PR merged. Also, I changed the `ModuleNotFound` error to include the name of the module. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> Users will be able to do stuff like: ```nu const x = [1 2 3] const y = $"foo($x)" // foo[1, 2, 3] ``` The main use case is `use`-ing and `source`-ing files at parse time: ```nu const file = "foo.nu" use $"($file)" ``` If the module isn't found, you'll see an error like this: ``` Error: nu::parser::module_not_found × Module not found. ╭─[entry #3:1:1] 1 │ use $"($file)" · ─────┬──── · ╰── module foo.nu not found ╰──── help: module files and their paths must be available before your script is run as parsing occurs before anything is evaluated ``` # 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. --> Although there's user-facing changes, there's probably no need to change the docs since people probably already expect string interpolation to work at parse time. Edit: @kubouch pointed out that we'd need to document the fact that stuff like file sizes and datetimes won't get formatted according to user's runtime configs, so I'll make a PR to nushell.github.io after this one
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
(description of your pull request here)
Tests
Make sure you've run and fixed any issues with these commands:
cargo fmt --all -- --checkto check standard code formatting (cargo fmt --allapplies these changes)cargo clippy --all --all-features -- -D warnings -D clippy::unwrap_used -A clippy::needless_collectto check that you're using the standard code stylecargo build; cargo test --all --all-featuresto check that all the tests pass