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

Support Visual Studio #28

Closed
chrissimon-au opened this issue Feb 1, 2022 · 37 comments
Closed

Support Visual Studio #28

chrissimon-au opened this issue Feb 1, 2022 · 37 comments
Labels
IDE Tagging issues for adding Contextive to a new IDE released

Comments

@chrissimon-au
Copy link
Contributor

See https://docs.microsoft.com/en-us/visualstudio/extensibility/starting-to-develop-visual-studio-extensions?view=vs-2019

@chrissimon-au chrissimon-au added the IDE Tagging issues for adding Contextive to a new IDE label Feb 1, 2022
@chrissimon-au
Copy link
Contributor Author

Initial poc tests with https://learn.microsoft.com/en-us/visualstudio/extensibility/adding-an-lsp-extension?view=vs-2022 suggest that the default LSP client may not be ideal:

From that page, In fact, support for LSP language servers is designed to work only in open folder/file scenarios. - we want Contextive to 'overlay' information supplied even when VS is used to open a solution or a project, not just the 'open folder' scenario.

May need to investigate using the OmniSharp language client and explicitly wiring up default handles, (e.g. see QuickInfo tooltip - https://learn.microsoft.com/en-us/visualstudio/extensibility/walkthrough-displaying-quickinfo-tooltips?view=vs-2022&tabs=csharp) into the language client call to the language server.

@jimdeane
Copy link

It would be great to have Visual Studio 2022 support. This is a great adjunct to our documentation; however, we extensively use NCrunch which is only available in Visual Studio. Is there any help we could provide?

@chrissimon-au
Copy link
Contributor Author

@jimdeane, thanks for your interest! I have recently had some confirmation from the folk at MS that the recommended extensibility approach is here: https://learn.microsoft.com/en-us/visualstudio/extensibility/visualstudio.extensibility/language-server-provider/language-server-provider?view=vs-2022

I was planning on starting on it in a few weeks, in June 2024. If you are keen to assist, feel free to take a look at that link and try and get it started. The existing language server is available as a separate binary download from github releases, so you could start by downloading that and configuring a hardcoded path just to test out the language server integration, and then once that's working we can look at integrating it into the CI build so that the windows binary gets embedded in the built/published extension.

If you don't get a chance, no worries, I hope to update this ticket in June with progress.

Cheers!

@FH-Inway
Copy link
Contributor

Gave this a shot today. Result for now was that something is happening (the language server .exe shows some CPU utilization when hovering over a word and some logs show messages exchanged with the language server). However, no visual result is visible in Visual Studio.

Not sure when or how I will continue with this, but definitely interested in seeing Contextive become available for VS.

@chrissimon-au
Copy link
Contributor Author

Thanks for getting the ball rolling on this! If you're able to share the logs that show messages exchanged, I'll see if I can help interpret what's happening.

Either way, even if you're not able to continue with this, I appreciate you making this effort, it will definitely help me make progress!

@FH-Inway
Copy link
Contributor

FH-Inway commented Jul 15, 2024

Here is the log: VS156025C6.LSPClient.Microsoft.VisualStudio.LanguageServer.Client.VSExtensibilityLanguageServerProviderClient.JCRC.zip

Sorry about the German in the exception message. After System.AggregateException it says that at least one error occured. And after System.ArgumentNullException, it says that the value may not be NULL.

image

@chrissimon-au
Copy link
Contributor Author

chrissimon-au commented Jul 19, 2024

Thanks for sharing this!

Looking through the raw xml logs, there are two errors:

  • No target methods are registered that match "workspace/configuration".
    • This doesn't appear to be be blocking operation, but means that there's no way to configure a definitions file in a different location
  • Exception thrown from request "(not specified)" for method window/showMessage

I think next steps might be for me to prep a version on a branch that:

  1. doesn't send the showMessage notification
  2. writes more detailed logs to a local file, as the svclog file doesn't include message payloads etc.

And then see if that gets us anywhere - if you have time to assist that would be helpful and I'll let you know when the test version of the languageserver is ready, but if not, totally understand!

@chrissimon-au
Copy link
Contributor Author

chrissimon-au commented Jul 20, 2024

Hi @FH-Inway -you should be able to download a zip containing a special build of the language server from here: https://github.com/dev-cycles/contextive/actions/runs/10017860097/artifacts/1721452354

It has detailed file logging (including json of protocol messages) to a date-rotating file in the same folder as the language server exe file itself.

Also, it no longer sends the window/showMessage message.

If you could give it a try with your visual studio extension and upload the logfiles that would be great! (Apologies, I don't have easy access to a windows box to try myself just at the moment).

(From this build and this commit.)

@FH-Inway
Copy link
Contributor

Thanks, that was helpful. The good news, progress!
image

My branch shows the changes I made to get there. Some I took from your branch.

To get the result from the screenshot, I had to hardcode the full path to the definitions.yml file in the PathResolver.fs.

What I think is happening is that when it cannot determine the definitions.yml file, it has no content for the window/showMessage message, which then fails with the error. But if it has a file, it has content for the message and it works.

See also the Contextive.LanguageServer log20240720-Analysis6.zip from the run where I created the screenshot and compare it with log20240720-Analysis5.zip, where I had not changed the PathResolver.fs yet. Line 50 is the interesting one in both files.

Here is also the Visual Studio log file from the successful run:
VS58BA788C.LSPClient.Microsoft.VisualStudio.LanguageServer.Client.VSExtensibilityLanguageServerProviderClient.RLEX.zip

It looks to me like we have to teach the Visual Studio extension to provide the workspace/folder/file path context information to the language server so it can determine the definitions.yml. And if it cannot, some kind of error message to the user.

Some other notes (and a bit of a rant 😄 )

I had a hell of a time to get this running. First, I tried the Contextive.LanguageServer.exe from your build. Unfortunately, it did not create a log file. Not sure why, I ended up providing a fixed file path for the log file.
Which meant building my own Contextive.LanguageServer.exe. That was the start of the "fun":

  • First, it meant getting familiar with paket.exe (ah yes, another package manager, don't have enough of those already 😉).
  • Once that was sorted out, my Visual Studio Code (for reasons that still elude me) decided to build the projects as .NET 8 projects (which worked for some reason 🤷).
  • So then I started playing with that version, which worked to a point. But then I did "something" (probably building the core and language server projects with Visual Studio instead of Visual Studio Code) and from then on, it only built them as .NET 7.
  • That in turn my poc Visual Studio extension did not like. No matter what I tried, the breakpoints did not get hit and no log files were created. Finally, I saw that it was looking for a .NET 7 runtime in a folder where only a .NET 8 runtime was available (some Visual Studio directory). I'm guessing it has to do with how Visual Studio starts the extension debugging. Maybe Multi-level lookup is disabled is also to blame.
  • So then I upgraded core and language server to .NET 8 and it started working again.
  • From there to the screenshot was pretty straightforward.

So yeah, a bit of a learning curve for me there. I'm sure someone more familiar with F# and Visual Studio extension finds this amusing. Oh well, live and learn, I guess 😃

@chrissimon-au
Copy link
Contributor Author

chrissimon-au commented Jul 20, 2024

Wow, thanks so much, amazing work - so great to see the screenshot (although it would be nice if VS supported markdown 🤦)! And apologies for the frustrations (will address some below)!

Based on your experience, I have a few hypotheses - both the symptoms you saw - the absence of the logfile and being unable to find the definitions file - are due to lack of clarity of the 'current working directory', but different directories.

Logfile

The language server I sent you used a relative path to the logfile, so it would be located in the working folder at the time of invoking the language server.

I assumed it would be next to the exe file because I misread the invocation code as setting the working folder, but actually it's setting the full path of the filename. So it's using the default working directory of %SYSTEMROOT%\system32. (If you take a look there, you might find some logfiles from your initial experiment). (Note: if you set UseShellExecute=true, it becomes the workdirectory of the calling process).

I suspect if you set the WorkingDirectory explicitly (maybe to the solution folder location?), then the original LanguageServer I sent you would put the logfiles in that location.

Since the release build doesn't log to file normally, I'm not too worried about this, but good to note for future dev work against Visual Studio.

Definitions File

The language server defaults to a relative location from the 'workspace root', however based on the error message Error loading definitions: Invalid Path: Unable to locate path '.contextive/definitions.yml' as not in a workspace. no workspace root is defined.

The workspace root is supposed to be defined in the initialize message.

👉 ACTION: To help diagnose this, could you uncomment line 14 in Program.fs which will enable full logging of the protocol messages, and will tell us what Visual Studio is sending down with respect to workspaces or root paths.

For IDEs that don't supply a workspace root, Contextive requires the path to be configured explicitly, but since the workspace/configuration method also isn't supported, it has no chance to compute the location.

There has been some discussion about changing the configuration approach to make it editor agnostic and remove dependency on the workspace/configuration method... perhaps this is a catalyst to do the work to migrate to that approach, rather than finding a way to get Visual Studio to send the workspace root path.

Warning Message

What I think is happening is that when it cannot determine the definitions.yml file, it has no content for the window/showMessage message

While I get the hypothesis, I'm not sure this is the cause - if it can't find the file, then the window/showMessage is supposed to contain exactly the error message as you saw in the logs, so it shouldn't be null. In fact, we don't normally send the window/showMessage if it does find the file - it's reserved for warnings such as unable to find the file or error parsing the file.

So this one is still a mystery, but given the low maturity of the visual studio LSP support, my feeling is probably a bug on that side, but happy to keep investigating as we do still need some way to signal issues to users.

Other Note / Rant Thoughts

Firstly, apologies for the hassle you had with this! As you can tell, this is a relatively early project, which has had limited community support so far, so I haven't invested heavily in supporting a variety of dev environments. The DevContainer should create a useful build environment, but as it's a linux container, building the windows binary is more painful (but not impossible) so I can understand why you didn't use it. I've also been experimenting with nix-shell, which might be a better cross-platform experience and enabling more IDEs (I use it for Rider, but it should make life easier for Visual Studio too). This is a good prompt to both:

  1. Provide better 'contributing' guidelines on how to get the dev environment working
  2. Provide better guidelines on how to enable debugging of the language server from arbitrary IDEs
  3. Add build steps and documentation for producing the language server for a variety of architectures in any given build environment

Regarding the .net sdk version issues - I doubt it's related, but for clarity, since you branched, I completed some work to bump contextive to .net8 which is now on the main branch. The language server.exe I sent you was built with .net8 but it's a self-contained exe so it shouldn't impact the local sdk versions. Since your branch was originally still .net7 VSCode starting to build them as .net8 is just as much a mystery to me! But I'm glad you got it sorted :)

@FH-Inway
Copy link
Contributor

Thanks for the extensive info. First of all, no worries about my frustrations, isn't that a developers daily business? 😉

Ok, let me go through it point by point:

Logfile

Unfortunately, I did not find any log files in %SYSTEMROOT%\system32. I did not investigate further.

Definitions file

Here is a log with JSON formatting: log20240721-Analysis7.zip
Took me a bit to figure out where the additional information is. It seems to be mainly the scope field of the verbose messages and inside of that, the Params.

Viewing those is a bit awkward, since it is a JSON within a JSON within a JSON 😄
I did that for one message manually: Params for Converting params for Request (2) initialize to OmniSharp.Extensions.LanguageServer.Protocol.Models.InternalInitializeParams.json
I then tried a few log viewers and Analogy.LogViewer produces a somewhat useful result. If there is a better way to view those logs, please let me know.
image

Anyways, looks to me like the rootPath and rootUri get provided. However, not sure if and how those are available and processed inside the contextive language server. I tried adding some debug log to the location where I think it gets the/a path from the configuration. But for me, the log just contains the value DidChangeConfigurationProvider. I think the line after that is the one where the Unable to get configuration from client! error in the logs comes from.

I did read a bit through the discussion on editor agnostic config files. Not sure I understand all aspects, to me, it looks like the language server would still need to know the root path of where to look for a config file.

Warning message

Since a successful run has no showMessage log, I agree with your reasoning. Did not look further into this.

Other notes

Again, don't worry about my rant/hassle, it's the good kind (as in "I'm motivated to learn how this works") 😉

  • DevContainer: I like those when it comes to Github codespaces, but my attempts to use them for Windows development have not been very successful so far. And since building the language server locally worked out, I leave learning about that to my future self for now.
  • Prompts for improvement look good to me.
  • Thanks for the .NET 8 Update, I rebased my branch on that.

@chrissimon-au
Copy link
Contributor Author

OK, great - now that we know it does supply the rootUri, it's possible to switch to using that. I hadn't support it until now because it's been deprecated since Workspaces was introduced in 2018... so a bit surprising VS is still only offering it.

But I've added support for it here including unit tests.

I think the next thing that would be interesting to try is:

  1. Explicitly set the WorkingDirectory when launching the LanguageServer process (e.g. here) to the solution folder location if possible, or some other writable location if not.
  2. Use this build of the LanguageServer

While it does skip the window/showMessage notification, and has detailed logging activated, it otherwise is the same as the current main language server - specifically, it has no hardcoded paths for either logging or the workspace root.

If this works, then we'll try re-introducing showMessage and check that the exception there doesn't interrupt usage.

If that works, then it would be great to do a more comprehensive user experience check, specifically things like:

  1. Does it still work if you open a solution file, or only if you open a folder?
  2. Can we make it work in all file types easily or do we have to nominate file types?
  3. Can we make the language server render a more visually appealing text if the client doesn't support markdown?
  4. Does auto-complete work and insert completions as expected?

Then can make an assessment on if/when to launch to the VS marketplace :)

Thanks again, really appreciate your help with this!

@chrissimon-au
Copy link
Contributor Author

Further to above, there is another change on my branch which only uses window/showMessage conditionally if the client advertises that it supports it.

This version of the language server can be downloaded here

@FH-Inway
Copy link
Contributor

Thanks!
I rebased my branch on your visual-studio branch and built the language server from there. See branch comparison.
Setting the working directory worked, see 6b6fd77

Regarding the more comprehensive user experience check:

  1. Does not work when opening a solution, only if opening a folder ❌ (have not yet looked at the differences in the logs)
  1. Seems to work in all file types ✅ See also 46a7fd3
    image
  2. Maybe if the text is html instead of markdown? ❌
  3. Auto-complete works ✅
    image

@chrissimon-au
Copy link
Contributor Author

Amazing progress, thank you!

I think at this point I'd be happy to put it on the marketplace with appropriate caveats, and then take it back to the MS team I'm in touch with to advocate for the necessary improvements on their end (support in solution mode, and markdown support).

Looking at the branch comparison, apart from the forced logging (and logfile name change) and some formatting changes, the language server is the same as the one in my branch. I'll revert the forced logging and merge it to main with just the functional changes (support for clients that don't offer workspaces & avoiding errors in clients that don't offer showMessage). I use conventional commits to generate release logs, so I'd like to merge those independently to get them in the release notes.

So in preparation for creating a clean PR and merging it, could you please revert all those language server changes and create a PR that just includes the new visualstudio folder and associated files?

That last change we'll need to look at in prep for getting it into CI is clarity on embedding the language server into the solution so that it gets included in the vsix file and deployed with the installed extension. If you are able to take a look at that, that would be incredible, but happy for it to be a separate PR, so we can distinguish the 'getting it working' PR and the 'getting it released' PR.

@chrissimon-au
Copy link
Contributor Author

OK, those two changes are now in main - it would be great to see your visual studio extension works with the language server you can download from the latest main build

@FH-Inway
Copy link
Contributor

I rebased again on main and created a new branch visual-studio (not to be confused with your visual-studio branch), which I cleaned up. It now only has one conventional commit, which only contains the extension changes and no changes on the language server.

The extension from that branch works with the latest main language server, but... 😄

I also tried embedding the language server into the extension solution by adding a reference to the language server project. Building works, all the language server stuff is added to the vsix. Well, almost all.

When running it, I get a message that the JSON-RPC connection was lost. Why? Windows event log tells me that it could not load Newtonsoft.Json, version 13.0.0.0 (note the version number).
The language server has Newtonsoft.Json 13.0.3 as dependency. The .dll is added just fine in the bin folder of the extension project. However, it is not added to the .vsix file or the folder that Visual Studio uses to run another instance of itself when debugging the extension.

My preliminary digging into this revealed that it may be related to Visual Studio having its own Newtonsoft.Json dlls and it wants to use those. See https://stackoverflow.com/questions/50003033/vsix-newtonsoft-isnt-in-package-vs15-5-suppress-package and JamesNK/Newtonsoft.Json#2534
Also Newtonsoft.Json.dll identifies as 13.0.0.0 when it is actually 13.0.3? Not sure I understand that one: JamesNK/Newtonsoft.Json#2662

If I redirect the extension to use the Contextive.LanguageServer.exe from the build directory (which has the 13.0.3 Newstonsoft.Json.dll) instead of the extension directory, it works.

This needs further looking into, I don't think we should release it without fixing it. Maybe something can be done with the extension manifest to include the .dll. While I understand that is not recommended, I believe this only applies for the Visual Studio extension itself. But since the language server is a separate component, it should be fine to use its own dll instead of the ones Visual Studio ships with.

Popup message in Visual Studio:
image

Windows event message:

Application: Contextive.LanguageServer.exe
CoreCLR Version: 8.0.524.21615
.NET Version: 8.0.5
Description: The process was terminated due to an unhandled exception.
Exception Info: System.TypeInitializationException: The type initializer for '<StartupCode$Contextive-LanguageServer>.$Contextive.LanguageServer.Program' threw an exception.
---> System.IO.FileNotFoundException: Could not load file or assembly 'Newtonsoft.Json, Version=13.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed'. The system cannot find the file specified.
File name: 'Newtonsoft.Json, Version=13.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed'
at <StartupCode$Contextive-LanguageServer>.$Contextive.LanguageServer.Program..cctor()
--- End of inner exception stack trace ---
at Contextive.LanguageServer.Program.main(String[] argv) in K:\Repositories\contextive\src\language-server\Contextive.LanguageServer\Program.fs:line 35

@FH-Inway
Copy link
Contributor

On another note, I don't think the extension will be useful if it cannot support files opened from a solution. Solutions are the main working context in Visual Studio, unlike Visual Studio Code, where you often work within a folder context. A lot of functionality is lost in Visual Studio if no solution, but a folder is opened. I can't imagine it will get much adoption with that limitation.

On another another note 😄, the extension is currently implemented in C#. I'd like to give it a go to implement it in F# and also add some tests to it similar to what was done for the Visual Studio Code extension.

@chrissimon-au
Copy link
Contributor Author

Thanks again, @FH-Inway!

VS Solution vs Folder mode

You are 100% correct that it's far less useful if it only works in folder mode.

My motivation in launching it to the app store even in this state is twofold:

  1. I get a lot of requests for support in VS. With an extension in the marketplace, I can point to it and say 'it's here, it may not work how you want, here's how to advocate to MS to improve their support so it will work how you want.'
  2. I have a line of comms to the MS team that is working on this - with it in the Marketplace I can more easily demonstrate to them how it works, why it would be beneficial, and how much interest there is in improving it.

I believe their justification is that the LSP is intended for unsupported languages -> unsupported languages wouldn't need solution mode -> no need to support it. However, as well as Contextive's use case, I think there are a number of polyglot solutions these days that would benefit from supporting additional language servers alongside the built-in language support offered in solution mode, and I hope I can make that case.

Build Process

Yes, apologies I should have made it clear - it was always my intention to re-use the pre-built language server and embed it as a resource in the extension, rather than have it re-built as a project reference in the extension solution. This is now the recommended LSP architecture, and it takes care of many issues - the ones you found (such as dependency conflict) but also frees us up to evolve our sdk version usage at different times, and ensures it's using an isolated, tested, immutable artefact for the language server component.

The github actions currently have a stage that prepare the language server and make it available as an artefact. Successive stages can download that artefact to the appropriate location before running a script to build the vsix that will have it embedded. I can take care of updating the github actions once the other parts are in place.

FSharp

I see you also like playing on hard mode 😎 - by all means, go for it, but also, I don't mind releasing an alpha version with what we have and migrating to FSharp later - totally up to you if you want to enjoy the challenge :)

@NatElkins
Copy link

I am a lone VS Code user in an organization that primarily uses Visual Studio. I would love to introduce this tool into our code base, but lack of VS support is a blocker. Just wanted to chime in and let you know VS support is highly desirable!

@FH-Inway
Copy link
Contributor

@chrissimon-au Fair enough. I removed the changes on the main solution and also removed the language server project dependency from the Visual Studio extension project. See pr #70

@NatElkins Thanks for letting us know and the interest. Would you be interested in helping us test this Visual Studio integration?

@NatElkins
Copy link

Sadly I'm on a Mac 😅 . But if I can get my hands on a Parallels installation through work I'll take a crack at it.

@jimdeane
Copy link

jimdeane commented Jul 25, 2024 via email

@chrissimon-au
Copy link
Contributor Author

Thanks to testing volunteers! To be clear about limitations in the current version - it should be treated as an 'alpha' version only, for a few reasons:

  • The Visual Studio Language Server Extensibility model we use has some functional limitations, specifically:
    • No support when opening a solution file, only when opening a 'folder' (which restricts other language support for C#, projects etc.)
    • No support for markdown rendering in the hover text, leading to less pleasant to look at descriptive hover panels

The goal of releasing this alpha version is as a proof of concept and to demonstrate interest in these features to the MS team to advocate for improving support.

Stay tuned, we should have a vsix produced on the CI pipeline in the next 24-48 hrs that will be testable locally, and shortly after that a version installable from the VS extension marketplace.

Huge thanks to @FH-Inway for this work!! 😍

@chrissimon-au
Copy link
Contributor Author

@jimdeane:

Along the way we've been experimenting (only hacked together PoCs) with a Word (Office 365 really...) add-in and a browser (Edge) extension to provide the same function.

Both of these have been on my longer term roadmap - knowing that there may be an application for them is definitely motivating! If you're able to open source the POCs you've done it might help accelerate, but stay tuned for work that will expand Contextive beyond the IDE into those locations.

Given the technical architecture of such things, it will likely require the establishment of a cloud service to host the terminology database - which might actually enable the volume of terms you're talking about with SNOMED. (It will also support making the slackbot I've demo'd in a few talks more broadly available too.).

It's an open question how to get the benefits of having term definitions co-located and versions in the repo while getting the benefits of making the terms available in a broader set of user contexts. I'll leave that discussion to another issue, though!

@NatElkins
Copy link

I think this extension/application has a lot of potential to improve code base understanding in an LLM world.

@FH-Inway
Copy link
Contributor

Alright, this has been fun. @chrissimon-au Thanks for the merge and the nice back and forth these last two weeks. Looking forward to seeing the alpha version of the extension on the marketplace.

@jimdeane Those PoCs for Office and browser sound very intruiging, would be great if you could share more about them. But yeah, let's do that in another issue or discussion. This one has gotten rather long 😄

@jimdeane
Copy link

jimdeane commented Jul 25, 2024 via email

@FH-Inway
Copy link
Contributor

New thread created here: #71

@chrissimon-au
Copy link
Contributor Author

OK, for those interested in alpha testing, there is a vsix downloadable here produced by this github actions run (you can also download it from the list of artifacts on the run).

The downloaded file will be a zip - unzip it to get the vsix and then install by double clicking or using the Tools->Extension Manger in Visual Studio.

Unfortunately I don't have access to a windows box right now, so I'm not able to check that the vsix even installs correctly, but from visual inspection of its contents, it looks like it should be correct.

Appreciate your assistance!

To test:

  1. Open a folder (not a solution)
  2. ensure there is a file in .contextive/definitions.yml that contains a set of definitions - you could use some of these as a starter, or there is a sample file in the Usage Giude
  3. Create another file - markdown, csharp, any kind should work. Auto-complete should include the terms from the definitions file, and if you use one of the terms, then hover should show the hover panel with definitions.

I'm not sure if we've tested Visual Studio's ability to notify contextive of changes to the definitions file, so if it doesn't work first time, try re-opening the folder with the definitions file already in place.

For @FH-Inway - the language server is embedded in the vsix file - right beside the extension's dll - so it should find and start it properly, but if anything doesn't work, incorrect path to the language server exe is probably the first thing I'd try and check.

@FH-Inway
Copy link
Contributor

Works for me. Though I noticed a couple of things:

  • It does not seem to work for all types of files. In my case, it does not work with .txt, .md or .bat files. Files with the .yml and .cs extension do work. These needs more testing to find a pattern of why some file types do not work.
  • Changes of the definitions file are not picked up right away. Re-opening the folder picks up the changes.
  • While Visual Studio is running, no Contextive.LanguageServer process is listed in task manager. When Visual Studio is closed, most of the time (but not always), a Contextive.LanguageServer process is added in task manager. It seems no cleanup is happening of already existing processes. At the time I'm writing this, task manager lists 5 Contextive.LanguageServer processes, each with a different memory consumption of about 30 MB.

image

@chrissimon-au
Copy link
Contributor Author

Although it's the older extensibility model, the only MS documentation on which LSP features are supported is here:

https://learn.microsoft.com/en-us/visualstudio/extensibility/adding-an-lsp-extension?view=vs-2019#language-server-protocol-supported-features

If that is still accurate in the new extensibility model, then window/showMessage should be supported, and workspace/didChangeWatchedFiles (which drives detecting changes to the definitions file) should also be supported.

Since both have not worked for us, perhaps we should assume that the support is not the same. I'll see if I can get any further clarity on which LSP features are supported in the new extensibility model.

@chrissimon-au
Copy link
Contributor Author

A few more updates after further investigation:

Leftover Contextive.LanguageServer.exe processes

In most other IDEs it's the language client's responsibility to stop the process. In the rust example there's even a comment suggesting that VS will send the stop command and then dispose the process.

That said, there is no stop command, so I don't know how reliable this is. There are shutdown and exit messages that are supposed to control the sequence of language server graceful exiting.

From the example I can't see any other hooks in the language server provider for manually implementing the shutdown sequence.

I'll add this to the list I report to MS about.

Updating Definitions

This relies on didChangeWatchedFiles - I mentioned above that it is supposed to be supported, but I realised from reviewing the logs you sent earlier that while it is supported, it doesn't support 'dynamic registration'.

Dynamic registration means that it is configured by the language server after the initialization sequence.

Without dynamic registration, it means the client must decide to start watching files 'on it's own' which means it needs to know which files to watch, which couples the configuration resolution algorithm to the server. (Which is why the LSP protocol authors no longer recommend that approach, amongst other reasons).

However, until the Visual Studio LSP implementation supports dynamic registration, as a pragmatic workaround, we can try and work out how to setup file watching in the client at least with the default definitions file location.

Next Steps

I will prep an update to the language server so that we can turn on debug logs with an environment variable and without a rebuild - then we can get some more diagnostics for more of these situations.

I've reached out to the MS team for guidance on the 'solution mode' and 'markdown' questions - when I get a reply, I'll find out where we can report issues like this officially/publicly and ask about the shutdown/exit issue.

@chrissimon-au
Copy link
Contributor Author

chrissimon-au commented Jul 30, 2024

Have had a reply from MS. Their preferred reporting approach is here: https://developercommunity.visualstudio.com/VisualStudio?q=lsp

I've also found https://github.com/microsoft/VSExtensibility which might have some helpful info in the future.

My plan now is to report each issue we find on the community site and cross-link it to a specific contextive issue so we can track them independently and get this current ticket closed off once it's launched to the marketplace.

To summarise issues I'll be reporting:

Note that the MS VS team prioritises work based on community votes on that developer community portal. If having Contextive in Visual Studio is important to you, please upvote the suggestions/bugs which should be linked to on each of the github issues above.

chrissimon-au pushed a commit that referenced this issue Sep 4, 2024
# [1.12.0](v1.11.1...v1.12.0) (2024-09-04)

### Bug Fixes

* **intellij:** Add support for 2042.2 series of intellij platform ([08308bf](08308bf))

### Features

* **language-server:** Add support for LSP Clients that only support rootUri and not workspaces (e.g. Visual Studio) ([7fe11b3](7fe11b3))
* **language-server:** Add yaml schema for definitions file ([#74](#74)) ([65ec44a](65ec44a))
* **language-server:** Only use 'window/showMessage' if it is supported by the LanguageClient (e.g. Visual Studio does not support it) ([965cb30](965cb30))
* **language-server:** validate definitions file for missing term names ([0fb0978](0fb0978))
* **visual-studio:** Add Visual Studio integration ([b052a82](b052a82)), closes [#28](#28)
chrissimon-au pushed a commit that referenced this issue Sep 4, 2024
# [1.12.0](v1.11.1...v1.12.0) (2024-09-04)

### Bug Fixes

* **intellij:** Add support for 2042.2 series of intellij platform ([08308bf](08308bf))

### Features

* **language-server:** Add support for LSP Clients that only support rootUri and not workspaces (e.g. Visual Studio) ([7fe11b3](7fe11b3))
* **language-server:** Add yaml schema for definitions file ([#74](#74)) ([65ec44a](65ec44a))
* **language-server:** Only use 'window/showMessage' if it is supported by the LanguageClient (e.g. Visual Studio does not support it) ([965cb30](965cb30))
* **language-server:** validate definitions file for missing term names ([0fb0978](0fb0978))
* **visual-studio:** Add Visual Studio integration ([b052a82](b052a82)), closes [#28](#28)
chrissimon-au pushed a commit that referenced this issue Sep 4, 2024
# [1.12.0](v1.11.1...v1.12.0) (2024-09-04)

### Bug Fixes

* **intellij:** Add support for 2042.2 series of intellij platform ([08308bf](08308bf))

### Features

* **language-server:** Add support for LSP Clients that only support rootUri and not workspaces (e.g. Visual Studio) ([7fe11b3](7fe11b3))
* **language-server:** Add yaml schema for definitions file ([#74](#74)) ([65ec44a](65ec44a))
* **language-server:** Only use 'window/showMessage' if it is supported by the LanguageClient (e.g. Visual Studio does not support it) ([965cb30](965cb30))
* **language-server:** validate definitions file for missing term names ([0fb0978](0fb0978))
* **visual-studio:** Add Visual Studio integration ([b052a82](b052a82)), closes [#28](#28)
* **vscode:** Publish to Open-Vsx Marketplace (closes [#80](#80)) ([2d23775](2d23775))
@chrissimon-au
Copy link
Contributor Author

Released on v1.12.0

Now available on the VS Marketplace here: https://marketplace.visualstudio.com/items?itemName=devcycles.ContextiveVisualStudio

Since publishing, I've realised that the VS extension publishing guidelines are missing some important steps, including nomination of which VS editions this is valid for. It's possible that this version will only work with Visual Studio Community edition.

I'll look to mark it as supporting more editions in the next release.

Also note that I'm aware that due to the many usability issues at the moment, I don't expect this will suit all cases, but now I have a CI pipeline for publishing and an extension in the marketplace, I hope this will be a baseline to improve on as VS improves their language server protocol support.

@FH-Inway
Copy link
Contributor

FH-Inway commented Sep 4, 2024

Nice to see it out there!

The ReadMe should also updated, it currently has the information for the VSCode extension.

@chrissimon-au
Copy link
Contributor Author

The readme was updated to be visual studio specific, except I missed the installation instructions link 🤦

Already sorted on main will get picked up in the next update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IDE Tagging issues for adding Contextive to a new IDE released
Projects
None yet
Development

No branches or pull requests

4 participants