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

Fix error highlighting in IDE. #715

Merged
merged 10 commits into from
Jul 13, 2023
Merged

Fix error highlighting in IDE. #715

merged 10 commits into from
Jul 13, 2023

Conversation

sampottinger
Copy link
Collaborator

@sampottinger sampottinger commented May 10, 2023

Error highlighting is a bit broken (see #714).

Root cause: Depending on how the Problem is made, the error may be given relative to start of line or start of tab. Changed method names to indicate to users of Problems which one they are working with.

I think this may have been introduced in the language server support effort as Problems may be made without Editor available. The behavior in #714 appears to have started then but there was a delay before it showed up in production given how releases were done.

Resolution: I clarified the interface and gave client code ability to determine if they want / need a Problem to be placed relative to start of line or start of tab where each are more convenient in different contexts.

Depending on how the Problem is made, the error may be given relative to
start of line or start of tab. Flag indicates to users of Problems which
one they are working with.
@sampottinger sampottinger changed the title Closes #714. Fix error highlighting. May 10, 2023
@sampottinger sampottinger changed the title Fix error highlighting. Fix error highlighting in IDE. May 10, 2023
@sampottinger sampottinger requested a review from benfry May 10, 2023 23:38
@sampottinger
Copy link
Collaborator Author

sampottinger commented May 10, 2023

Hey @benfry! It looks like some ambiguity in Problem caused an issue when we implemented language server support that broke error highlighting. LMK if this looks good! Sorry I noticed something was off a while ago but I thought it was a theme issue and didn't have time to dig into exactly what was going on until today when I realized it wasn't just my local install. 😅

@sampottinger
Copy link
Collaborator Author

CC @SableRaf

@sampottinger
Copy link
Collaborator Author

Doing one bit of cleanup this morning but we should prioritize this

@sampottinger
Copy link
Collaborator Author

Cool did the clean up! Ready for review... I think it's probably best to have client code indicate which type of start offset it wants when interacting with a problem. I've tested the PDE... checking language server.

@sampottinger
Copy link
Collaborator Author

Yay! Confirmed language server working. :D

@sampottinger
Copy link
Collaborator Author

I know we don't have an imminent release but this is is getting in the way of a GSoC project. To make sure he can still finish on time / pass, I'm going to merge this to main but I won't make a release. Without this sorted, the error message rewriting isn't visible to the user.

@sampottinger sampottinger merged commit f0f1d01 into main Jul 13, 2023
@sampottinger sampottinger mentioned this pull request Aug 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant