Markup Microsoft.PowerShell.EditorServices.Server

Author: andyleejordan

src/PowerShellEditorServices/Server/PsesDebugServer.cs

@@ -99,6 +99,7 @@ public async Task StartAsync()
                         .AddLogging()
                         .AddOptions()
                         .AddPsesDebugServices(ServiceProvider, this, _useTempSession))
+                    // TODO: Consider replacing all WithHandler with AddSingleton
                     .WithHandler<LaunchAndAttachHandler>()
                     .WithHandler<DisconnectHandler>()
                     .WithHandler<BreakpointHandlers>()

src/PowerShellEditorServices/Server/PsesLanguageServer.cs

@@ -31,11 +31,16 @@ internal class PsesLanguageServer
 
         /// <summary>
         /// Create a new language server instance.
+        ///
+        /// NOTE: This class is only ever instantiated via <see
+        /// cref="EditorServicesServerFactory.CreateLanguageServer"/>. It is essentially a
+        /// singleton. The factory hides the logger.
         /// </summary>
         /// <param name="factory">Factory to create loggers with.</param>
         /// <param name="inputStream">Protocol transport input stream.</param>
         /// <param name="outputStream">Protocol transport output stream.</param>
-        /// <param name="hostStartupInfo">Host configuration to instantiate the server and services with.</param>
+        /// <param name="hostStartupInfo">Host configuration to instantiate the server and services
+        /// with.</param>
         public PsesLanguageServer(
             ILoggerFactory factory,
             Stream inputStream,
@@ -52,6 +57,10 @@ public PsesLanguageServer(
 
         /// <summary>
         /// Start the server listening for input.
+        ///
+        /// For the services (including the <see cref="PowerShellContextService">
+        /// context wrapper around PowerShell itself) see <see
+        /// cref="PsesServiceCollectionExtensions.AddPsesLanguageServices"/>.
         /// </summary>
         /// <returns>A task that completes when the server is ready and listening.</returns>
         public async Task StartAsync()
@@ -62,11 +71,12 @@ public async Task StartAsync()
                     .WithInput(_inputStream)
                     .WithOutput(_outputStream)
                     .WithServices(serviceCollection => serviceCollection
-                        .AddPsesLanguageServices(_hostDetails))
+                        .AddPsesLanguageServices(_hostDetails)) // NOTE: This adds a lot of services!
                     .ConfigureLogging(builder => builder
-                        .AddSerilog(Log.Logger)
+                        .AddSerilog(Log.Logger) // TODO: Set dispose to true?
                         .AddLanguageProtocolLogging()
                         .SetMinimumLevel(_minimumLogLevel))
+                    // TODO: Consider replacing all WithHandler with AddSingleton
                     .WithHandler<PsesWorkspaceSymbolsHandler>()
                     .WithHandler<PsesTextDocumentHandler>()
                     .WithHandler<GetVersionHandler>()
@@ -91,10 +101,14 @@ public async Task StartAsync()
                     .WithHandler<ShowHelpHandler>()
                     .WithHandler<ExpandAliasHandler>()
                     .WithHandler<PsesSemanticTokensHandler>()
+                    // The OnInitialize delegate gets run when we first receive the _Initialize_ request:
+                    // https://microsoft.github.io/language-server-protocol/specifications/specification-current/#initialize
                     .OnInitialize(
                         // TODO: Either fix or ignore "method lacks 'await'" warning.
                         async (languageServer, request, cancellationToken) =>
                         {
+                            Log.Logger.Debug("Initializing OmniSharp Language Server");
+
                             var serviceProvider = languageServer.Services;
                             var workspaceService = serviceProvider.GetService<WorkspaceService>();
 
@@ -125,6 +139,7 @@ public async Task StartAsync()
         /// <returns>A task that completes when the server is shut down.</returns>
         public async Task WaitForShutdown()
         {
+            Log.Logger.Debug("Shutting down OmniSharp Language Server");
             await _serverStart.Task.ConfigureAwait(false);
             await LanguageServer.WaitForExit.ConfigureAwait(false);
         }

src/PowerShellEditorServices/Server/PsesServiceCollectionExtensions.cs

@@ -24,16 +24,21 @@ public static IServiceCollection AddPsesLanguageServices(
                     (provider) =>
                         PowerShellContextService.Create(
                             provider.GetService<ILoggerFactory>(),
+                            // NOTE: Giving the context service access to the language server this
+                            // early is dangerous because it allows it to start sending
+                            // notifications etc. before it has initialized, potentially resulting
+                            // in deadlocks.
                             provider.GetService<OmniSharp.Extensions.LanguageServer.Protocol.Server.ILanguageServerFacade>(),
                             hostStartupInfo))
-                .AddSingleton<TemplateService>()
+                .AddSingleton<TemplateService>() // TODO: What's the difference between this and the TemplateHandler?
                 .AddSingleton<EditorOperationsService>()
                 .AddSingleton<RemoteFileManagerService>()
                 .AddSingleton<ExtensionService>(
                     (provider) =>
                     {
                         var extensionService = new ExtensionService(
                             provider.GetService<PowerShellContextService>(),
+                            // NOTE: See above warning.
                             provider.GetService<OmniSharp.Extensions.LanguageServer.Protocol.Server.ILanguageServerFacade>());
                         extensionService.InitializeAsync(
                             serviceProvider: provider,