Add support for workspace read requests

[Mark Sujew (msujew)]

Related to microsoft/language-server-protocol#1264 (does not fully resolve it, since this PR does not include features to write into a file system - only read from it).

Adds support for the server to read files/directories/stat info from the client.

As indicated by microsoft/language-server-protocol#1264 (comment), I also thought it'd be best to start with read-only access to the file system. However, the new interfaces/types should be extendable enough to also add write requests if required later on.

Some questions/considerations:

• Right now, if the read/stat fails, the LSP returns null. Should it return a response error instead?
• I'm not entirely sure about the properties in FileSystemClientCapabilities. Does it make sense to expose each individual request type as an opt-in flag? Or is it enough to provide a read?: boolean flag (maybe extend this with a write?: boolean flag later on)?
• The FileType.unknown value results from the fact that vscode offers vscode.FileType.unknown as a possible value to return for FileStat.type. Should this be included in the protocol, or should stats/directory entries with this type simply be omitted?
• The LSP has been using folder instead of directory for the most part. Should this change align to this? I've simply used the same nomenclature as vscode does.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[Mark Sujew (msujew)]

@microsoft-github-policy-service agree

[Dirk Bäumer (dbaeumer)]

Thanks. Very nice PR.

[Dirk Bäumer (dbaeumer)]

Can we make this a flag property with a bit wise implementation. Makes it easier to expand in the future.

[Mark Sujew (msujew)]

I thought about combining it with the type property, similar to how its done in vscode, but decided against it. Is this what you had in mind?

[Dirk Bäumer (dbaeumer)]

Mark Sujew (@msujew) no, I like that they are separate but I would rename isSymlink to flags and have a SymLink flag. If we have more flags in the future it is easier to extend.

[Mark Sujew (msujew)]

Ok, I believe that's what I've done in b356b60 already 👍

[Dirk Bäumer (dbaeumer)]

See above

[Martijn Bakker (Bakker-Martijn)]

What if we read a file that is binary? I have some library files and we need to obfuscate the source for end-users.
Because of the way the fileread is implemented it is always decoded using textdecoder.

const fileRead: FileSystemReadFileSignature = async (uri, encoding) => {
	try {
		const bytes = await vscode.workspace.fs.readFile(uri);
		const decoder = new TextDecoder(encoding || 'utf-8');
		return decoder.decode(bytes);
	} catch {
		return null;
	}
};

Does it make sense when encoding === 'buffer' we return the bytes? Or some other mechanism for this use-case?

I currently am already testing with this pull request and all works fine. But I have used middleware to overwrite the implementation. (Which is also fine for me)

[Mark Sujew (msujew)]

Does it make sense when encoding === 'buffer' we return the bytes? Or some other mechanism for this use-case?

Martijn Bakker (@Bakker-Martijn) WDYT about encoding: "base64" for this and then get the buffer as a base64 string? Otherwise we would need to type the result value as string | number[], with the number[] taking 3-4 bytes per byte in the original file, which we would need to pipe through the jsonrpc protocol (a >200% increase). base64 only increases the size by 33%. I'd be open to integrate this into the PR.

Dirk Bäumer (@dbaeumer) do you have an opinion on this?

[Martijn Bakker (Bakker-Martijn)]

Mark Sujew (@msujew) I had the same thought process. But this is not allowed: https://developer.mozilla.org/en-US/docs/Web/API/Encoding_API/Encodings

I believe the textEncoder only wants to convert to plain text (not base64 or similar). So this would not resolve my current issue.

I also notices the size increase. Which is not ideal...
I could also handle the decoding in the middleware. This way I can still send plain text through the pipe. However, End-users could see the text when looking at the verbose log of the language server. Not sure if I think this is an real issue though.

Just wanted to point this out. It also depends if binary files will be read by other languages servers or not (perhaps I am one of the few). Can imagine not implementing this, having the middleware as fallback is also OK for me.

--Edit:
Or do you mean skipping the textEncoder when encoding === "base64" and converting the uint8array to a base64 string?
That is also completely fine. I would say that this would indeed make more sense than sending number[] (looking at the size increase).

[Mark Sujew (msujew)]

Or do you mean skipping the textEncoder when encoding === "base64" and converting the uint8array to a base64 string?

Yes, exactly. Essentially just special casing the client code and documenting this into the protocol. I.e. servers can request binary file content via encoding: "base64", which clients should respect.

[Martijn Bakker (Bakker-Martijn)]

That would be perfect for my use-case. I think that is an excellent idea :)