feat(config): add JSON schema for validation,completion,documentation #228

mikavilpas · 2024-03-23T07:51:09Z

Help users write and maintain their configuration

This change introduces a JSON schema for the configuration file. This schema is used to validate the configuration file and provide feedback to the user if the configuration file is invalid.

presenterm-json-schema-demo2.mov

To use this schema, the schema must be referenced in the yaml configuration file like this:

# yaml-language-server: $schema=./schema.json

See the documentation for this feature here: https://github.com/redhat-developer/yaml-language-server?tab=readme-ov-file#using-inlined-schema

This configures the yaml-language-server to use the schema.json file to validate the configuration file. The language server is well supported in editors such as

Neovim with nvim-lspconfig. Personally I also use mason-lspconfig.nvim to manage my language servers
Visual Studio Code with the YAML extension
More supported clients are listed in https://github.com/redhat-developer/yaml-language-server?tab=readme-ov-file#clients

Supported features

Autocompletion of keys in the configuration file
Hover support for keys in the configuration file
Validation of the configuration file
- Invalid keys are detected
- Invalid values are detected for values that are supposed to be of a certain type or numeric range
- Bundled in themes are suggested but any theme name is accepted (in case the user wants to use a custom theme)
the top sections of the config have a markdown link to the documentation when hovered

Testing the feature

To test this feature, you can do the following steps:

fetch this git branch for testing
open the configuration file in your editor
add the magic comment to the top of the file with the path to the schema.json file
- I used # yaml-language-server: $schema=./schema.json to develop this feature
(restart your yaml-language-server if necessary)
observe the autocompletion, hover and validation features by making edits in your editor

mikavilpas · 2024-03-23T11:56:04Z

Just realized one setting defaulted to a different value in the schema than what the actual default is. This is now fixed.

mfontanini · 2024-03-23T14:45:47Z

Love the goal of this! I wonder if this could be dynamically generated by using something like schemars. e.g. run presenterm --generate-config-json-schema or something and spit it to stdout.

My concern is this will eventually drift off from the config when new parameters are added and/or it is a bit harder for people to use given this is standalone file in the repo that they don't get if they download a binary artifact.

mfontanini · 2024-03-23T14:46:37Z

schema.json

+          "anyOf": [
+            {
+              "type": "string",
+              "enum": [


As an example of what I mean by drifting off, this list can easily get out of sync with the list of supported themes.

mikavilpas · 2024-03-23T15:17:58Z

Oh I see your point, and I think you are 100% correct.

I'll see if I can build the schema automatically based on the rust types 🤔 👍🏻

mikavilpas · 2024-03-23T19:33:28Z

question: It looks like AsciiBlocks is the default image rendering protocol and it cannot be configured in the configuration explicitly. Is this correct? I suppose it makes sense, but wanted to make sure.

mfontanini · 2024-03-23T21:17:43Z

It should be configurable. This works for me, as in, starting presenterm with this config will complain because I don't have sixel enabled during compile time:

defaults:
  image_protocol: sixel

mikavilpas · 2024-03-24T07:35:47Z

Ok, I have a working proof of concept available for review and feedback now.

This version is able to generate the schema using schemars. The schema seems to work and should now be easy to keep in sync with the code.

Open question: how should you think the build system should generate this schema?

Right now I just used a POC that runs when the program itself runs, but of course this cannot stay.
I am not sure if this is now a runtime dependency which might have an effect on the program's size. Ideally I think this could be totally left out at runtime, but I don't know if that actually happens as schemars works via adding attributes to types that are used in production

Do you have an idea how things like this are usually managed in rust projects?

mfontanini · 2024-03-24T21:30:25Z

I am not sure if this is now a runtime dependency which might have an effect on the program's size

I wouldn't worry about this. I'm generally cautious about adding dependencies but this is only pulling a couple and I think it's for a good cause so I don't mind the extra binary size.

What I was thinking though, is not to have the schema file in the repo at all and let people generate it manually if they feel like using it. e.g. if you're interested in using a yaml lsp, you can run presenterm --generate-config-file-scema > /some/path/i/like and then use the yaml lsp directive in your config file to point to this. Alternatively, we could at least add a step in the CI that verifies that generating the file spits out an identical file to whatever schema file we ship in the repo. Thoughts? I don't know if it's worth shipping it in the repo if people can generate it on their own.

mfontanini · 2024-03-24T21:33:35Z

src/custom.rs

+    schema.metadata().description =
+        Some("The theme to use by default in every presentation unless overridden.".to_string());
+    schema.instance_type = Some(SingleOrVec::Vec(vec![schemars::schema::InstanceType::String]));
+    schema.enum_values = Some(vec![


I know you spent some time to get this enum to work, but I'm not sure if it's the right way to go. Because you can have your own custom themes, this end up being misleading to users. I see that you do allow an arbitrary string as well, but I think if I see my autocompletion show a list of things (e.g. the build in themes), I would always assume I need to use a value from that list so I may get confused if I have a local custom theme and it's not showing up.

Sure, we can leave it out too. I'll revert it back to just a string with no suggestions for the built in themes.

mikavilpas · 2024-03-25T06:09:15Z

I wouldn't worry about this. I'm generally cautious about adding dependencies but this is only pulling a couple and I think it's for a good cause so I don't mind the extra binary size.

Okay 👍🏻
I was thinking it could also be a separate cargo project/script in the repo in order to clearly limit the dependencies out of the production build - but this is certainly simpler.

What I was thinking though, is not to have the schema file in the repo at all and let people generate it manually if they feel like using it. e.g. if you're interested in using a yaml lsp, you can run presenterm --generate-config-file-scema > /some/path/i/like and then use the yaml lsp directive in your config file to point to this. Alternatively, we could at least add a step in the CI that verifies that generating the file spits out an identical file to whatever schema file we ship in the repo. Thoughts? I don't know if it's worth shipping it in the repo if people can generate it on their own.

I would go for the second option (shipping a file), but I guess a switch for creating the schema might also be a good option for advanced users. It's almost no extra work to do it as you said you are fine with having the schema related dependencies being shipped.

I forgot to mention this, but here's my initial idea for how the schema could be built and used:

when the schema generation works, the schema can be committed to the repo so it's clearly associated with a git version
the example configuration could include the magic comment and point to the project's github url for the schema
- e.g. # yaml-language-server: $schema=https://github.com/mikavilpas/presenterm/blob/master/config-schema.json
there could be a build step in github that verifies the schema is up to date, and fail the build if it's not
there could optionally be a build step to verify the example configuration conforms to the schema that the configuration includes

This is essentially the setup that the lazygit project uses for their configuration (see their docs https://github.com/jesseduffield/lazygit/blob/master/docs/Config.md?plain=1#L19)

mikavilpas · 2024-03-25T17:05:47Z

Just added a command for creating the schema:

Let me know how you think the schema should be generated and if it should be added to the config.

I will also clean up the commits when everything is ready - wanted to keep the history super easy to understand for easier discussion.

mfontanini · 2024-03-30T15:47:21Z

Sorry, I've been busy and had no time to look into this.

I forgot to mention this, but here's my initial idea for how the schema could be built and used:

All of that sounds great! We can add a step in the CI in a separate PR but I think all of that sounds great 👌

mfontanini

This is great!

mfontanini · 2024-03-30T15:48:46Z

src/main.rs

@@ -31,6 +31,10 @@ struct Cli {
    #[clap(long, hide = true)]
    generate_pdf_metadata: bool,

+    /// Generate a JSON schema for the configuration file.
+    #[clap(long, hide = true)]


I don't know if I'd hide = true this, this should be visible for people doing a --help

Ah, nice catch. Probably a copy paste mistake, now fixed.

mfontanini · 2024-03-30T15:49:55Z

src/custom.rs

@@ -45,17 +48,24 @@ pub enum ConfigLoadError {
    Invalid(#[from] serde_yaml::Error),
 }

-#[derive(Clone, Debug, Deserialize)]
+// TODO(JsonSchema): suggest the valid themes based on themes.presentation.theme_names() but allow


As I commented before, I think this isn't necessary (this may be a leftover comment!)

Yep 😄
fixed now.

This adds a new command line flag that makes presenterm print out a JSON schema for the configuration. Usage: presenterm --generate-config-file-schema > schema.json The schema can be used for validation, completion and documentation of the configuration file. To use it, you have to have yaml-language-server set up for your editor. Add a magic comment to the top of the configuration file with the path to the schema: # yaml-language-server: $schema=schema.json

mikavilpas · 2024-03-31T06:37:45Z

Ok I think things are looking good now. Just rebased to the main branch and cleaned the commit history.

I'll see if I can work on the CI part next, but as far as I'm concerned, this PR is ready to be merged.

mfontanini · 2024-03-31T14:44:19Z

Thanks again!

mfontanini · 2024-03-31T14:44:41Z

I can do the CI part later today, no worries 👌

mfontanini · 2024-03-31T15:28:27Z

I think I'm going to rename the schema.json file to config-file-schema.json. I imagine we may want a schema for the theme file as well so this future proofs this. Will ping you in the next PR.

mfontanini reviewed Mar 23, 2024

View reviewed changes

mikavilpas force-pushed the configuration-json-schema branch from 3b26707 to abd24f1 Compare March 24, 2024 07:25

mikavilpas marked this pull request as draft March 24, 2024 07:26

mfontanini reviewed Mar 24, 2024

View reviewed changes

mfontanini approved these changes Mar 30, 2024

View reviewed changes

mikavilpas force-pushed the configuration-json-schema branch from 3dbcccf to da8cdbf Compare March 31, 2024 06:34

mikavilpas marked this pull request as ready for review March 31, 2024 06:38

mfontanini approved these changes Mar 31, 2024

View reviewed changes

mfontanini merged commit aa9f35f into mfontanini:master Mar 31, 2024
5 checks passed

mikavilpas deleted the configuration-json-schema branch March 31, 2024 19:09

mikavilpas mentioned this pull request Jun 18, 2024

Generate schemas automatically at build time yazi-rs/schemas#24

Open

BrewTestBot mentioned this pull request Jul 29, 2024

presenterm 0.8.0 Homebrew/homebrew-core#178870

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(config): add JSON schema for validation,completion,documentation #228

feat(config): add JSON schema for validation,completion,documentation #228

mikavilpas commented Mar 23, 2024

mikavilpas commented Mar 23, 2024

mfontanini commented Mar 23, 2024

mfontanini Mar 23, 2024

mikavilpas commented Mar 23, 2024

mikavilpas commented Mar 23, 2024

mfontanini commented Mar 23, 2024

mikavilpas commented Mar 24, 2024

mfontanini commented Mar 24, 2024

mfontanini Mar 24, 2024

mikavilpas Mar 25, 2024

mikavilpas commented Mar 25, 2024

mikavilpas commented Mar 25, 2024 •

edited

Loading

mfontanini commented Mar 30, 2024

mfontanini left a comment

mfontanini Mar 30, 2024

mikavilpas Mar 31, 2024

mfontanini Mar 30, 2024

mikavilpas Mar 31, 2024

mikavilpas commented Mar 31, 2024

mfontanini commented Mar 31, 2024

mfontanini commented Mar 31, 2024

mfontanini commented Mar 31, 2024

feat(config): add JSON schema for validation,completion,documentation #228

feat(config): add JSON schema for validation,completion,documentation #228

Conversation

mikavilpas commented Mar 23, 2024

Help users write and maintain their configuration

Supported features

Testing the feature

mikavilpas commented Mar 23, 2024

mfontanini commented Mar 23, 2024

mfontanini Mar 23, 2024

Choose a reason for hiding this comment

mikavilpas commented Mar 23, 2024

mikavilpas commented Mar 23, 2024

mfontanini commented Mar 23, 2024

mikavilpas commented Mar 24, 2024

mfontanini commented Mar 24, 2024

mfontanini Mar 24, 2024

Choose a reason for hiding this comment

mikavilpas Mar 25, 2024

Choose a reason for hiding this comment

mikavilpas commented Mar 25, 2024

mikavilpas commented Mar 25, 2024 • edited Loading

mfontanini commented Mar 30, 2024

mfontanini left a comment

Choose a reason for hiding this comment

mfontanini Mar 30, 2024

Choose a reason for hiding this comment

mikavilpas Mar 31, 2024

Choose a reason for hiding this comment

mfontanini Mar 30, 2024

Choose a reason for hiding this comment

mikavilpas Mar 31, 2024

Choose a reason for hiding this comment

mikavilpas commented Mar 31, 2024

mfontanini commented Mar 31, 2024

mfontanini commented Mar 31, 2024

mfontanini commented Mar 31, 2024

mikavilpas commented Mar 25, 2024 •

edited

Loading