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

Registered custom schemas stopped working in 0.5.0 #216

Closed
itowlson opened this issue Aug 22, 2019 · 26 comments
Closed

Registered custom schemas stopped working in 0.5.0 #216

itowlson opened this issue Aug 22, 2019 · 26 comments

Comments

@itowlson
Copy link
Contributor

In 0.4.1 you could call registerContributor on the extension API object and get intellisense - e.g. code completion, red wigglies, hover text, etc. In 0.5.0 this stopped working - no error but you get no intellisense.

To reproduce, create a new VS Code extension via yo code and add an onLanguage:yaml activation event. Then paste the following as the new contents of src/extension.ts:

'use strict';
import * as vscode from 'vscode';

export async function activate(context: vscode.ExtensionContext) {
    await registerYamlSchema();
}

const SCHEMA = "foozzzzzz";

const schemaJSON = JSON.stringify({
    type: "object",
    properties: {
        version: {
            type: "string",
            description: "A stringy string string"
        }
    }
});

export async function registerYamlSchema() {
    const yamlPlugin = await activateYamlExtension();
    if (!yamlPlugin) {
        vscode.window.showWarningMessage("NO YAMLS");
        return;
    }

    yamlPlugin.registerContributor(SCHEMA, onRequestSchemaURI, onRequestSchemaContent);
}

function onRequestSchemaURI(resource: string): string | undefined {
    if (resource.endsWith('porter.yaml')) {
        return `${SCHEMA}://schema/porter`;
    }
    return undefined;
}

function onRequestSchemaContent(schemaUri: string): string | undefined {
    const parsedUri = vscode.Uri.parse(schemaUri);
    if (parsedUri.scheme !== SCHEMA) {
        return undefined;
    }
    if (!parsedUri.path || !parsedUri.path.startsWith('/')) {
        return undefined;
    }

    return schemaJSON;
}

const VSCODE_YAML_EXTENSION_ID = 'redhat.vscode-yaml';

export interface YamlExtension {
    registerContributor(
        schema: string,
        requestSchema: (resource: string) => string | undefined,
        requestSchemaContent: (uri: string) => string | undefined
    ): void;
}

export async function activateYamlExtension(): Promise<YamlExtension | undefined> {
    const extension = vscode.extensions.getExtension(VSCODE_YAML_EXTENSION_ID);
    if (!extension) {
        return undefined;
    }

    const extensionAPI = await extension.activate();
    if (!extensionAPI || !extensionAPI.registerContributor) {
        return undefined;
    }

    return extensionAPI;
}

Run this and test it with a file named porter.yaml containing a version attribute e.g.

name: HELLO
version: 0.1.0
description: "An example Porter configuration"

With vscode-yaml 0.4.1 installed, hovering over version gives a hover containing the description from the schema.

With vscode-yaml 0.5.1 (and, from other tests, I believe also 0.5.0), hovering over version produces hover requests in the verbose trace log, but no hover appears.

@itowlson
Copy link
Contributor Author

In 0.5.1, the extension activates successfully, but if you set a breakpoint on onRequestSchemaURI it is never hit.

@itowlson
Copy link
Contributor Author

The bug appears to have been introduced in the "Update to 0.5.0" commit itself (the previous commit works). The only relevant change in that commit is updating the language server from 0.4.1 to 0.5.4. So it looks like this is a language server issue...

@itowlson
Copy link
Contributor Author

It looks like the issue was introduced to the language server on 26 May. Here are the relevant commits from that time (links are to my fork, sorry, but nothing has been modified):

@squillace
Copy link

Hi @JPinkney, I'm sure you're swamped, but I wanted to tag you only because you helped us submit a PR a while back. The refactor breaks about 250,000 installations of the VS Code Kube extension among others -- and we ain't mad I promise! It happens, and we do it accidentally sometimes as well.

However, I was hoping to find someone with whom to collaborate on fixing this. Do let us know if we can team with someone. -- Ralph

@JPinkney
Copy link
Contributor

From my investigations, it looks like its because for 0.5.x we switched to using the JSON language server for validation, hover, document symbols.

Before (0.4.x and below), when the schema was getting resolved for validation and hover this would be called which integrates with the customSchemaProvider, loading in the new schema.

But now, since we are relying on calling features from the JSON language server that part of code will no longer be called and instead for hover this will be called and then that points to
this.

I haven't yet been able to figure out a solution for this but if you or anyone else can think of a way to do it I'm happy to implement it

@squillace
Copy link

Hi @JPinkney, thanks for the quick assessment. Yeah, let's have a look on our side, and see if we can help get this working again.

@itowlson
Copy link
Contributor Author

Here is what sinks us, from the Microsoft JSON language service:

let jsonSchemaService = new JSONSchemaService(params.schemaRequestService, params.workspaceContext, promise);

https://github.com/microsoft/vscode-json-languageservice/blob/68173c4595b662699927dd7c42cb6df1ee3f2205/src/jsonLanguageService.ts#L62

Although the JSON language service defines an interface IJSONSchemaService, it appears to provide no way for consumers to inject their own implementation. The YAML server implements the interface with the custom schema provider, but cannot tell the JSON language service to use it.

@itowlson
Copy link
Contributor Author

The available extension point (without forking) is SchemaRequestService; is there anything we can do with this? SRS is simply a function from a URI to, er, something - the documentation is not clear:

The result should the schema file comment, or,
in case of an error, a displayable error string

From usage it looks like this should fetch the schema file content, which could be implemented via the YAML extension's contributor callbacks.

@itowlson
Copy link
Contributor Author

Hello:

Requests for schemas with URLs not handled by the [JSON language] server are forwarded to the client through an LSP request. This request is a JSON language server specific, non-standardized, extension to the LSP.

https://github.com/microsoft/vscode/blob/master/extensions/json-language-features/server/README.md#schema-content-request

Can we handle that request on the client and pump it into the YAML schema contributor structure, even though we are using the JSON language service as a library rather than via the LSP?

@JPinkney
Copy link
Contributor

Just wanted to give a quick update: The only way I was able to reliably have the json language service invoke the schemaRequestHandler was by making the customSchemaProvider call right before I do a request to hover/validation. Then when I make the hover/validation request it correctly sends the custom schema request events to the client and the client sends the correct response back.

When trying this fix with the vscode kubernetes extension everything seems to work correctly. However, I don't know if there's any drawbacks to this approach or "gotch'as" that I haven't found yet.

I'm going to be working on a lot of tests around custom schemas today. I'll build a *.vsix file with these changes and hopefully we can have some people test to make sure all the functionality is working correctly and I can hopefully have a release out tomorrow.

I've created a PR on the language server side if you want to take a look: https://github.com/redhat-developer/yaml-language-server/pull/177/files

@JPinkney
Copy link
Contributor

@itowlson I've created a vsix file here and tried it with the vscode kubernetes extension and all the yaml based features seem to be working.

Can you give it a try from your side when you get a chance?

@itowlson
Copy link
Contributor Author

Trying the VSIX now - thanks heaps for the quick fix Josh!

@itowlson
Copy link
Contributor Author

porter-vscode looking good

@itowlson
Copy link
Contributor Author

I think you've got a bug where a contributed schema applies to files other than what you intended it to. For example, if I open porter.yaml in porter-vscode, you correctly show hover according to the Porter schema. But if I then open foo.yaml which is some random YAML file with no particular schema association, if it happens to contain keys that match the Porter schema then the hover for those keys is drawn from Porter.

It looks like doHover might be updating the language-service-level jsonHover object, modifying its jsonLanguageService instance to use the resolved schema on all YAML files...?

@itowlson
Copy link
Contributor Author

There's also a bug where validation seems to kick in only when you edit or open a file. If it's open at startup then you seem to get hover but no validation. Not sure if this was there before.

@itowlson
Copy link
Contributor Author

The Kubernetes extension seems to be working too.

@itowlson
Copy link
Contributor Author

Longer term I wonder if the fix is to have the schemaRequestService (which gets passed into the JSON language service) consult the customSchemaProvider. Then you don't need special handling in the hover and validation objects, and it might help avoid the issue where you're modifying the schema URI for the entire JSON language service. But your fix is definitely shorter... and I definitely don't understand how all this works so what you've done may well be safe after all!

@JPinkney
Copy link
Contributor

I found a much easier way to fix this issue that actually allows us to pass in the schemaRequestService directly so there is no more managing schemas inside of the jsonLanguageService. I've uploaded a vsix file here if you want to check it out.

One thing that was interesting when debugging though is that for some reason when you install the kubernetes extension this line: https://github.com/Azure/vscode-kubernetes-tools/blob/master/src/yaml-support/yaml-schema.ts#L41 is undefined. This means that the yaml-language-server is not given any schema back and that's why the validation doesn't work automatically on startup

@itowlson
Copy link
Contributor Author

Thanks for flagging that Josh - I will take a look and get it fixed on our side. I like the sound of your new design - will try the new VSIX when I get back to a computer.

@JPinkney
Copy link
Contributor

On thinking about it more, I think the validation issue when installing the kubernetes extension didn't appear before because we had to reload VSCode after an extension was installed. My guess is that completely bypassed the issue

@itowlson
Copy link
Contributor Author

The latest VSIX seems to work. I noticed that it wasn't showing hover tips for 'live' k8s resources (those viewed directly on a cluster via the k8s extension's Cluster Explorer), but that seemed to be the case in 0.4.1 as well so that's not a regression. I think the problem may be that the YAML extension only registers the file: and untitled: URL schemes with the YAML language server - 'live' resource views use the k8smsx: scheme and I'm not yet sure how to address this. I suggest we get the immediate fix out, and then circle round to that one - does that work for you?

Once again, I am very much appreciating your efforts on this.

@squillace
Copy link

@JPinkney here, too. Thanks for all the effort.

@itowlson
Copy link
Contributor Author

@JPinkney I sent a PR (#224) re the live resources issue, but that's orthogonal to the registered custom schemas issue - don't mean to distract you from this one!

@itowlson
Copy link
Contributor Author

Kia ora Josh, when do you expect to be able to ship the fix? Trying to decide whether I need to add a notification to the k8s extension advising people about the issue.

@JPinkney
Copy link
Contributor

The new release came out roughly 1 pm EST!

@itowlson
Copy link
Contributor Author

Doh! In that case, belated but heartfelt thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants