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

Code location from previous command failure is sometimes retained #2129

Open
quexxon opened this issue Nov 21, 2024 · 4 comments
Open

Code location from previous command failure is sometimes retained #2129

quexxon opened this issue Nov 21, 2024 · 4 comments

Comments

@quexxon
Copy link
Collaborator

quexxon commented Nov 21, 2024

In this example, calling p initially shows the expected code location in the echo error, but after calling echo erroneously at the top-level, the code location from the last failed top-level execution is displayed thereafter when calling p.

ysh-0.24.0$ proc p (...args) {
>   echo $args # introducing a bug so this fails
> }

ysh-0.24.0$ p 1 2 3
    echo $args # introducing a bug so this fails
    ^~~~
[ interactive ]:2: fatal: Word eval got a List, which can't be stringified (OILS-ERR-203)

ysh-0.24.0$ echo $[{}]
  echo $[{}]
       ^~
[ interactive ]:5: fatal: Expr sub expected one of (Null Bool Int Float Str Eggex), got Dict

ysh-0.24.0$ p 1 2 3
  echo $[{}]
       ^~
[ interactive ]:5: fatal: Word eval got a List, which can't be stringified (OILS-ERR-203)
@andychu
Copy link
Contributor

andychu commented Nov 21, 2024

Ah OK, I think this is because we have a loc.Missing combined a failure to set the "fallback" location

I will look at this, thank you!

@andychu
Copy link
Contributor

andychu commented Dec 10, 2024

OK I repro'd this ... looking into it

@andychu
Copy link
Contributor

andychu commented Dec 10, 2024

OK I narrowed the bug down to this code:

https://github.com/oils-for-unix/oils/blob/master/core/state.py#L1370

  • in theory, we want locations attached to every message

    • in practice, sometimes we have loc.Missing, and "fall back" to the "last known" location
  • there is a fallback location for expressions, and a fallback location for statements

  • the expression location is considered more "specific", so it's preferred

But that's wrong here

Because the one for statements is more recent


So there are probably 2 fixes

  • remove the loc.Missing, supply a real location with the error
  • fix the logic about expressions vs. statements, not sure what it should be

this is a very good bug, thanks for the report!

andychu pushed a commit that referenced this issue Dec 11, 2024
This is issue #2129

- Reset self.loc_for_expr on every line
  - the expression location is supposed to refine the line location
- Pass location info in Stringify(), so we don't need to use the
  fallback location
@andychu
Copy link
Contributor

andychu commented Dec 11, 2024

I just fixed this, and improved the error location too.



    echo $args
         ^~~~~
[ -c flag ]:3: fatal: Word eval got a List, which can't be stringified (OILS-ERR-203)
  echo $[{}]
       ^~
[ -c flag ]:8: fatal: Expr sub expected one of (Null Bool Int Float Str Eggex), got Dict
    echo $args
         ^~~~~
[ -c flag ]:3: fatal: Word eval got a List, which can't be stringified (OILS-ERR-203)

Thanks for the report!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants