Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New vim #4260

Merged
merged 4 commits into from
Sep 12, 2024
Merged

New vim #4260

merged 4 commits into from
Sep 12, 2024

Conversation

bstaletic
Copy link
Collaborator

@bstaletic bstaletic commented Aug 28, 2024

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:

  • I have read and understood YCM's CONTRIBUTING document.
  • I have read and understood YCM's CODE_OF_CONDUCT 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

A bit of a cleanup now that ubuntu 24.04 is out.
The tests will fail because I have not pushed the container images.
After I push those, the "Old vim" CI job (running ubuntu's default vim) will be newer than "New vim" job that is running whatever is needed for a fully-featured YCM.


This change is Reviewable

Copy link

codecov bot commented Aug 29, 2024

Codecov Report

Attention: Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 89.86%. Comparing base (b6e8c64) to head (bf5252c).
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4260      +/-   ##
==========================================
+ Coverage   89.79%   89.86%   +0.07%     
==========================================
  Files          37       37              
  Lines        4780     4758      -22     
==========================================
- Hits         4292     4276      -16     
+ Misses        488      482       -6     

@bstaletic bstaletic force-pushed the new-vim branch 6 times, most recently from c52eda1 to 16e5f70 Compare August 31, 2024 15:17
Copy link
Collaborator Author

@bstaletic bstaletic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vim layer coverage is screwed.

Coverage.py warning: Plugin file tracers (covimerage.CoveragePlugin) aren't supported with PyTracer

Reviewed 11 of 11 files at r1, all commit messages.
Reviewable status: 0 of 2 LGTMs obtained (waiting on @bstaletic)

Copy link
Collaborator Author

@bstaletic bstaletic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So... after porting covimerage over night, here are the coverage results:

Name                                               Stmts   Miss  Cover
----------------------------------------------------------------------
autoload/youcompleteme.vim                          1003    172    83%
autoload/youcompleteme/filetypes.vim                  12      0   100%
autoload/youcompleteme/finder.vim                    426     41    90%
autoload/youcompleteme/hierarchy.vim                 173     14    92%
autoload/youcompleteme/symbol.vim                     43      1    98%
plugin/youcompleteme.vim                             198     30    85%
python/ycm/__init__.py                                 0      0   100%
python/ycm/base.py                                    76      0   100%
python/ycm/buffer.py                                  74      2    97%
python/ycm/client/__init__.py                          0      0   100%
python/ycm/client/base_request.py                    147     11    93%
python/ycm/client/command_request.py                 107      7    93%
python/ycm/client/completer_available_request.py      16      0   100%
python/ycm/client/completion_request.py              108      2    98%
python/ycm/client/debug_info_request.py               62      0   100%
python/ycm/client/event_notification.py               27      2    93%
python/ycm/client/inlay_hints_request.py              26     17    35%
python/ycm/client/messages_request.py                 37      0   100%
python/ycm/client/omni_completion_request.py          13      0   100%
python/ycm/client/resolve_completion_request.py       44      1    98%
python/ycm/client/semantic_tokens_request.py          26      6    77%
python/ycm/client/shutdown_request.py                 10      0   100%
python/ycm/client/signature_help_request.py           52      6    88%
python/ycm/client/ycmd_keepalive.py                   14      1    93%
python/ycm/diagnostic_filter.py                       55      0   100%
python/ycm/diagnostic_interface.py                   192     14    93%
python/ycm/hierarchy_tree.py                         111      8    93%
python/ycm/inlay_hints.py                             42     19    55%
python/ycm/omni_completer.py                          66      6    91%
python/ycm/paths.py                                   37      3    92%
python/ycm/scrolling_range.py                         61     13    79%
python/ycm/semantic_highlighting.py                   48      7    85%
python/ycm/signature_help.py                          92      4    96%
python/ycm/syntax_parse.py                           124      1    99%
python/ycm/text_properties.py                         39      8    79%
python/ycm/unsafe_thread_pool_executor.py             70     18    74%
python/ycm/vimsupport.py                             592     55    91%
python/ycm/youcompleteme.py                          529     22    96%
----------------------------------------------------------------------
TOTAL                                               4752    491    90%

That seems reasonable, the tiny difference compared to my expectation could be due to different environment.
The covimerage fork that supports coveragepy 7 can be found here:
https:/bstaletic/covimerage/tree/covimerage-7

Reviewable status: 0 of 2 LGTMs obtained (waiting on @bstaletic)

@bstaletic
Copy link
Collaborator Author

Considering my fork and no activity in upstream... should we put my fork in the docker container? Should I publish my fork on pypi? Not sure...

@puremourning
Copy link
Member

Let's use your fork. I don't think many people use covimerage

Since Ubuntu 24.04 is out, we can clean up a bit.
Some of the `VimSupportsFoo()` functions will always return `True` in
vim, but we also need to take into account othervim.
coverage and click were pinned because covimerage was outdated.
Now covimerage states its dependencies properly and works with
coveragepy 7.
@bstaletic
Copy link
Collaborator Author

Green!

@bstaletic
Copy link
Collaborator Author

Coverage looks decent.

  • Overall coverage is about the same.
  • The 70% patch coverage is correct - we don't have tests for text properties nor do we have negative tests for plugin/youcompleteme.vim.
  • The indirect changes in autoload/youcompleteme.vim seem suspicious, as the function has to have been executed. Especially since the call site is covered.

I'll merge this because CI is busted.

@bstaletic bstaletic merged commit e177cb4 into ycm-core:master Sep 12, 2024
8 of 11 checks passed
@bstaletic bstaletic deleted the new-vim branch September 12, 2024 00:25
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