ycm-core / ycmd

A code-completion & code-comprehension server
https://ycm-core.github.io/ycmd/
GNU General Public License v3.0
1.69k stars 764 forks source link

Only parse the initial file the first time it's opened #1722

Closed puremourning closed 8 months ago

puremourning commented 9 months ago

When the language_server_completer is first constructed, it's usually a FileReadyToParse callback. But at that time, we can't actually parse anything because we haven't done the initialization exchange with the server, so we queue this up for later.

The way this was implemenented was to register a "on file ready to parse" callback to update the server with the file contents at the time the completer was constructed. In practice, as the completer is usually constructed via a file ready to pase event, this actually happens immediately and we then shunt the request into "OnInitializeComplete" handlers, which are fired when the initialize exchange happens.

Why don't we just put something directly in OnInitializeComplete handlers? Well because in the constructor, we don't have the request_data - we only get that in the OnFileReadyToParse callback, so we have this jank. WCGW?

As a result of this dance, we actually introduced a subtle error. OnFileReadyToParse handlers are called every time we have a file parse event, and - we never remove this "parse the file contents as they were at the time of initialization" request from the "file ready to parse handlers" list. So every time we has a file parse event, we update the server with stale data, then immediately correct it! In practice, this isn't all that noticable, but it is braindead.

Simple solution is to add a "once" flag to the handler creating oneshot handlers, and clear such handlers after firing them (or converting them to initialize handlers). Jank to fix jank. It might be better to find a way to just remove these handler "lists" altogether, as there is only one other place in the codebase where one is registered, and we could always just pass request_data to constructor if we have it...


This change is Reviewable

codecov[bot] commented 8 months ago

Codecov Report

Merging #1722 (3204179) into master (80a034d) will increase coverage by 0.00%. The diff coverage is 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1722 +/- ## ======================================= Coverage 95.45% 95.45% ======================================= Files 83 83 Lines 8136 8141 +5 Branches 163 163 ======================================= + Hits 7766 7771 +5 Misses 320 320 Partials 50 50 ```
mergify[bot] commented 8 months ago

Thanks for sending a PR!

mergify[bot] commented 8 months ago

Thanks for sending a PR!