Add option for choosing stdio as the LSP communication channel

[HarleyRossetto]

Resolves #76436

[HarleyRossetto]

@JoeRobich re: this comment: #72871 (comment)

Where would you guys like to redirect Console.Out to?

[CyrusNajmabadi]

Can we do this as well:

One thought is that we would use the stream returned from Console.OpenStandardOutput() for messaging and reassign Console.Out using Console.SetOut() to redirect Console.WriteLine() calls to a different stream.

[HarleyRossetto]

Can we do this as well:

One thought is that we would use the stream returned from Console.OpenStandardOutput() for messaging and reassign Console.Out using Console.SetOut() to redirect Console.WriteLine() calls to a different stream.

Yeah, whats the preferred destination?
Do we want a temporary file? stderr?

Do we also need to do anything with the logger factory and redirect that too or will simply redirecting Console.Out flow through to that too?

[JoeRobich]

Yeah, whats the preferred destination?
Do we want a temporary file? stderr?

I think ideally it would be Stream.Null. I don't think that we would want to always write to a temporary file, although it could be useful when troubleshooting. StandardError seems like a good in-between place to land, although I've certainly gotten feedback about writing non-error messages to stderr before. @jasonmalinowski what are your thoughts?

[HarleyRossetto]

Yeah, whats the preferred destination?
Do we want a temporary file? stderr?

I think ideally it would be Stream.Null. I don't think that we would want to always write to a temporary file, although it could be useful when troubleshooting. StandardError seems like a good in-between place to land, although I've certainly gotten feedback about writing non-error messages to stderr before. @jasonmalinowski what are your thoughts?

I also had the thought of outputting to another long lived log file under the extension log directory path today, so just throwing that out there too.

Will await Jason's thought on all of this.

[jasonmalinowski]

Do we also need to do anything with the logger factory and redirect that too or will simply redirecting Console.Out flow through to that too?

We'd have to very carefully check on the ordering that everything gets created to make sure it'd grab Console.Out at the right time. We should most definitely add a test for that. :-)

Writing out stderr all the logging/errors/whatever-else-people-tried-writing-to-stdout seems reasonable to me, especially because you'd want somewhere to write if things go really bad and you can't get LSP started. Honestly, stderr might be where the runtime might send stuff regardless.

[JoeRobich]

We'd have to very carefully check on the ordering that everything gets created to make sure it'd grab Console.Out at the right time. We should most definitely add a test for that. :-)

Well we won't reference Console.Out directly but will get the stream via Console.OpenStandardOutput() which should always return the correct output stream.

@HarleyRossetto Sound like we can move forward with redirecting to stderr.

[HarleyRossetto]

Ok thanks for everyone's input, at work currently, will get to it this evening. You should have something that review tomorrow.

[JoeRobich]

Thanks for updating @HarleyRossetto! Will likely be a couple weeks before we can look at moving this forward. Most of the team will be out until the new year.

[JoeRobich]

We should add validation that --stdio and --pipe are not both passed together. Logging an error and returning a non-zero exit code would be appropriate.

[JoeRobich]

We should move this redirection to the top of RunAsync.

[HarleyRossetto]

On this point, should it be just after the logger is created?

static async Task RunAsync(ServerConfiguration serverConfiguration, CancellationToken cancellationToken)
{
    // Create a console logger as a fallback to use before the LSP server starts.
    using var loggerFactory = LoggerFactory.Create(builder => ...);

    var logger = loggerFactory.CreateLogger<Program>();

    // Handle LSP communication channel method and Console.Out redirection
    if (serverConfiguration.UseStdIo)
    {
        if (serverConfiguration.ServerPipeName is not null)
        {
            logger.LogError("Server cannot be started with both --stdio and --pipe options.");
            Environment.Exit(1);
        }

        // Redirect Console.Out to try prevent the standard output stream from being corrupted.
        Console.SetOut(new StreamWriter(Console.OpenStandardError()));
    }
    
    ...

[dibarbet]

@HarleyRossetto I suspect we actually need to do this before we setup the logger - currently we have a log wrapper that preferentially logs to LSP, but will fallback to logging via stdout if LSP isn't yet initialized.

If we don't redirect stdout before we create the logger, it's possible for the logger to write messages to stdout while LSP initialization is on-going (and possibly corrupt the LSP stream).

logger.LogError("Server cannot be started with both --stdio and --pipe options."); Environment.Exit(1);

For this I would throw an exception with a custom message so you don't need to utilize the logger

[HarleyRossetto]

@dibarbet Thanks for the further guidance! I've updated my latest commit based on your comment.

I've just thrown a regular exception, is that OK or do you have a preference for another type?
I did find a CommandLineArgumentCombinationException type as part of the NuGet.Common package the LSP picks up from a dependency, but I wasn't convinced you would necessarily want to use a custom exception from a dependency.

[dibarbet]

thanks for the contribution! a couple small comment updates then we can get it merged

[HarleyRossetto]

@dibarbet and @JoeRobich Thank you for your guidance and review on this one, much appreciated.

Updated based on your lasted comments.