Sort completions by match algorithm by ysthakur · Pull Request #4 · ysthakur/nushell

ysthakur · 2024-07-04T20:06:13Z

Description

Just want to see the workflows run

User-Facing Changes

Tests + Formatting

After Submitting

…ell#14073)  # Description  Swagger supports lists (a.k.a arrays) in query parameters: https://swagger.io/docs/specification/v3_0/serialization/ It supports three different styles: - explode=true - spaceDelimited - pipeDelimited With explode=true being the default and hence most common. It is the hardest to use inside of nushell, as the others are just a `string join` away. This commit adds lists with the explode=true format. # User-Facing Changes  Before: : {a[]: [one two three], b: four} | url build-query Error: nu::shell::unsupported_input × Unsupported input ╭─[entry nushell#33:1:1] 1 │ {a[]: [one two three], b: four} | url build-query · ───────────────┬─────────────── ───────┬─────── · │ ╰── Expected a record with string values · ╰── value originates from here ╰──── After: : {a[]: [one two three], b: four} | url build-query a%5B%5D=one&a%5B%5D=two&a%5B%5D=three&b=four Despite reading CONTRIBUTING.md I didn't get approval before making the change. My judgment is that this doesn't qualify as being "change something significantly". # Tests + Formatting I added the Example instance for the automatic tests. I couldn't figure out how to add an Example for the error case, so I did that with manual testing. E.g.: : {a[]: [one two [three]], b: four} | url build-query Error: nu:🐚:unsupported_input × Unsupported input ╭─[entry #3:1:1] 1 │ {a[]: [one two [three]], b: four} | url build-query · ────────────────┬──────────────── ───────┬─────── · │ ╰── Expected a record with list of string values · ╰── value originates from here ╰──── : {a[]: [one two 3hr], b: four} | url build-query Error: nu:🐚:unsupported_input × Unsupported input ╭─[entry #4:1:1] 1 │ {a[]: [one two 3hr], b: four} | url build-query · ──────────────┬────────────── ───────┬─────── · │ ╰── Expected a record with list of string values · ╰── value originates from here ╰────  I ran the four cargo commands on my local machine. I had to run the tests with: LANG=C and -j 1 and even then I got one failure: thread 'commands::umkdir::mkdir_umask_permission' panicked at crates/nu-command/tests/commands/umkdir.rs:148:9: assertion `left == right` failed: Most *nix systems have 0o00022 as the umask. So directory permission should be 0o40755 = 0o 40777 & (!0o00022) left: 16893 right: 16877 but this isn't related to this change (I seem to not be running most *nix system; and don't have a lot of RAM for the number of cores). The other three cargo commands didn't have errors or warnings. # After Submitting  I will add the new example to [the documentation](https://github.com/nushell/nushell.github.io). # Open questions / possible future work Things I noticed, and would like to mention and am open to adding, but don't think I am deep enough in nushell to do them pro-actively. ## Add an argument for the other query parameter list styles I don't know how frequent they are and I currently don't need them, so following KISS I didn't add them. ## long input_span marked In e.g.: : {a[]: [one two 3hr], b: four} | url build-query Error: nu::shell::unsupported_input × Unsupported input ╭─[entry #4:1:1] 1 │ {a[]: [one two 3hr], b: four} | url build-query · ──────────────┬────────────── ───────┬─────── · │ ╰── Expected a record with list of string values · ╰── value originates from here ╰──── the entire record is marked as input_span instead of just the "3hr" that is causing the problem. Changing that would be trivial, but I'm not deep enough into nushell to understand all the consequences of changing that. ## Error message says string values despite accepting numbers etc. The error message said it only accepted strings despite accepting numbers etc. (anything it can coerce into string). I couldn't find a good wording myself and that was how it was before. I simply added a "list of strings".

@fdncred

# Description  Before this PR, you can access rendered error values that are raised in a `try/catch` block by accessing the `rendered` element of the catch error value: ``` $ try { ls nonexist.txt } catch {|e| print "my cool error:" $e.rendered } my cool error: nu::shell::directory_not_found × Directory not found help: /home/rose/nonexist.txt does not exist ``` However, the rendered errors don't include the labels present in the real rendered error, which would look like this: ``` $ ls nonexist.txt Error: nu::shell::directory_not_found × Directory not found ╭─[entry nushell#46:1:4] 1 │ ls nonexist.txt · ──────┬───── · ╰── directory not found ╰──── help: /home/rose/nonexist.txt does not exist ``` After this PR, the rendered error includes the labels: ``` $ try { ls nonexist.txt } catch {|e| print "my cool error:" $e.rendered } my cool error: Error: nu:🐚:directory_not_found × Directory not found ╭─[entry #4:1:10] 1 │ try { ls nonexist.txt } catch {|e| print "my cool error:" $e.rendered } · ──────┬───── · ╰── directory not found ╰──── help: /home/rose/nonexist.txt does not exist ``` This change is accomplished by using the standard error formatting code to render an error. This respects the error theme as before without any extra scaffolding, but it means that e.g., the terminal size is also respected. I think this is fine because the way the error is rendered already changed based on config, and I think that a "rendered" error should give back _exactly_ what would be shown to the user anyway. @fdncred, let me know if you have any concerns with the way this is handled since you were the one who implemented this feature in the first place. # User-Facing Changes  The `rendered` element of the `try`/`catch` error record now includes labels in the error output. # Tests + Formatting  - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - 🟢 `toolkit test` - 🟢 `toolkit test stdlib` # After Submitting  N/A

Closes nushell#15543 # Description 1. Simplify code in ``datetime.rs`` based on a suggestion in my last PR on "datetime from record" 1. Make ``into duration`` work with durations inside a record, provided as a cell path 1. Make ``into duration`` work with durations as record # User-Facing Changes ```nushell # Happy paths ~> {d: '1hr'} | into duration d ╭───┬─────╮ │ d │ 1hr │ ╰───┴─────╯ ~> {week: 10, day: 2, sign: '+'} | into duration 10wk 2day # Error paths and invalid usage ~> {week: 10, day: 2, sign: 'x'} | into duration Error: nu::shell::incorrect_value × Incorrect value. ╭─[entry #4:1:26] 1 │ {week: 10, day: 2, sign: 'x'} | into duration · ─┬─ ──────┬────── · │ ╰── encountered here · ╰── Invalid sign. Allowed signs are +, - ╰──── ~> {week: 10, day: -2, sign: '+'} | into duration Error: nu:🐚:incorrect_value × Incorrect value. ╭─[entry #5:1:17] 1 │ {week: 10, day: -2, sign: '+'} | into duration · ─┬ ──────┬────── · │ ╰── encountered here · ╰── number should be positive ╰──── ~> {week: 10, day: '2', sign: '+'} | into duration Error: nu:🐚:only_supports_this_input_type × Input type not supported. ╭─[entry nushell#6:1:17] 1 │ {week: 10, day: '2', sign: '+'} | into duration · ─┬─ ──────┬────── · │ ╰── only int input data is supported · ╰── input type: string ╰──── ~> {week: 10, unknown: 1} | into duration Error: nu:🐚:unsupported_input × Unsupported input ╭─[entry nushell#7:1:1] 1 │ {week: 10, unknown: 1} | into duration · ───────────┬────────── ──────┬────── · │ ╰── Column 'unknown' is not valid for a structured duration. Allowed columns are: week, day, hour, minute, second, millisecond, microsecond, nanosecond, sign · ╰── value originates from here ╰──── ~> {week: 10, day: 2, sign: '+'} | into duration --unit sec Error: nu:🐚:incompatible_parameters × Incompatible parameters. ╭─[entry #2:1:33] 1 │ {week: 10, day: 2, sign: '+'} | into duration --unit sec · ──────┬────── ─────┬──── · │ ╰── the units should be included in the record · ╰── got a record as input ╰──── ``` # Tests + Formatting - Add examples and integration tests for ``into duration`` - Add one test for ``into duration`` # After Submitting If this is merged in time, I'll update my PR on the "datetime handling highlights" for the release notes.

