Add JSON schema for config

[EmilyGraceSeville7cf]

• PR Description
  Adds JSON schema to enable config validation.
  
  

• Please check if the PR fulfills these requirements

• Cheatsheets are up-to-date (run go run scripts/cheatsheet/main.go generate)
• Code has been formatted (see here)
• Tests have been added/updated (see here for the integration test guide)
• Text is internationalised (see here)
• Docs (specifically docs/Config.md) have been updated if necessary
• You've read through your own file changes for silly mistakes etc

[EmilyGraceSeville7cf]

Can somebody clarify color format being used here in config?

[EmilyGraceSeville7cf]

I have almost done. The following features are not implemented yet:

• custom keybindings support

[EmilyGraceSeville7cf]

BTW, here is my script to speed up schema creation in Nushell:

# Convert YAML files with no nesting to JSON schema `properties` key contents.
# YAML key values are supposed to be `default` values in JSON schema.
def main [file_to_convert: string] {
    let content = open $file_to_convert | to json

    $content |
        jq 'with_entries(select(.value | type != "object") | .value = { 
                title: (.key | sub("(?<a>[A-Z])"; " \(.a)"; "g") | ascii_downcase),
                description: ("A " + (.key | sub("(?<a>[A-Z])"; " \(.a)"; "g") | ascii_downcase) + " keybinding\nhttps://github.com/jesseduffield/lazygit/blob/master/docs/Config.md#default"),
                type: (.value | type),
                default: .value
            })' |
        xclip -selection clipboard
}

[stefanhaller]

Thanks for working on this, it looks extremely useful!

You might add a bit more information to the PR description about how to use it. It took me a while to figure this out.

A few thoughts from my side:

• Do we need this to be versioned? We make changes to the config fairly often, e.g. by adding new keys, or by adding new enum values to an existing key, or sometimes even by renaming existing keys. We need to create a new version of this file whenever this happens. (However, if people add a mode line to their config file, then it's unclear when the schema version is ever updated, as lazygit doesn't write the file. Users would be responsible for doing that whenever there's a new version.)
• Alternatively, we could only maintain the latest version of the schema; however, this means that people who didn't upgrade to the latest lazygit version yet (e.g. because their package manager is slow to update its lazygit package) may start to see warnings about unknown keys because they have been renamed in the latest version.
• Either way, it's a maintenance burden; whenever people add new things to the config, we need to adapt the schema. This is easy to forget, so we probably need at least a checkbox in the PR template. I wonder if we can think about some automated way of testing validity of the schema on CI, e.g. by trying to validate the default config from docs/Config.md against the schema. We need to be sure that the benefit is big enough to justify all this cost.
• When opening the config file from within lazygit (by pressing e in the status panel) and the file doesn't exist, we should consider creating one that contains # yaml-language-server: $schema=https://raw.githubusercontent.com/jesseduffield/lazygit/master/schemas/0.0.7/config.json so that new users get the benefit automatically. (This is easier when the schema is not versioned, see above.) Could be a follow-up PR, of course.
• I'm missing a bit of documentation. A few sentences about how to make use of this in VS Code or other IDEs could be added to Config.md, maybe along with a link to something like this page for more information.

[jesseduffield]

I share @stefanhaller 's thoughts re: maintenance burden. Some thoughts:

• I'd like to explore using something like https://github.com/invopop/jsonschema to generate the schema directly from the config struct using reflection. That way we can have a command to re-generate the schema whenever we update the config. We could have a catch-all command that generates keybinding docs, the json schema, and anything else you need before raising a PR.
• I agree about versioning concerns. I would really like to find a way to not have to do versioning. Even if that means leaving deprecated keys in the struct and tagging them as deprecated. But that won't save us when we want to make structural changes (I'm anticipating this with custom commands, though in that case we'd probably want to version the commands anyway)
• Once we've got a good idea of how we can easily keep the schema in-sync with the struct, we'll need to consider how big a maintenance burden that it vs the benefit

EDIT: also, if we go all-in on a json schema, it would be good to then generate markdown documentation from it e.g. via https://github.com/adobe/jsonschema2md
So we'd have the struct definition in pkg/config/user_config.go which is used to generate the json schema which is used to generate the docs. This could take us from 2 touchpoints (Config.md and user_config.go) to just one.

[EmilyGraceSeville7cf]

Do we need this to be versioned? We make changes to the config fairly often, e.g. by adding new keys, or by adding new enum values to an existing key, or sometimes even by renaming existing keys. We need to create a new version of this file whenever this happens. (However, if people add a mode line to their config file, then it's unclear when the schema version is ever updated, as lazygit doesn't write the file. Users would be responsible for doing that whenever there's a new version.)

I think it depends also on how easy is to support such versioned thing. AFAIK it's not always straightforward. I suggest add some version key to be able to instantly tell what config version we are using. Task project uses this approach for instance.

Either way, it's a maintenance burden; whenever people add new things to the config, we need to adapt the schema. This is easy to forget, so we probably need at least a checkbox in the PR template. I wonder if we can think about some automated way of testing validity of the schema on CI, e.g. by trying to validate the default config from docs/Config.md against the schema. We need to be sure that the benefit is big enough to justify all this cost.

Totally agree here. But as for start I can just copy this schema to SchemaStore directly to get at least some assistance while working with config. Later I plan to automatically generate it and put generation result in this repository.

When opening the config file from within lazygit (by pressing e in the status panel) and the file doesn't exist, we should consider creating one that contains # yaml-language-server: $schema=https://raw.githubusercontent.com/jesseduffield/lazygit/master/schemas/0.0.7/config.json so that new users get the benefit automatically. (This is easier when the schema is not versioned, see above.) Could be a follow-up PR, of course.

We don't need to even do that: we can just put a reference to schema from this repo like I did for another project. There is just one thing to keep in mind: our JSON schema should exist as a JSON file which means it should be generated (or manually written) before it's referred to.

I'm missing a bit of documentation. A few sentences about how to make use of this in VS Code or other IDEs could be added to Config.md, maybe along with a link to something like this page for more information.

You should have YAML Red Hat VS Code extension preinstalled to enable intellisence in config file. Yes, just that: installed extension. It automatically pulls schema and validate config against it. As a sidenote: if a user can create their own schema for our config and want to use it they can use their local schema instead of remote one. This is useful when they wanna add more hints to JSON schema matching their personal workflow.

[stefanhaller]

Registering the schema with SchemaStore is nice, but we'd probably have to rename our config file first to something like "lazygit-config.yml", don't we? "config.yml" feels a bit too generic to register. (Although I see that "config.json" was also registered, for "ASP.NET project config file", so there's precedence...)

I also find Jesse's suggestion to auto-generate the schema using https://github.com/invopop/jsonschema super attractive. I'd combine this with trying to use https://github.com/invopop/yaml for reading/writing (to avoid having to put in both yaml and json tags); this might also give us other benefits, e.g. being able to warn about unrecognized fields to catch typos. @EmilySeville7cfg Are you interested in working on this?

[EmilyGraceSeville7cf]

Are you interested in working on this?

Definitely interested. ;)

[EmilyGraceSeville7cf]

Feel free to test my JSON schema. It's ready for usage. If everything is fine we can put this schema in this repo, I can refer to it from SchemaStore and in the next PR implement it's autogeneration.

UPD: all errors are fixed (default config passes without any issues).

[stefanhaller]

customCommands is not right yet. It's an array.

But honestly, I'm not sure it makes sense to continue working on manually crafting the schema, when we agreed we want to auto-generate it. I'm not sure I feel comfortable with the approach of merging this now, and then only switch to auto-generation later.

[EmilyGraceSeville7cf]

I've put this schema directly to SchemaStore. When PR is merged users automatically get completions for **/.config/lazygit/config.yml and **/.config/lazygit/config.yaml files.

Should I close this PR and implement JSON schema generation in a separate one or not?

[stefanhaller]

I've put this schema directly to SchemaStore. When PR is merged users automatically get completions for **/.config/lazygit/config.yml and **/.config/lazygit/config.yaml files.

Very nice, huge usability improvement, thanks. (I've commented on that PR about the file patterns.)

We still need to tell people about it, so once that PR is merged, it would be good to mention the Red Hat extension in Config.md. So a separate PR for that would be welcome.

Should I close this PR and implement JSON schema generation in a separate one or not?

Since the manually created schema is now already on schemastore, I think there's no point in committing it here, too. So I'd vote for not merging this, and instead work on auto-generation. But that's just my personal opinion, I'll leave this for Jesse to decide.

[jesseduffield]

Agreed: no need to merge this: let's focus on auto-generating the schema

[jesseduffield]

Let me know if you're up for working on that @EmilySeville7cfg :)

[stefanhaller]

Let me know if you're up for working on that @EmilySeville7cfg :)

If not, I'd be interested in giving it a try. I don't want to take it from you, please go ahead if you want to; just in case.

And I do think it's important that we solve this soon; I'm working on a PR that adds a new user config, and I'm getting a big red warning in the editor when I try to put it in. It would be unfortunate if users see this with a new release.

[EmilyGraceSeville7cf]

Let me know if you're up for working on that @EmilySeville7cfg :)

Currently, I am adding JSON schemas for all lazy* projects such as lazydocker, lazynpm, etc. In a few days, I think I gonna start working on schema generation. When some JSON schema becomes autogenerated I remove it from SchemaStore and reference to lazy* repo (where it exists).

[stefanhaller]

Awesome, sounds good. Let me know if you need a rubber duck to bounce implementation ideas off of. 😄