Handle possible errors from RangeVisibleInBuffer() #4192
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bstaletic
force-pushed
the
errors-from-range-visible-in-buffer
branch
from
cb50e20
to
4b3df69
Compare
First and more common error is that by the time we execute
buffer = vim.buffers[ bufnr ]
the buffer might not be there any more. This is because
`RangeVisibleInBuffer()` is called asynchronously and the user may bwipeout
a buffer in between polls.
This regularly happens in our vim tests. In such a case, we get a nasty
traceback from `vimsupport` module.
The solution is to catch the KeyError and return None.
However, `ScrollingBufferRange()` also was not ready to handle None values
from `RangeVisibleInBuffer()`, even though `RangeVisibleInBuffer()` could return
None even before, if a visible window for `bufnr` can not be found.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #4192 +/- ##
==========================================
+ Coverage 88.90% 89.00% +0.10%
==========================================
Files 34 34
Lines 4398 4403 +5
==========================================
+ Hits 3910 3919 +9
+ Misses 488 484 -4 |
|
We seem not only to not catch traces during test teardown but not produce coverage either? The coverage part is probably ok, but not actually catching this KeyError in CI is odd. |
puremourning
approved these changes
Oct 4, 2023
There was a problem hiding this comment.
Cheers!
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: 1 of 2 LGTMs obtained (waiting on @bstaletic)
|
Thanks for sending a PR! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Prelude
Thank you for working on YCM! :)
Please complete these steps and check these boxes (by putting an
xinsidethe brackets) before filing your PR:
rationale for why I haven't.
actually perform all of these steps.
Why this change is necessary and useful
First and more common error is that by the time we execute
the buffer might not be there any more. This is because
RangeVisibleInBuffer()is called asynchronously and the user may bwipeout a buffer in between polls.This regularly happens in our vim tests. In such a case, we get a nasty traceback from
vimsupportmodule.The solution is to catch the KeyError and return None.
However,
ScrollingBufferRange()also was not ready to handle None values fromRangeVisibleInBuffer(), even thoughRangeVisibleInBuffer()could return None even before, if a visible window forbufnrcan not be found.As for the missing tests... showing that an inherently racy scenario does not cause a stacktrace in vim level test... I think I prefer to keep what is left of my sanity.
This change is