… commands to utilize that (nushell#16414) # Description - Implemented `FromValue` for `std::time::Duration`. - It only converts positive `Value::Duration` values, negative ones raise `ShellError::NeedsPositiveValue`. - Refactor `job recv` and `watch` commands to use this implementation rather than handling it ad-hoc. - Simplified `watch`'s `debounce` & `debounce-ms` and factored it to a function. Should make removing `debounce-ms` after its deprecation period ends. - `job recv` previously used a numeric cast (`i64 as u64`) which would result in extremely long duration values rather than raising an error when negative duration arguments were given. # User-Facing Changes Changes in error messages: - Providing the wrong type (bypassing parse time type checking): - Before ``` Error: nu::shell::type_mismatch × Type mismatch. ╭─[entry nushell#40:1:9] 1 │ watch . --debounce (1 | $in) {|| } · ──────────┬───────── · ╰── Debounce duration must be a duration ╰──── ``` - After ``` Error: nu:🐚:cant_convert × Can't convert to duration. ╭─[entry #2:1:9] 1 │ watch . --debounce (1 | $in) {|| } · ──────────┬───────── · ╰── can't convert int to duration ╰──── ``` - Providing a negative duration value: - Before ``` Error: nu:🐚:type_mismatch × Type mismatch. ╭─[entry nushell#41:1:9] 1 │ watch . --debounce -100ms {|| } · ────────┬──────── · ╰── Debounce duration is invalid ╰──── ``` - After ``` Error: nu:🐚:needs_positive_value × Negative value passed when positive one is required ╭─[entry #4:1:9] 1 │ watch . --debounce -100ms {|| } · ────────┬──────── · ╰── use a positive value ╰──── ```

This PR adds the `toolkit run pr` and `toolkit download pr` commands to `toolkit`. This is a more fleshed out version of the snippet shared in nushell#16633, with robust error handling and cross-platform unzip support. When using `toolkit run pr`, the script will also check if the most recent binary for that PR has already been downloaded, and if so it will run that instead. I tried to make the error reporting as good as a built-in command to see how difficult that would be, and with use of the `--head: oneof<>` trick, it turned out pretty good. With access to the call span, the workflow is very similar to when writing a built-in command. I also used a `Spanned`-like record, which helped as well. ```nushell toolkit run pr 16740 # => Error: nu::shell::error # => # => × Command not found # => ╭─[entry #4:1:26] # => 1 │ overlay use -pr toolkit; toolkit run pr 16740 # => · ───────┬────── # => · ╰── requires `gh` # => ╰──── # => help: Please install the `gh` commandline tool ``` <details> <summary>More error reporting notes</summary> In an earlier version of the script, `run pr` called `download pr` directly. I ended up changing the way this worked so that `run pr` could use the workflow_id. Here's a couple snippets I thought were neat from this older version.   **Passing span via `--head`:** ```nushell def download [--head: oneof<>] { let span = $head | default (metadata $head).span error make {msg: "a", label: {text: here, span: $span}} } def run [--head: oneof<>] { let span = (metadata $head).span download --head=$span } download # => Error: nu:🐚:error # => # => × a # => ╭─[entry nushell#6:1:1] # => 1 │ download # => · ────┬─── # => · ╰── here # => ╰──── run # => Error: nu:🐚:error # => # => × a # => ╭─[entry nushell#7:1:1] # => 1 │ run # => · ─┬─ # => · ╰── here # => ╰──── ``` **Using "spanned" number as CLI parameter and as internal caller parameter** ```nushell def download [number: oneof<int, record<item: int, span: record>>] { let number = match $number { {item: $_, span: $_} => $number, $val => {item: $number, span: (metadata $number).span} } error make {msg: "a", label: {text: here, span: $number.span}} } def run [number: int] { let number = {item: $number, span: (metadata $number).span} download $number } download 123 # => Error: nu:🐚:error # => # => × a # => ╭─[entry nushell#9:1:10] # => 1 │ download 123 # => · ─┬─ # => · ╰── here # => ╰──── run 123 # => Error: nu:🐚:error # => # => × a # => ╭─[entry nushell#10:1:5] # => 1 │ run 123 # => · ─┬─ # => · ╰── here # => ╰──── ``` </details> ## Release notes summary - What our users need to know The toolkit in the Nushell repository can now download and run PRs by downloading artifacts from CI runs. It can be run like this: ```nushell use toolkit toolkit run pr <number> ```

## User-facing Changes * New arguments! (`error make "hello"`) * New parts for `error_struct`! (`error make {inner: [] labels: [] ...}`) * Pipeline inputs for chained errors! (`try {error make foo} catch {error make bar}`) * Pipeline inputs for normal errors! (`"help" | error make`) * External errors! (`error make {src: {path: $nu.cofig-path} ...}`) * Backwards compatibility! ### Arguments and Inputs The main changes are in how the arguments are passed. Everything is still backwards compatible with the old `error make` commands, there's just a nice extra layer we get from the pipeline and a few new args (that were already added in nushell#17037). There are some new ways to (hopefully intentionally) cause an error, such as using a naked `error make`, pipelines from records and simple string input! #### Inputs Because `error make` will just make an error anyway, it can technically take any input to make an error, but only properly formatted record input will create a chain. the `x | f $in` pattern can be used for string input, if that is more comfortable. #### With no arguments This is a completely new way to do this, with no arguments the `error make` invocation is highlighted, along with a simple `originates from here` message. This makes normal errors very easy to create without any special message setup. ``` > error make Error: nu::shell::error × originates from here ╭─[entry #4:1:1] 1 │ error make · ────────── ╰──── ``` #### Create a single argument * With pipeline input: `{msg: foo} | error make` * With an argument: `error make {msg: foo}` * With a string argument: `error make foo` ``` Error: nu:🐚:error × foo ╭─[entry #2:1:12] 1 │ error make {msg: foo} · ────────── ╰──── ``` #### Chaining errors together These will automatically create a chain of errors, placing the pipeline as an `inner` to the argument. This can very easily be used to get a bit more detail in a try loop using the naked `error make`: ``` Error: nu:🐚:error × originates from here ╭─[source:1:31] 1 │ try {error make "foo"} catch {error make} · ────────── ╰──── Error: nu:🐚:error × foo ╭─[source:1:6] 1 │ try {error make "foo"} catch {error make} · ────────── ╰──── ``` Or with more complex errors: * With both, combining the errors: `{msg: foo} | error make bar` * With the raw error from try: `try {error make foo} catch {error make bar}` Both are equivalent to: * `error make {msg: bar inner: [{msg: foo}]}` ``` Error: nu:🐚:error × bar ╭─[entry #1:1:29] 1 │ try {error make foo} catch {error make bar} · ────────── ╰──── Error: nu:🐚:error × foo ╭─[entry #1:1:6] 1 │ try {error make foo} catch {error make bar} · ────────── ╰──── ``` ### Labels As is noticeable in the examples above, simple errors no longer use an extra line for the label. If no label is present, `error make` will place a bar under the span of itself or the argument to `error make`. Labels have also gotten a bit of a rewrite, but they're pretty much the same as those in nushell#17037, except for `label`, which is now only a single label (not `oneof<list, label>`). #### Simple Labels `label.text` and `labels.*.text` is no longer required for a span to show up, an empty text will simply underline. This example can either use `label: $record` or be written as `labels: [$record]`: ``` > def f [x] { error make {msg: here label: {span: (metadata $x).span}} } f abcd Error: nu::shell::error × here ╭─[entry nushell#7:4:3] 3 │ } 4 │ f abcd · ──── ╰──── ``` #### Multiple labels Any number of labels can be added in the `labels` column, allowing for more detailed error messages, especially for functions: ``` > def f [x y z] { error make {msg: here labels: [ {text: "there" span: (metadata $x).span} {text: "everywhere" span: (metadata $y).span} {text: "somewhere" span: (metadata $z).span} ] } } f abcd [x y z] {d: a} Error: nu:🐚:error × here ╭─[entry nushell#11:9:3] 8 │ } 9 │ f abcd [x y z] {d: a} · ──┬─ ───┬─── ───┬── · │ │ ╰── somewhere · │ ╰── everywhere · ╰── there ╰──── ``` #### External sources There is a `ShellError::OutsideSpannedLabeledError` that can be used to refer to external sources, not just the internal nushell spanns. This has been expanded to allow the multi-label stuff to work using the new `src` column: ``` > "foo\nbar\nbaz" | save -f /tmp/foo.bar error make { msg: 'error here' src: {path: /tmp/foo.bar} labels: [ {text: "this" span: {start: 4 end: 7}} ] } Error: nu:🐚:outside × error here ╭─[/tmp/foo.bar:2:1] 1 │ foo 2 │ bar · ─┬─ · ╰── this 3 │ baz ╰──── ``` ### Errors That Can't be Caught These will not work since `try` will never get parsed: - `try {1 + ""} catch {error make badmath}` - (TODO: Add more examples) ## Internal Changes Most of the parsing from an error record to an actual error is now moved into `nu-protocol`, using `FromValue` to turn it into a useful internal type. ### `nu-protocol::LabeledError` This struct has a few changes, the main one being the type of `LabeledError.inner`. It is now a `ShellError`, not another `LabeledError`. It should be trivial to do a `.into()` for things that already use `LabeledError.with_inner(x)`. ### `nu-protocol::ShellError::into_value` I renamed the old `into_value` to `into_full_value` to better say what it is, since it doesn't just do the `IntoValue::into_value` method, it also requires some context to create the `Value`. Now `ShellError` has an `IntoValue` implementation matching other types. ### `nu-protocol::ShellError::{OutsideSource, OutsideSourceNoUrl}` Miette's derived types don't have a nice way to maybe include a url, so there are now two types! These allow using multiple labels on outside sources. They are used internally for the new `{src: {}}` part of the `error_struct`, and they look a lot more like the `LabeledError`, but without the need for a separate type and all the fun `impl`s that would require for the `Diagnostic::source_code` method. ### Misc * Spelling fix: `into_chainned` => `into_chained` ## Current bugs: - [x] `OutsideSpannedLabeledError` The inner most error of `try {']' from nuon} catch {error make}` will reference `span: {start: 0, end: 1}`, which in `']' from nuon` will point to the `]` character, but when it does this in `error make` as an input it will point to the very first character (probably the `n` in `nu`). ## Release notes summary - What our users need to know ### New `error make` functionality! * New arguments! (`error make "hello"`) * New parts for `error_struct`! (`error make {inner: [] labels: [] ...}`) * Pipeline inputs for chained errors! (`try {error make foo} catch {error make bar}`) * Pipeline inputs for normal errors! (`"help" | error make`) * External errors! (`error make {src: {path: $nu.cofig-path} ...}`) * Backwards compatibility! ## Tasks after submitting  - [ ] Update the [documentation](https://github.com/nushell/nushell.github.io)

ysthakur added 16 commits July 3, 2024 21:21

Add a NuMatcher struct

2777afe

Make NuMatcher take &CompletionOptions rather than owning

c9f27d6

Make flag_completions use NuMatcher

a8bad74

No sort option for NuMatcher

8497f79

Respect case sensitivity when sorting

7ad8f51

Make var complns use NuMatcher

3a70e4e

Make file, dir, and .nu complns use NuMatcher

cf266ae

Make custom complns use NuMatch

5135874

Remove directory_path_completions function

22112e2

Make command compltns use NuMatcher

f835400

Remove print debugging statements

7d91aec

Don't sort command and .nu compltns twice

20e51b8

Get rid of matches_str/u8

2ff0b77

Sort suggestions after collecting all of them

c544022

Sort fuzzy completions in alphabetical order after sorting by score

2d2c2dc

Add completions.sort option

ce8a688

ysthakur closed this Aug 10, 2024

ysthakur deleted the no-sortby branch August 10, 2024 15:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sort completions by match algorithm#4

Sort completions by match algorithm#4
ysthakur wants to merge 16 commits intomainfrom
no-sortby

ysthakur commented Jul 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ysthakur commented Jul 4, 2024

Description

User-Facing Changes

Tests + Formatting

After Submitting

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant