multilspy icon indicating copy to clipboard operation
multilspy copied to clipboard
verified publisher icon microsoft

Adding language support for JS/TS via typescript-language-server

Open themichaelusa opened this issue 1 year ago • 1 comments

This is still a work in progress, but should be ready to go in no more than a day.

TODOs:

  • [x] Revert changes to pyproject.toml
  • [x] Remove junk files from my fork (todo.txt, tests_js, etc)
  • [ ] Proper tests for JS/TS integration under multilspy/tests
  • [ ] Some code styling nits + comments where needed
  • [x] Revert event loop changes (these are for a separate PR - assuming I can replicate hanging behavior w/ SyncLanguageServer)

themichaelusa avatar Oct 09 '24 00:10 themichaelusa

@microsoft-github-policy-service agree company="Rubrick, Inc."

themichaelusa avatar Oct 09 '24 00:10 themichaelusa

I think i solved the problem described here: https://github.com/microsoft/multilspy/issues/5#issuecomment-2405189497

see attached file: initialize_params.json

I basically removed the window section from the capabilities. I don't think it is needed for our API usage, and it causes the stuck problem

image

mrT23 avatar Oct 15 '24 05:10 mrT23

I think i solved the problem described here: #5 (comment)

see attached file: initialize_params.json

I basically removed the window section from the capabilities. I don't think it is needed for our API usage, and it causes the stuck problem

image

Ok great. Had a busy weekend - will write test cases tonight and we should be ready to merge.

themichaelusa avatar Oct 15 '24 23:10 themichaelusa

Dear @themichaelusa and @mrT23 ,

Thank you very much for your efforts on implementing JS/TS support. I will try to review this ASAP.

Excited about this development :)

LakshyAAAgrawal avatar Oct 16 '24 18:10 LakshyAAAgrawal

Just went over the code. Looks great. Will try to test it on my system soon and then merge. In the meanwhile, could you please address the few nits I have added?

Done! Please let me know if there are any issues running it on your end.

Misc notes on portability: I don't have the means to test Windows support, but I've been testing this on MacOS (arm) locally, and we've been running our fork in production on GCP - so we can be reasonably confident re: Linux support.

themichaelusa avatar Oct 16 '24 23:10 themichaelusa

I have reviewed the full PR and verified it running correctly. Thank you very much for this PR.

Can you make this one final change (remove print statement), and I will immediately proceed to merge.

No problem! I look forward to opening many future PRs. This is a great package and has a lot of potential

themichaelusa avatar Oct 21 '24 19:10 themichaelusa

Love to see this

shobrook avatar Oct 23 '24 11:10 shobrook