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

Zed parser errors showing non-user code #3051

Closed
philrz opened this issue Apr 19, 2024 · 2 comments · Fixed by #3065
Closed

Zed parser errors showing non-user code #3051

philrz opened this issue Apr 19, 2024 · 2 comments · Fixed by #3065
Labels
bug Something isn't working

Comments

@philrz
Copy link
Contributor

philrz commented Apr 19, 2024

Repro is with Zui commit 4b851d8.

The changes in #2972 generally improved the presentation of Zed query parse errors. For instance, to establish the "before" state, at commit f985e66, here's how the error looked when typing an incomplete query like from (.

Repro-before.mp4

This kind of awful looking error dump is now replaced with the more succinct and helpful error messages such as shown here. However, for this specific from ( example, here's what we see at commit 4b851d8 that has the changes from #2972.

Repro-after.mp4

A paste of that error:

error parsing Zed at line 2, column 3:
  | { i: count(), v: this}
= ^ ===

This is a reference to some of the behind-the-scenes additions we make to the user's query in order to gather metadata about how many values were in the query result, paginate results, etc.

Since this part of the query was not typed by the user, it seems likely to cause confusion. I don't know if the fix for this would actually have to be in the Zed parser, but I'm starting by opening up in Zui since triggering the problem is unique to how it augments the user's query.

@philrz philrz added the bug Something isn't working label Apr 19, 2024
@philrz
Copy link
Contributor Author

philrz commented Apr 25, 2024

This was discussed at a recent team meeting. In an exchange between @mattnibs and @jameskerr it sounded like there was consensus on an approach that would address this in the short term. The proposal is that when there's a parser error detected on a query, the app will call the compile endpoint with the original unmodified query and present that error. At some point further down the line there may be a more sophisticated approach that offloads more of this logic to the Zed service side.

@philrz
Copy link
Contributor Author

philrz commented May 7, 2024

Verified in Zui commit f7727f3.

Repeating the repro steps as shown in the attached video, now the error message seen by the user references only the Zed code in the editor.

Verify.mp4

Thanks @jameskerr!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant