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

Editor Error Markers #3087

Merged
merged 10 commits into from
Jun 12, 2024
Merged

Editor Error Markers #3087

merged 10 commits into from
Jun 12, 2024

Conversation

jameskerr
Copy link
Member

@jameskerr jameskerr commented Jun 10, 2024

Markers in the editor when the query contains errors.

@jameskerr jameskerr requested review from mattnibs and philrz June 11, 2024 22:06
@philrz

This comment was marked as resolved.

@mattnibs

This comment was marked as resolved.

@mattnibs
Copy link
Contributor

I think we should get rid of this error area when the errors are markers.
338760960-5becd398-6d28-4dd1-a3a3-90a72e0e6659

@jameskerr
Copy link
Member Author

Hi all, I've fixed the error in the source set code in the last commit. I was adding the pipe at the wrong moment.

@mattnibs You're right that there is some duplication. The error markers are shown as the user types. Not when they press enter to submit the query. During the time they've modified the query but not yet submitted it, they'll only see the red markers. Once they press enter, they will see the error in the results view. I think this is better than displaying nothing in the results error.

So I think we should leave this as is for now. Here's a video showing the current behavior:

CleanShot.2024-06-12.at.10.48.37.mp4

@jameskerr jameskerr merged commit c4ff726 into main Jun 12, 2024
4 checks passed
@jameskerr jameskerr deleted the editor-validate branch June 12, 2024 20:50
@philrz philrz linked an issue Jun 12, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Help user find the location in the editor for a syntax error
3 participants