-
Notifications
You must be signed in to change notification settings - Fork 178
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
RFC: Create bubbles at once #762
Comments
/cc @mdgriffith (author of inline messenger) |
The inline messenger package handles it roughly the way you suggest. All messages are created, just hidden. It still loops through all messages to see if they contain the cursor point, when the cursor moves. If a message is found it is given an ".is-selected" class, at which point they're shown. Technically, every message does create 2 markers in inline-messages, which was the solution to get the bubble positioning correct, though the two markers are created and destroyed together. I guess, as I'm writing this I'm thinking...do we want a solution in linter or using inline-messages? I kinda figure we want something in linter until the plan/details for inline-messages + linter is hashed out. |
Some additional thoughts if the message lookup on cursor change is slow. I'm probably going to implement some of the following things in inline-messenger. findMarkers might be faster than manually looping through all messages, but then you have to track from the marker to the message. Checking to see if the currently viewed message is still in range might be a good way to escape before looping through messages. Putting the messages in an object that is indexed by line might be a good trick if findMarkers turns out to be slow. |
Glad to know that inline-messenger is already handling it, we'll be porting to it soon so we can just close this issue. |
Currently we are rechecking all of the ranges on every cursor move event, which is quite expensive, and unlike other decorations, doesn't update as we type, consider we have the following situation
and we get the error at the start of
:non-existing-class
, then we add the decorations for it, atom itself will update the decorations as we type, to focus on the error string, but the case is different with bubbles, we don't create their markers at once, we validate them on each cursor move event, and because the co-ordinates in that marker would still point to that old location after we change it, they are kinda valid.So my proposed solution here would be that we add all the bubble place hold markers at once, then once the user selects that range, show the bubble otherwise hide it, and we could do it in one marker for both underline and bubble, instead of two.
/cc @atom-community/linter-contributors
The text was updated successfully, but these errors were encountered: