VSpaceCode / vscode-which-key

which-key like menu for Visual Studio Code
https://vspacecode.github.io/docs/whichkey
MIT License
174 stars 18 forks source link

Better validation on binding settings #7

Open stevenguh opened 4 years ago

stevenguh commented 4 years ago

This is split from https://github.com/VSpaceCode/VSpaceCode/issues/78 and to track the work to have better error messages when which key binding json is incorrect.

stevenguh commented 4 years ago

Just jotting notes here. Thinking we can use JSON Schema with https://ajv.js.org. To handle requirements of different requirement of properties on the type name, we can use the if/then/else as described in https://json-schema.org/understanding-json-schema/reference/conditionals.html and https://ajv.js.org/keywords.html#ifthenelse

And we can use https://github.com/adobe/jsonschema2md to generate documentations

MarcoIeni commented 4 years ago

I've used json schema before. I could write the schema if you want.

Tell me:

stevenguh commented 4 years ago

Thank you for helping, it would be great if you can help as I am going to be busy in the next few weeks. The schemas are used to validate the bindings and bindingOverrides.

Thebindings contains an array of BindingItem(s).

BindingItem

key value
key required string
name required string
type required enum string (command, commands, bindings and transient)
command required string if type == command | (optional if type == transient && only if commands is not exists)
commands required array of strings if type == commands | (optional if type == transient && only if command is not exists)
args optional any object | array of objects if type == commands
bindings Array of BindingItem(s), required if type == bindings

The bindingOverrides contains an array of OverrideBindingItem(s).

OverrideBindingItem

key value
keys required (string | array of string)
position optional number
name optional string if keys is negative else required
type optional enum string as in BindingItem if keys is negative else required
command Same as BindingItem
commands Same as BindingItem
args Same as BindingItem
bindings Same as BindingItem

Can you put the schemas in src/scemas in a new branch? Then I can use the schema to validate finish this out.

MarcoIeni commented 4 years ago

if keys is negative else required

Do you mean position, right?

Furthermore, is args required if type == commands?

stevenguh commented 4 years ago

Do you mean position, right?

Sorry, yes.

Furthermore, is args required if type == commands?

They are optional as not all commands need args

MarcoIeni commented 4 years ago

Take a look at b3316781385e80

Are the examples valid? They are validated by vscode, but I don't think the schema is that accurate at the moment and I am not a json schema expert, so feel free to edit both schemas or examples if you want.

The biggest problem at the moment is that I don't know a better way to apply the following constraint to the schemas/binding.json file, too.

https://github.com/VSpaceCode/vscode-which-key/blob/b3316781385e80ce03bbff7c619558e5b12ac435/src/schemas/bindingOverrides.json#L49

Do you know how to solve this problem? If not, I will try to figure it out by myself and if I find no way I will just copy and paste.

stevenguh commented 4 years ago

The examples seems to be correct, although it doesn't encompass the whole spectrum. It would be still be a great testing point. Thank you for getting the preliminary schema up. I'd suggest to change the schema for bidningOverrides and bindings to the array type. Ultimately, we only care if the array user entered is correct.

I also added a testing code for loading schema and validating with Ajv. It is very preliminary, you can run it by

npm run test-compile
node out/test/suite/schema.test.js

Also make sure you run npm install to install the dependencies.

stevenguh commented 3 years ago

Kind of just jotting here. Apparently, we can add the (limited) schema into package.json so there is a prompt of possible value when people edit the settings.json For example:

"whichkey.bindings": {
    "type": "array",
    "items": {
        "type": "object",
        "title": "Binding",
        "properties": {
            "key": {
                "type": "string",
                "description": "Key of this binding in this level"
            },
            "name": {
                "type": "string",
                "description": "The display name of this binding"
            },
            "type": {
                "type": "string",
                "enum": ["command", "commands", "transient", "conditional"],
                "description": "The type of this binding"
            }
        }
    },
    "markdownDescription": "The bindings of the which key menu",
    "default": []
}

Maybe the hack mentioned in https://github.com/microsoft/vscode/issues/95270#issuecomment-662091151 can bypass the limited functionality. Also maybe be able to reuse across extensions like VSpaceCode and which-key