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

fix: ls provider's score is equal to the default one #4722

Merged
merged 2 commits into from
Aug 22, 2023

Conversation

tcx4c70
Copy link
Contributor

@tcx4c70 tcx4c70 commented Aug 5, 2023

For now, ls provider's score will be equal to the default one if ls provider's documentSelector has pattern. For example, a ls registers capability textDocument/documentHighlight with documentSelector {'language': 'cs', 'scheme': 'file', 'pattern': '**/*.cs'}, it will get score 5, which is equal to the default one with documentSelector '*', even if the language id has matched. Since the capability is registered later than the default one, DocumentHighlightManager will only use the default one to provide highlight, which is not expected.

If all of language, scheme, and pattern match, we should return the highest score of them, instead of the last score.

For now, ls provider's score will be equal to the default one if ls
provider's documentSelector has pattern. For example, a ls registers
capability `textDocument/documentHighlight` with documentSelector
`{'language': 'cs', 'scheme': 'file', 'pattern': '**/*.cs'}`, it will
get score 5, which is equal to the default one with documentSelector
'*', even if the language id has matched. Since the capability is
registered later than the default one, DocumentHighlightManager will
only use the default one to provide highlight, which is not expected.

If all of language, scheme, and pattern match, we should return the
highest score of them, instead of the last score.

Signed-off-by: Adam Tao <tcx4c70@gmail.com>
@codecov
Copy link

codecov bot commented Aug 5, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01% 🎉

Comparison is base (eba04ac) 98.75% compared to head (ff02fd4) 98.76%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4722      +/-   ##
==========================================
+ Coverage   98.75%   98.76%   +0.01%     
==========================================
  Files         273      273              
  Lines       25920    25923       +3     
  Branches     5376     5379       +3     
==========================================
+ Hits        25596    25604       +8     
+ Misses        188      186       -2     
+ Partials      136      133       -3     
Files Changed Coverage Δ
src/core/funcs.ts 97.61% <100.00%> (ø)

... and 6 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fannheyward
Copy link
Member

in testing, sorry for the delay review.

@fannheyward
Copy link
Member

fannheyward commented Aug 22, 2023

The fix is OK, please test for this case:

expect(workspace.match([{ language: 'xml', scheme: 'file', pattern: '**/*.xml' }], doc.textDocument)).toBe(10)

to src/__tests__/modules/workspace.test.ts L295

Signed-off-by: Adam Tao <tcx4c70@gmail.com>
@fannheyward fannheyward merged commit 0a2c8b8 into neoclide:master Aug 22, 2023
4 checks passed
chemzqm added a commit that referenced this pull request Aug 31, 2023
15c8946 Revert "chore(package): use yarn instead"
dacd62f chore(package): use yarn instead
0e6be47 fix(package): use npm
0189ace chore(inlayHint): check nvim version
d81fde9 feat(inlayHint): inline display for nvim (#4648)
1038082 Allow watching workspaces at /tmp/some-dir (#4632)
34f5d96 refactor(glob): use latest glob module
b2ae10c fix(type): fix bad types
5385efd chore(package): require node 16.18.0
5ea8b52 chore(README): gitter url to matrix (#4520)
fc260ab fix(input): delete buffer on vim only
02c2542 fix(native): get pathstr by cursor position (#4553)
1c9c72f fix(workspaceFolder): check directory of fsPath (#4566)
b1e5d71 fix(util): check $XDG_CONFIG_HOME before use as data home (#4492)
abdb129 fix(input): bdel term buf (#4608)
c790182 chore(doc): target of diagnosticInfo
5571414 feat(diagnostic): add target to diagnosticInfo (#4642)
e4ffae5 fix(list): scrolling float preview for vim (#4647)
c5a7123 feat(data): allow languageserver.env (#4658)
44898ec fix(format): formatOnSaveTimeout config with scope (#4689)
d432de2 feat(links): only fetch links on documeng changed (#4690)
ed89997 fix: clean unused code (#4710)
4f3b4d9 feat(handler): hasProvider can accept bufnr (#4714)
18152f2 chore(package): update dependencies
bacbe49 refactor(highlighter): highligher.ts to highlighter.ts (#4730)
5423ee0 feat(completion): stop completion when navigate
29065d9 chore(doc): escape coc_status
1d36382 chore(doc): add refactor buffer section
0a2c8b8 fix: ls provider's score is equal to the default one (#4722)
fab97c7 perf: return early in around and buffer source (#4721)
eba04ac fix: autoActiavte to autoActivate, keeped to kept (#4716)
f7545f2 docs: update outdated url (#4717)
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.

None yet

2 participants