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

Add json schema (#6749) #7646

Closed
wants to merge 1 commit into from
Closed

Add json schema (#6749) #7646

wants to merge 1 commit into from

Conversation

Freed-Wu
Copy link

Screenshot from 2024-01-20 23-24-42

@kchibisov
Copy link
Member

Thanks, but we're not interested in maintaining it.

it's also not clear what benefit it provides, since it can't be destributed and picked up by each program automatically, unlike the manual page with has a standard procedure of discoverability (apropos alacritty) and reading man 5 alacritty.

@kchibisov kchibisov closed this Jan 20, 2024
@Freed-Wu
Copy link
Author

it's also not clear what benefit it provides

Sorry for my forgetfulness to explain.

Refer cmhughes/latexindent.pl#206, json schema allow any editors supporting LSP to complete the configurations. In this screenshot, we complete window.opacity of alacritty.toml.

Before this PR, there is another repo to maintain it. Due to distinction-dev/alacritty-schema#3, I think move it to upstream will be better to get updating.

If you still reject it, please reply me. I'll push it to schemastore. However, I think the problem like #6749 will occur again when it cannot keep latest with official alacritty repo.

@Freed-Wu
Copy link
Author

it can't be destributed and picked up by each program automatically

It shouldn't be distributed. Refer cmhughes/latexindent.pl#206.

@kchibisov
Copy link
Member

As I've said, we're not interested in that. You already have documentation distributed and as I said this file can't be used by us, since it requires actual .toml config, but we don't have it and don't want to have it, and it can't be provided for each distro by the package, you point to some distribution of the schemes, but I'd rather not provide any of that at all, since such scheme can represent only the latest version, but not the version you're using, which is also solved by man since it's distributed along side your alacritty installation if you're using package manager (brew, any linux, doesn't matter).

@Freed-Wu
Copy link
Author

Freed-Wu commented Jan 20, 2024

You already have documentation distributed

Json schema is not documentation. 😢 It just be used for completion.

@KillDozerX2, @tplk maintain a json schema for old version alacritty. I'll take a look about if they can accept it.

@kchibisov
Copy link
Member

but it doesn't provide any value and can't guarantee the correct completions that's the thing. The point in the linked issue is the docs mismatch between the version user had and the one in schema.

And the popup completion you have confuses you, since it tells you things for the version schema was generated for, not the actual version you have, so yes, it's also documentation. Anything that tells you the options you have and what they mean also a form of documentation, otherwise why you have text embeded in the schema telling what the options are?

So to not confuse users, like it was on the issue, I'd rather not provide any schema at all, so they can use the standard flow to discover what options they have and what they mean for their version.

@Freed-Wu
Copy link
Author

since it tells you things for the version schema was generated for, not the actual version you have

The json schema of this repo should server latest version's config. IMO, if this program keep compatibility, the new version's json schema should validate old version's config successfully. Even if you add some breaking changes, schemastore allow you use two schemas like:

    {
      "name": "alacritty",
      "description": "alacritty config",
      "fileMatch": [
        "alacritty.toml"
      ],
      "url": "/the/url/of/new/version's/config",
      "versions": {
        "0.1.0": "/the/url/of/an/old/version's/config"
        // ...
      }
    },

But maintain many different version's json schemas are too troublesome. 😢

@Freed-Wu
Copy link
Author

In this problem, I guess the maintainer of schemastore know more? @hyperupcall @madskristensen

@hyperupcall
Copy link

hyperupcall commented Jan 21, 2024

@Freed-Wu

From my experience, most people use JSON Schema for completion and the convenience of having the documentation right within their editor. Some people use it for validation, but that is less common. Similar to projects such as DefinitelyTyped nothing is guaranteed in terms of absolute correctness, but I think it's still incredibly useful despite that.

It is difficult to maintain schemas for multiple versions if there is no way to autogenerate them. But, having schemas for multiple versions is usually not necessary if the shape of the config file is roughly the same, and keys are mostly added after ever release). In that case, the schema may "incorrectly" include config keys that have been removed in later versions (and visa versa) but the tradeoff is that the schema still provides hints and or does not error during validation for the people who have not yet updated. For example, that is the case with clangd, and I think that is what I would recommend in this case (from my possibly limited understanding of the alacritty schema across versions). There are plenty of tricks you can do to fix this from a different angle, especially with more recent versions of JSON Schema, but I haven't seen them much - probably because the time invested isn't worth the 100% correctness.

If the schema cannot be added upstream, then we can always accept them at SchemaStore. :)

About errors like in #6749, you can get around them by using additionalProperties: true. The tradeoff with that, is that validation will no longer error when it sees an unrecognized key, of course. But, you will still get documentation hints. Usually we discourage that, but if you really think it is important, then that is an option. Conversely, doing so will mean config keys added in newer versions of Alacritty are more likely not to be in the schema - no errors will be generated for people to notice (and then fix the schema).

About the file now being TOML instead of YAML, that does not matter since there are plenty of extensions like taplo's Even Better TOML Extension that can handle that.

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

Successfully merging this pull request may close these issues.

3 participants