-
Notifications
You must be signed in to change notification settings - Fork 256
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
other: add json schema #1394
other: add json schema #1394
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine with adding this in practice, though a few questions as someone who doesn't use this much:
- How do I (or others) maintain this if I make changes to the config file? If I have to upload to anywhere, can I automate this?
- How do users use this? Do they get this for free if they have LSP + some extension/plugin installed? Or do they have to do anything else?
- How do I manage things if I have to make changes to the config file (e.g. in the
main
branch) but haven't released a stable version with the changes? I'm guessing some messing around via$id
?
Just keep network connceted. Their plugin will download schema according to the list of URLs of schemastore.
IMO a good practice is use json schema to generate document about config and parse the config. It is future's work. Current PR is only the first commit about json schema.
I guess you means if user's bottom is a old version, the repo's json schema is a new version. If you keep the compatibility, the new version's json schema will can validate the old version's config successfully. 😄 If you do some incompatible changes which make old version's config cannot be compatible with new version's config, schemastore allow you use two json schemas like: {
"name": "bottom",
"description": "bottom config",
"fileMatch": [
"bottom.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. 😢 |
Oh, I think I misunderstand your problem. When you update your config, you should update json schema, too. Because schemastore only use url, so it will be updated automatically. |
These are my answers for your questions. If I have any incorrect part, the expert of json schema and the maintainer of schemastore (@hyperupcall @madskristensen) can point it 😄 |
I don't think I would call myself a JSON Schema expert - the only thing I would like to add is that for configuration files like these, most of the time there is a single schema for all versions of the file. It is a good tradeoff for time and the usefulness of the schema (autocorrect, documentation, etc). Sometimes, this is good enough for validation as well, but not always and I wouldn't personally ask a project to start using JSON Schema for validation just because it makes it easier to create schemas.
For most config files, there is just a single schema. Whether it is hosted upstream or at SchemaStore, usually it is updated when HEAD changes or there is a new release. (We rely on contributors to do this). When a key is added a new key is added to the schema, while deprecated or removed keys are marked as deprecated or marked as removed. They usually aren't actually removed from the schema because the schema is used to validate against all versions.
In the SchemaStore repository there is a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made some small changes, but otherwise I'm good with adding this then, looks like this won't be too much effort to support.
I can handle the follow-up PRs to add this to schema store as needed. Thanks for bringing this idea to my attention!
@all-contributors please add @Freed-Wu for code, docs. |
I've put up a pull request to add @Freed-Wu! 🎉 |
Description
~/.config/bottom/bottom.toml
:Issue
If applicable, what issue does this address?
Closes: #1382
Testing
If relevant, please state how this was tested. All changes must be tested to work:
If this is a code change, please also indicate which platforms were tested:
Checklist
If relevant, ensure the following have been met:
cargo fmt
)README.md
, help menu, doc pages, etc.)