YouCompleteMe
YouCompleteMe copied to clipboard
[RFC] Correctly handle multiline diagnostics in diagnostic_interface
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:
- Is this a net negative in the average case, when it comes to performance?
- Since we need to split props into single line ones, should we just always deal with single line props?
- 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.]
Codecov Report
Merging #3997 (0b23dc9) into master (3c5a063) will decrease coverage by
0.38%
. The diff coverage is45.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
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.