VSCode Extension

[belav]

It would be great to have an extension similar to https://github.com/prettier/prettier-vscode

This is mainly just a placeholder for getting the work started and determining what needs to be done.

Format on save will require #282

[shocklateboy92]

I was thinking we could do one better, and integrate it directly into omnisharp. That way, all the 'format' actions would use csharpier by default and have a smoother experience than just formatting on save.

However, as per OmniSharp/omnisharp-roslyn#2167, they don't think omnisharp is the place for it. I want to try adding that functionality directly into Rosyln itself. So that way, people can specify the formatter (and version) as a nuget package, much like how Roslynator provides refactorings and code analysis.

Of course, I should get off my lazy ass and start the discussion in Roslyn, but it's amazing how little time I have for anything these days 🤕

[belav]

Interesting... if roslyn allows formatters, that would remove the need for a lot of the IDE integration work.

If we do start down the VSCode extension path, I ran across this link which would be useful - https://code.visualstudio.com/blogs/2016/11/15/formatters-best-practices

[shocklateboy92]

Okay, I chatted with @sharwell offline and he's of the opinion that unless a formatter handles all possible scenarios (including incomplete code that doesn't compile), it shouldn't go to Roslyn. While I don't fully agree, I respect his position. This means the roslyn path is a no-go.

I'll probably create a vim extension before VS-code one (because I use the former more than the latter), but we will need some changes in csharpier cli to make it work.

Keeping it running between formats

Currently, (especially for small files) it seems the dotnet process spends more time initializing than the actual meat of parsing/formatting. In order avoid the perf hit when running on save, we should keep csharpier running as long as the editor is.

We could do something complicated like make csharpier serve an http endpoint that takes the code and formats it, but I'm hoping we can get away with something simple and continue to use stdin. If we can make csharpier/dotnet not close the buffer on the EOF byte, we can use that to delimit multiple files/runs.

Keeping version in project

One of the reasons I wanted to push for a nuget/roslyn based approach like Roslynator is that then the formatter and version will be specified in the csproject as a nuget reference. That way, all team members who contribute to a specific project will format the same (despite has changing formatting rules between versions).

This doesn't really need changes in csharpier cli I suppose, but I'm mentioning it here for completeness. The way @sharwell suggested it was to use dotnet tool manifest. Essentially, don't ship a version of csharpier with the vs-code/vim plugin, but have it invoke it as a dotnet tool. That way, if a manifest specifies the version, it will use that, otherwise it will fall back to the user's global version.

[belav]

including incomplete code that doesn't compile

Right now csharpier spits back out the exact code it got when it fails to compile, I'm not sure if that would qualify as handling it.

Currently, (especially for small files) it seems the dotnet process spends more time initializing than the actual meat of parsing/formatting.

That was definitely the case when csharpier was still a prettier plugin. It started a new .net process for each file, which completely killed performance. I decided to port it to .net instead of trying to figure out how to keep the process alive. Serializing the AST to json was also taking a pretty big chunk of time and there didn't seem to be a good solution to that problem.

[saborrie]

I just saw #282 is done and you'd added support for stdin/stdout so I've had a go at a proof-of-concept vscode extension using that: https://github.com/saborrie/vscode-csharpier

it has this nodejs function:

import * as cp from "child_process";

function runFormatter(input: string): Promise<string> {
  return new Promise((resolve, reject) => {

    const csharpier = cp.spawn("dotnet", ["csharpier", "--write-stdout"], {
      stdio: "pipe",
    });

    let output = "";
    csharpier.stdout.on("data", (chunk) => {
      output += chunk.toString();
    });
    csharpier.on("exit", () => {
      resolve(output);
    });

    csharpier.stdin.write(input);
    csharpier.stdin.end();
  });
}

Hope this helps in some way. I'll post another comment here tomorrow (or soon) with some ideas I've had.

[belav]

Awesome! I don't know anything about writing vscode extensions (yet) so this will definitely help.

I'd like to focus on improving the formatting of csharpier, but once it gets to something we can call beta then take on things like the VSCode extension.

But if you or anyone else wants to work on the extension sooner just let me know if there is anything I can do to help

[saborrie]

I've been busy but I've tried to throw together some notes and a diagram:

Would it be best to move any vscode extension (plus any other extra packages) into the main Csharpier repo?
A monorepo might be easier to manage as new versions of Csharpier can be versioned and then released along with updates to all extensions.
Alternatively, does it make sense to create a csharpier github organisation to put all the csharpier related repos into (like https://github.com/prettier/prettier-vscode)?

Vscode is nodejs based, in my proof-of-concept extension I wrote a function in node which can call csharpier by spawning another process. I think it would make sense to set this up as a standalone node-csharpier package, which can then be imported into the vscode extension.

As shocklateboy92 wrote, it would be good to have a way to fix the Csharpier version used within a project - especially if the formatting of csharpier continues to be worked on. This means that there will need to be a way of installing a local version of csharpier on a per project basis. However you do this (perhaps with a node-csharpier install) then the vscode extension could look for a local version, and if not found fall back to the global version.

A possible structure:

[shocklateboy92]

Thanks for your work (and diagrams) @saborrie! Much appreciated ❤️

Would it be best to move any vscode extension (plus any other extra packages) into the main Csharpier repo?

Yes. Create a CSharpier.VsCode directory under Src/ of this repo.

Alternatively, does it make sense to create a csharpier github organisation to put all the csharpier related repos into

We will most likely do so in the future, but I don't think we need to just yet.

Vscode is nodejs based, in my proof-of-concept extension I wrote a function in node which can call csharpier by spawning another process.

How did you find the performance in your POC? Did it make saving things in VS Code feel "sluggish"?

I think it would make sense to set this up as a standalone node-csharpier package, which can then be imported into the vscode extension

Any particular reason? Do you envision many other projects needing to invoke csharpier in Js/Ts?

This means that there will need to be a way of installing a local version of csharpier on a per project basis. However you do this (perhaps with a node-csharpier install) then the vscode extension could look for a local version, and if not found fall back to the global version.

So, dotnet tools already support a per-workspace manifest/versioning like npm/yarn's package.json:
https://docs.microsoft.com/en-us/dotnet/core/tools/local-tools-how-to-use
I'm pretty sure when you spawn dotnet csharpier from child_process, it will already try the local version and then fall back to global.

You should add some helpful messaging in the UI explaining this to users and maybe some functionality to install a local/global version easily. Not sure what this would look like.

[saborrie]

Vscode is nodejs based, in my proof-of-concept extension I wrote a function in node which can call csharpier by spawning another process.

How did you find the performance in your POC? Did it make saving things in VS Code feel "sluggish"?

It was slightly sluggish - I even tried spawning the process before the formatting function call and then only piping stdin but it made no difference. I think it does need to be done with a single long running process rather than stopping and starting csharpier, but I'm not sure how. I might have suggested gRPC if not for seeing this: https://gist.github.com/badsyntax/9827722afcb33a4b0e03c809f1aede98. Perhaps it's possible to use protobuf over stdin/stdout which would have the advantage over just sending the formatted text as it can support metadata like time taken, errors etc.

I think it would make sense to set this up as a standalone node-csharpier package, which can then be imported into the vscode extension

Any particular reason? Do you envision many other projects needing to invoke csharpier in Js/Ts?

1. I was thinking this would make development of the node->csharpier communication easier. Even if you don't publish it to the npm registry, by separating it you can write a separate test suite for it and it can be worked on in isolation.

2. I'd mostly expect to see it in automation pipeline scenarios. It would be useful to have an easy way to invoke csharpier from nodejs for anyone using tools like gulp, or even just regular nodejs scripts.

This means that there will need to be a way of installing a local version of csharpier on a per project basis. However you do this (perhaps with a node-csharpier install) then the vscode extension could look for a local version, and if not found fall back to the global version.

So, dotnet tools already support a per-workspace manifest/versioning like npm/yarn's package.json:
https://docs.microsoft.com/en-us/dotnet/core/tools/local-tools-how-to-use
I'm pretty sure when you spawn dotnet csharpier from child_process, it will already try the local version and then fall back to global.

You should add some helpful messaging in the UI explaining this to users and maybe some functionality to install a local/global version easily. Not sure what this would look like.

I'll give that a try in the POC and see what it takes to detect whether it's installed in the local folder. I did think of a problem which is that the vscode editor might not actually be open in the same directory as any manifest files, and when particular files get formatted, they might be within a subdirectory that had a different manifest file than the root, plus vscode supports formatting unsaved/untitled files.

[belav]

As someone who knows almost nothing about writing console apps or stdin/stdout beyond what I learned working on csharpier and knows nothing about protobuf or gRPC....

I played around with trying to get a POC of a long running console app that could read multiple files from stdin. The potential solution I came up with was sending a specific set of characters to the app to indicate the end of a file. When the app sees those characters it formats what it already read, and then waits for more data on stdin.

It feels hacky, would need to key off of something that is guaranteed to not exist in a real file, couldn't really read line by line because csharpier needs to see the line endings, and I have no idea what reading character by character may do to performance.

There really should be a better way to do this, but trying to google it didn't get me anywhere.

The POC

  const longRunner = spawn("LongRunner.exe", [], {
      stdio: "pipe"
  });

  longRunner.stdout.on("data", chunk => {
      console.log(chunk.toString());
  });

  longRunner.stdin.write("test1\n")
  longRunner.stdin.write("blah blah\n")
  longRunner.stdin.write("ENDOFFILE\n")
  await sleep(100);

  longRunner.stdin.write("test2\n")
  longRunner.stdin.write("ENDOFFILE\n")
  await sleep(100);

using var streamReader = new StreamReader(
    Console.OpenStandardInput(),
    Console.InputEncoding
);
var stringBuilder = new StringBuilder();
var line = await streamReader.ReadLineAsync();
while (line != null)
{
    if (line == "ENDOFFILE")
    {
        Console.WriteLine("file: ");
        Console.WriteLine(stringBuilder.ToString());
        stringBuilder = new StringBuilder();
    }
    else
    {
        stringBuilder.AppendLine(line);
    }

    line = await streamReader.ReadLineAsync();
}

[shocklateboy92]

That's more or less what I was thinking, except using the EOF byte to signal the end of the file, instead of a string that can appear in a document like ENDOFFILE.

As I type this, it occurred to me that we could also base64 (or Json) encode the whole file, so we know where the end is. Less efficient, but safer since it can't be broken by any user input.

[belav]

That's more or less what I was thinking, except using the EOF byte to signal the end of the file, instead of a string that can appear in a document like ENDOFFILE.

I knew there was something like this that existed, but I wasn't failing when googling EOF character and NULL character.

I did some digging into the performance of CSharpier against a single file.
One thing to note, passing --fast will speed up the formatting by bypassing the syntax tree validation. It cut ~10% off the time for formatting a single file.

By far the slowest part is roslyn initializing behind the scenes. It is ~40% of the time. That cost is only paid for the first file that is formatted.
Finding/initializing the IgnoreFile is ~8% of the time.
Finding/initializing the ConfigurationFile is ~3% of the time.
The library used to deal with command line parameters takes ~3% of the time.
Nothing else really stood out.

Even if we could optimize those last three, that doesn't seem like enough of a gain to make the sluggish feel go away. Keeping csharpier running to format files seems like the way to go.

[pontusntengnas]

Hi, I made an extension for vscode just to test it out for my personal use and it seems to work OK for me so I thought I could share it here if it could be of interest to you.

https://github.com/pontusntengnas/CSharpier-vscode

• It uses the --fast option.
• It formats existing files in place and new files in the same stdin/stdout way as in @saborrie:s extension.
• It checks so that CSharpier is installed and link to this repo if not.

Its not super fast no, it works for me but could be improved, I see that you have been looking at a long running process and that seems like a good idea.

[saborrie]

Out of curiosity, I've had a go at length-delimited-protobuf over stdio between NodeJS and C#: https://github.com/saborrie/node-csharp-protobuf-stdin-poc

I don't know whether this is an ultimately better or worse choice than using the EOF byte - it is possible length delimited is a bad idea, but it does add some interesting capabilities:

• the ability to send information besides just the content of the file (e.g. settings determined by vscode) - I've illustrated this by adding a "setting1" field
• all the performance benefits of protobuf over JSON or BASE64 etc

the C# server looks like this:

using System;
using System.IO;

using Formatter.Protos;

using Google.Protobuf;

namespace server
{
    class Program
    {
        static void Main(string[] args)
        {
            using var stdin = Console.OpenStandardInput();
            using var stdout = Console.OpenStandardOutput();

            FormatRequest message;
            while ((message = FormatRequest.Parser.ParseDelimitedFrom(stdin)) != null)
            {
                var response = new FormatResponse
                {
                    Content = message.Content.ToUpper() + "!",
                    Ms = 10
                };

                CodedOutputStream codedOutput = new CodedOutputStream(stdout);
                codedOutput.WriteFixed32((uint)response.CalculateSize());
                response.WriteTo(codedOutput);
                codedOutput.Flush();
            }
        }
    }
}

and the NodeJS client looks like this:

const proto = require("protobufjs");
const path = require("path");
const cp = require("child_process");

const server = cp.exec("dotnet run", {
  stdio: "pipe",
  cwd: path.resolve(__dirname, "../server"),
});

const callbacks = [];

// hack to convice the server.stdout to return binary buffers
if (server.stdout._readableState) {
  delete server.stdout._readableState.decoder;
  delete server.stdout._readableState.encoding;
}

async function main() {
  const root = await proto.load(path.resolve(__dirname, "../Formatter.proto"));
  const FormatRequest = root.lookupType("FormatRequest");
  const FormatResponse = root.lookupType("FormatResponse");

  // every time the stream becomes readable, emit all the events from it.
  server.stdout.on("readable", function () {
    let sizeb;
    while (null !== (sizeb = server.stdout.read(4))) {
      const size = sizeb.readInt32LE();
      const bytes = server.stdout.read(size);
      var message = FormatResponse.decode(bytes);
      const callback = callbacks.shift();
      if (callback) {
        callback(message);
      }
    }
  });

  function sendMessage(request) {
    const requestBuffer = FormatRequest.encodeDelimited(request).finish();
    var response = new Promise((resolve) => {
      callbacks.push(resolve);
    });
    server.stdin.write(requestBuffer);
    return response;
  }

  console.log(await sendMessage({ content: "message 1", setting1: true }));
  console.log(await sendMessage({ content: "message 2", setting1: true }));
  console.log(await sendMessage({ content: "message 3", setting1: true }));
  console.log(await sendMessage({ content: "message 4", setting1: true }));
  console.log(await sendMessage({ content: "message 5", setting1: true }));
  console.log(await sendMessage({ content: "message 6", setting1: true }));
  console.log(await sendMessage({ content: "message 7", setting1: true }));
  console.log(await sendMessage({ content: "message 8", setting1: true }));
}

main();

[belav]

Here are some of my thoughts after I finally spent some time digging into this more.

Modifying csharpier to support a long running mode - we'd want it to work for a VSCode extension but also potentially for VS/Rider extensions and bash scripts.
VS and Rider could potentially use csharpier directly instead of working through the CLI. They would need to use the version of CSharpier that is specified by the dotnet-lools.json file, and I am not sure how to go about that.
I believe the precommit hook here runs csharpier once for each file. Ideally that could be modified to pipe all the files into a single long running instance to speed it up.

I built out a POC that sends a file name + file content to a long running .net process, delimiting the values with \u0003, which is the End of Text control character. It seemed more appropriate than the End of File control. The server portion of that code is more involved than the protobuf POC, but the client code is simpler.

I don't believe we will need to send anything to csharpier besides file name and optionally file content. And I don't think csharpier needs to respond with anything except for the file content.

I am inclined to go with delimiting values for now. If we need to send more configuration options, or we need to send back error messages then we could switch to protobuf.

Another possibility that I haven't really put any time into yet, would be calling c# directly from JS with something like https://github.com/tjanczuk/edge, but that would probably only apply to a VSCode extension.

[Ciantic]

I use this setup in VSCode:

1. Install Trigger task on save -extension

Then add this task:

Workspace tasks.json:

{
    "version": "2.0.0",
    "tasks": [
        // Other tasks omitted
        {
            "label": "format (current file)",
            "type": "shell",
            "command": "dotnet csharpier ${relativeFile}",
        },
    ]
}

Workspace settings.json:

{
    "triggerTaskOnSave.restart": false,
    "triggerTaskOnSave.on": true,
    "triggerTaskOnSave.tasks": {
        "format (current file)": ["**/*.cs"]
    }

}

It kind of works.

It's notable that official prettier plugin formats the code before saving, but csharpier does the saving twice: once with bad formatting, and then again when csharpier kicks in causing editor to "blink" on save. This is probably required tho because chsarpier is pretty slow, but I don't care, it works!

[belav]

I made good progress today on modifying CSharpier to stay running and a VSCode extension that pipes files to the running instance. The formatting speed is 🚀

[belav]

The initial version of this is out https://marketplace.visualstudio.com/items?itemName=csharpier.csharpier-vscode

[Ciantic]

Great! I assume it doesn't yet leave it running in the background as it gives me ~600ms time for formatting single file:

["INFO" - 12.04.42] Formatting started for c:\Source\dotNET\PersistentJobs\src\PersistentJobs\PersistentJob.cs.
["INFO" - 12.04.42] Formatted in 643.4020001888275ms

[Ciantic]

Hmm, I updated the global tool to v0.12, it now works faster, but too fast for it's own sake, it cuts my file in half! It seems to sometimes remove characters from the end of the file, causing it to corrupt.

I opened an issue #500

[belav]

Great! I assume it doesn't yet leave it running in the background as it gives me ~600ms time for formatting single file:

It'll only leave it running if csharpier is >= 0.12.0. It was pretty easy to make the extension work with < 0.12.0 as well by just piping a single file.