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

feat: Compatible with the extension host class of Vscode #3713

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

weirongxu
Copy link
Contributor

@weirongxu weirongxu commented Mar 17, 2022

I've try to port some vscode extensions. (like mintlify)
However, this extension uses the MarkdownString API, so it's a bit difficult to port to coc.
So I tried to migrate some extension host classes to coc.

vscode source https://github.com/microsoft/vscode/blob/main/src/vs/workbench/api/common/extHostTypes.ts

Regarding the vscode-languageserver-protocol API (e.g. Range.create(), which can be replaced by new Range()).
I'm not sure whether we are keeping these older APIs, I can mark it as @deprecated, but another option is to keep them backward compatible.
Please let me know if you have any suggestions about this.

TODO

  • classes
    • Position
    • Range
    • Selection
    • TextEdit
    • WorkspaceEdit (pending)
    • Location
    • Diagnostic
    • Hover
    • MarkdownString
  • export
  • typing
  • unit test

@weirongxu weirongxu changed the title feat(workspace): Compatible with the extension host class of Vscode feat: Compatible with the extension host class of Vscode Mar 17, 2022
@codecov
Copy link

codecov bot commented Mar 18, 2022

Codecov Report

Merging #3713 (b83595f) into master (093e430) will decrease coverage by 3.05%.
The diff coverage is 50.00%.

❗ Current head b83595f differs from pull request most recent head 20c0216. Consider uploading reports for the commit 20c0216 to get more accurate results

@@            Coverage Diff             @@
##           master    #3713      +/-   ##
==========================================
- Coverage   82.05%   79.00%   -3.06%     
==========================================
  Files         223      211      -12     
  Lines       23309    21450    -1859     
  Branches     5184     4875     -309     
==========================================
- Hits        19127    16946    -2181     
- Misses       2434     2743     +309     
- Partials     1748     1761      +13     
Impacted Files Coverage Δ
src/util/string.ts 87.17% <50.00%> (-3.04%) ⬇️
src/cursors/util.ts 43.33% <0.00%> (-46.33%) ⬇️
src/plugin.ts 48.37% <0.00%> (-46.20%) ⬇️
src/model/installer.ts 48.61% <0.00%> (-40.05%) ⬇️
src/provider/linkedEditingRangeManager.ts 60.00% <0.00%> (-26.67%) ⬇️
src/language-client/utils/async.ts 67.50% <0.00%> (-25.00%) ⬇️
src/util/errors.ts 50.00% <0.00%> (-21.43%) ⬇️
src/model/progress.ts 68.29% <0.00%> (-20.60%) ⬇️
src/cursors/session.ts 78.21% <0.00%> (-18.93%) ⬇️
src/cursors/index.ts 76.47% <0.00%> (-16.48%) ⬇️
... and 216 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 093e430...20c0216. Read the comment docs.

@weirongxu weirongxu force-pushed the feat/workspace-utils branch from 5897774 to df1c2b3 Compare March 28, 2022 05:06
@chemzqm
Copy link
Member

chemzqm commented Apr 8, 2022

It seems you need rename those class names to something like VPosition, since coc.nvim already exposed them from vscode-languageserver-types

@weirongxu
Copy link
Contributor Author

weirongxu commented Apr 8, 2022

It seems you need rename those class names to something like VPosition, since coc.nvim already exposed them from vscode-languageserver-types

Thank you for your reply.
Can we keep the VPosition of the vscode host compatible with the Position? In this case, just need to expose the new Postion.

I'm busy these days, I'll probably come back in a week

@chemzqm
Copy link
Member

chemzqm commented Apr 8, 2022

Can we keep the VPosition of the vscode host compatible with the Position? In this case, just need to expose the new Postion

The current Position is an interface and namespace, unless you only add static methods to exists namespace, it can't be compatible. Or rename all Position in typings.d.ts to something like IPosition

@weirongxu
Copy link
Contributor Author

The current Position is an interface and namespace, unless you only add static methods to exists namespace, it can't be compatible. Or rename all Position in typings.d.ts to something like IPosition

I got it now, I'll make some effort, otherwise use a new name like VPosition, and thanks again

@weirongxu weirongxu force-pushed the feat/workspace-utils branch from df1c2b3 to 8c534be Compare May 7, 2022 04:32
@weirongxu weirongxu force-pushed the feat/workspace-utils branch from 8c534be to 20c0216 Compare June 5, 2022 12:10
@luisdavim
Copy link

is this abandoned?

@chemzqm
Copy link
Member

chemzqm commented Jan 26, 2023

I prefer a new module named coc-vscode-adaptor which provide APIs the same as VSCode, which would make migrate VSCode extensions much easier.

@weirongxu
Copy link
Contributor Author

I'm really sorry, because I couldn't focus on the development of coc related extensions for a long time before, which caused this pr to be stranded.

I'll check the code and associated implementation again after a while. If I can't go on, we would use another extension to implement the compatibility layer as chemzqm said.

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.

3 participants