Fix status bar diagnostics flashing #20797
Conversation
cla-bot
bot
added
the
cla-signed
gartnera
force-pushed
the
fix-status-bar-diagnostics-flashing
branch
from
7baeae4 to
0747dc6
Compare
| } | ||
| } | ||
|
|
||
| fn _debounced_diagnostic_update(&mut self, cx: &mut ViewContext<Self>) { | ||
| // do not schedule another task if one is already running |
There was a problem hiding this comment.
What is the point of this, and all the rest new comments around?
The code around is quite self-descriptive and shows the behavior perfectly, so let's remove all the comments added here.
| if this._diagnostic_update_task.is_none() { | ||
| return; | ||
| } | ||
| this._diagnostic_update_task = None; |
There was a problem hiding this comment.
This looks somewhat magical, as we're removing ourselves from the state during running.
Could we invert the approach?
No if self._diagnostic_update_task.is_some() check, but instead, we always, unconditionally, do self._diagnostic_update_task = Some(....
This way, we will always start waiting, replacing the previous wait/update.
| return; | ||
| } | ||
| this._diagnostic_update_task = None; | ||
| this.current_diagnostic = this.pending_diagnostic.clone(); |
There was a problem hiding this comment.
Correct me if I'm wrong, but pending_diagnostic can only be None?
As we do self.pending_diagnostic = new_diagnostic only if NOT new_diagnostic.is_some().
Why is this field needed even?
Feels like we need to always do
DEBOUNCE
self.current_diagnostic = new_diagnostic;
in a task and always replace the previous task?
Or, we can name this _diagnostics_cleanup_task and it seems that we only debounce the cleanup part of the update.
| return; | ||
| } | ||
|
|
||
| self._diagnostic_update_task = Some(cx.spawn(|this, mut cx| async move { |
There was a problem hiding this comment.
NIT: this is badly readable, esp. in GitHub interface, could it be diagnostic_indicator or whatever that is?
|
I wanted this to be shipped before the next release, so had created a PR with a proper impl: #21463 Thank you again for spotting this. |
The diagnostic element of the status bar flashes as you're typing which is very distracting. I think this is due to a race between the cursor position updating and the diagnostics updating.
current_diagnosticwill rapidly switch between a real diagnostic andNone. Add a debouncer which will wait 50ms to clear the current diagnostic if the new diagnostic isNone.Before
2024-11-17-153642.mp4
After
2024-11-17-153720.mp4
As you can see the text no longer flashes.
Release Notes: