Skip to content

fix: Add safety check for b:ycm_hover#4298

Merged
puremourning merged 1 commit intoycm-core:masterfrom
ZaharChernenko:fix/ycm_hover-safety-check
Dec 30, 2025
Merged

fix: Add safety check for b:ycm_hover#4298
puremourning merged 1 commit intoycm-core:masterfrom
ZaharChernenko:fix/ycm_hover-safety-check

Conversation

@ZaharChernenko
Copy link
Contributor

@ZaharChernenko ZaharChernenko commented Apr 9, 2025

PR Prelude

  • I have read and understood YCM's [CONTRIBUTING][cont] document.
  • I have read and understood YCM's [CODE_OF_CONDUCT][code] document.
  • I have included tests for the changes in my PR. If not, I have included a
    rationale for why I haven't.
  • I understand my PR may be closed if it becomes obvious I didn't
    actually perform all of these steps.

Why this change is necessary and useful

The function s:ShowHoverResult() assumes b:ycm_hover always exists when processing hover responses. However, this assumption is unsafe, and leads to errors like this:

Error detected while processing function <SNR>151_PollCommands[30]..<SNR>151_ShowHoverResult:
line   36:
E121: Undefined variable: b:ycm_hover
Error detected while processing function <SNR>151_PollCommands[30]..<SNR>151_ShowHoverResult:
line   36:
E116: Invalid arguments for function has_key( b:ycm_hover, 'popup_params' )
Error detected while processing function <SNR>151_PollCommands[30]..<SNR>151_ShowHoverResult:
line   42:
E121: Undefined variable: b:ycm_hover
Error detected while processing function <SNR>151_PollCommands[30]..<SNR>151_ShowHoverResult:
line   42:
E116: Invalid arguments for function setbufvar

So I added a check for the existence of a variable.


This change is Reviewable

@puremourning
Copy link
Member

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Dec 30, 2025

rebase

✅ Branch has been successfully rebased

@puremourning puremourning force-pushed the fix/ycm_hover-safety-check branch from a641c99 to 79b7e54 Compare December 30, 2025 13:12
@codecov
Copy link

codecov bot commented Dec 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.80%. Comparing base (5110798) to head (79b7e54).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4298   +/-   ##
=======================================
  Coverage   89.80%   89.80%           
=======================================
  Files          37       37           
  Lines        4765     4765           
=======================================
  Hits         4279     4279           
  Misses        486      486           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@puremourning
Copy link
Member

Thanks!

@puremourning puremourning merged commit 159e8de into ycm-core:master Dec 30, 2025
8 checks passed
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.

2 participants