Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use span in error make #10914

Closed
KAAtheWiseGit opened this issue Nov 1, 2023 · 8 comments · Fixed by #10923
Closed

Use span in error make #10914

KAAtheWiseGit opened this issue Nov 1, 2023 · 8 comments · Fixed by #10923
Labels
delight this feature would delight users enhancement New feature or request error-handling How errors in externals/nu code are caught or handled programmatically (see also unhelpful-error) good first issue Good for newcomers
Milestone

Comments

@KAAtheWiseGit
Copy link
Contributor

KAAtheWiseGit commented Nov 1, 2023

Related problem

Currently error make takes in a record, which has start and end fields. I think this is somewhat unergonomic, as in majority of times some existing span is passed to it. This would get rid of currently necessary patter of creating a temporary span variable. And if a custom span has to be made (a rarer case, IMO), it can still be created manually, just with one more level of nesting.

Describe the solution you'd like

Replace label: {text: "...", start: ..., end: ...} with label: {text: "...", span: ...}.

Example

Currently (an example from error make docs):

    def foo [x] {
        let span = (metadata $x).span;
        error make {
            msg: "this is fishy"
            label: {
                text: "fish right here"
                start: $span.start
                end: $span.end
            }
        }
    }

With the proposed change:

    def foo [x] {
        error make {
            msg: "this is fishy"
            label: {
                text: "fish right here"
                span: (metadata $x).span
            }
        }
    }
@KAAtheWiseGit KAAtheWiseGit added enhancement New feature or request needs-triage An issue that hasn't had any proper look labels Nov 1, 2023
@fdncred
Copy link
Collaborator

fdncred commented Nov 1, 2023

I wonder if we can use both? like if we detect a "span:" key, look for a span and if we see a "start:" and "end:" use the individual ones. I say that we probably want to keep start/end because sometimes you have to tweak the span's start or end. That probably doesn't happen that often but it may happen.

@KAAtheWiseGit
Copy link
Contributor Author

@fdncred in theory, there are two ways to ways to tweak the ends with this proposal:

mut span = (metadata $x).span
span.end += 10
...
error make {
    label: {
        span: {
            start: 10
            end: 20
        }
    }
}

I didn't think about the idea to mix them, though, and I like it. One thing which I worry about is the error message when the two get mixed. If someone puts both span and start/end, what should the error say?

@fdncred
Copy link
Collaborator

fdncred commented Nov 1, 2023

It should be an error if someone puts both span and start/end stating that it's disallowed and to choose either span or start/end. One other benefit to doing it this way is that it keeps us from having a breaking-change.

@amtoine
Copy link
Member

amtoine commented Nov 2, 2023

i think this would be quite cool to have 🙏

@amtoine amtoine added good first issue Good for newcomers delight this feature would delight users error-handling How errors in externals/nu code are caught or handled programmatically (see also unhelpful-error) and removed needs-triage An issue that hasn't had any proper look labels Nov 2, 2023
@fdncred
Copy link
Collaborator

fdncred commented Nov 2, 2023

Related/Unrelated, I was reminded again today that error make doesn't allow the addition of the help: line. It would be nice to have an optional help: key in the creation of error make.

Example

 ls | into list -adfa
Error: nu::parser::unknown_flag

  × The `into list` command doesn't have flag `-a`.
   ╭─[entry #44:1:1]
 1 │ ls | into list -adfa
   ·                 ┬
   ·                 ╰── unknown flag
   ╰────
  help: Available flags: --help(-h). Use `--help` for more information.

In fact, I'd love to be able to optionally support every functionality that nushell supports with miette-style errors.

@amtoine
Copy link
Member

amtoine commented Nov 2, 2023

completely support you here @fdncred 👍
i even opened an issue about that a while back 😉

@KAAtheWiseGit
Copy link
Contributor Author

KAAtheWiseGit commented Nov 2, 2023

@amtoine I checked it out and it turns out the help option was added to GenericError since then. I think this means we can kill two birds with one stone here.

@amtoine
Copy link
Member

amtoine commented Nov 2, 2023

@KAAtheWiseGit
oh yeah, if we can kill these two birds at once, i'd be super happy to have a more powerful and easier to use error make 😇

KAAtheWiseGit added a commit to KAAtheWiseGit/nushell that referenced this issue Nov 2, 2023
- Replaced `start`/`end` with span.
- Fixed standard library.
- Add `help` option.
- Add a couple more errors for invalid record types.

Resolve nushell#10914
KAAtheWiseGit added a commit to KAAtheWiseGit/nushell that referenced this issue Nov 2, 2023
- Replaced `start`/`end` with span.
- Fixed standard library.
- Add `help` option.
- Add a couple more errors for invalid record types.

Resolve nushell#10914
KAAtheWiseGit added a commit to KAAtheWiseGit/nushell that referenced this issue Nov 2, 2023
- Replaced `start`/`end` with span.
- Fixed standard library.
- Add `help` option.
- Add a couple more errors for invalid record types.

Resolve nushell#10914
fdncred pushed a commit that referenced this issue Nov 3, 2023
- Replaced `start`/`end` with span.
- Fixed standard library.
- Add `help` option.
- Add a couple more errors for invalid record types.

Resolve #10914


# Description



# User-Facing Changes

- **BREAKING CHANGE:** `error make` now takes in `span` instead of
`start`/`end`:

  ```Nushell
  error make {
      msg: "Message"
      label: {
          text: "Label text"
          span: (metadata $var).span
      }
  }
  ```
- `error make` now has a `help` argument for custom error help.
@hustcer hustcer added this to the v0.87.0 milestone Nov 4, 2023
hardfau1t pushed a commit to hardfau1t/nushell that referenced this issue Dec 14, 2023
- Replaced `start`/`end` with span.
- Fixed standard library.
- Add `help` option.
- Add a couple more errors for invalid record types.

Resolve nushell#10914


# Description



# User-Facing Changes

- **BREAKING CHANGE:** `error make` now takes in `span` instead of
`start`/`end`:

  ```Nushell
  error make {
      msg: "Message"
      label: {
          text: "Label text"
          span: (metadata $var).span
      }
  }
  ```
- `error make` now has a `help` argument for custom error help.
dmatos2012 pushed a commit to dmatos2012/nushell that referenced this issue Feb 20, 2024
- Replaced `start`/`end` with span.
- Fixed standard library.
- Add `help` option.
- Add a couple more errors for invalid record types.

Resolve nushell#10914


# Description



# User-Facing Changes

- **BREAKING CHANGE:** `error make` now takes in `span` instead of
`start`/`end`:

  ```Nushell
  error make {
      msg: "Message"
      label: {
          text: "Label text"
          span: (metadata $var).span
      }
  }
  ```
- `error make` now has a `help` argument for custom error help.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delight this feature would delight users enhancement New feature or request error-handling How errors in externals/nu code are caught or handled programmatically (see also unhelpful-error) good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants