YouCompleteMe icon indicating copy to clipboard operation
YouCompleteMe copied to clipboard

[RFC] Correctly handle multiline diagnostics in diagnostic_interface

Open bstaletic opened this issue 2 years ago • 1 comments

PR Prelude

Thank you for working on YCM! :)

Please complete these steps and check these boxes (by putting an x inside the brackets) before filing your PR:

  • [x] I have read and understood YCM's CONTRIBUTING document.
  • [x] I have read and understood YCM's CODE_OF_CONDUCT document.
  • [x] I have included tests for the changes in my PR. If not, I have included a rationale for why I haven't.
  • [x] 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

While there doesn't seem to be a bug here (#3993 not withstanding), there might be some improvement possible.

Previously, when we would attempt props_to_remove.remove( multiline_prop ), it was wrong 100% of the time. This PR attempts to fix that.

The thing to keep in mind is that, on vim, prop_list() never returns multiline properties. Instead, it splits multiline properties into multiple single line properties. To figure out if should skip adding a multiline property, we also need to split it by lines.

Open questions:

  1. Is this a net negative in the average case, when it comes to performance?
  2. Since we need to split props into single line ones, should we just always deal with single line props?
  3. Does this mess anything up regarding neovim?

As for the tests... no idea how to test this thing, as it shouldn't change any directly observable behaviour.

[Please explain in detail why the changes in this PR are needed.]


This change is Reviewable

bstaletic avatar Dec 28 '21 23:12 bstaletic

Codecov Report

Merging #3997 (0b23dc9) into master (3c5a063) will decrease coverage by 0.38%. The diff coverage is 45.00%.

@@            Coverage Diff             @@
##           master    #3997      +/-   ##
==========================================
- Coverage   91.29%   90.91%   -0.39%     
==========================================
  Files          27       27              
  Lines        3712     3730      +18     
==========================================
+ Hits         3389     3391       +2     
- Misses        323      339      +16     

codecov[bot] avatar Dec 29 '21 18:12 codecov[bot]

As you said, this is quite more complexity and maybe bad for perf. So far as I know we're not seeing a lot of complaints about this. We can revisit if we do.

puremourning avatar Sep 29 '22 19:09 puremourning