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 new button configuration schema #196

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

atakiya
Copy link

@atakiya atakiya commented Mar 25, 2023

Basically a revert of the revert of d689dfb, but now supporting autocomplete and validation for both types of button configurations, among a couple fixes here and there.
Most, if not all of the values have been copied from the pre-existing ones.

Closes #195

Note: This is probably not ideal

  1. normal JSON schemas allow using $ref to minimize repetition, which is supposedly not supported in vscode extensions manifests, so a lot of the schema is repetitive (see https://code.visualstudio.com/api/references/contribution-points#Configuration-property-schema) - this makes maintenance in my opinion worse.
  2. Descriptions for most of the entries are missing, I'm not entirely sure how the buttons activate in regards to the git and non-git buttons, so I've left that out for now.
  3. I'm unaware of how the extension prioritizes the configuration. This is probably quite important to communicate to endusers, because one of them most likely overwrites the other and can cause confusion.
  4. Advanced schema tables like this do not support showing an UI in the vscode settings

This is mostly based on work from leonardssh@d689dfb

* Added interface types for buttons in config.ts
* Added default suggestions to config entries
* Added JSON-schema for the config object(s)
Forgot they could also be parameter expansion strings, which would throw a warning
Copy link
Collaborator

@nick22985 nick22985 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Yea I was planning to look into it when I realised the pull was reverted because it wasn't type-checking didn't get around to it.

Maybe tho change the config back to what it default was tho xD

@atakiya
Copy link
Author

atakiya commented Mar 26, 2023

Maybe tho change the config back to what it default was tho xD

Could you elaborate?
The schema defaults have been set to the same values that the other/old configuration options are set to.

@nick22985
Copy link
Collaborator

yea I think it has been changed previously and I didn't notice it lol. This is the new default dw sorry mb.

@atakiya atakiya marked this pull request as ready for review March 27, 2023 13:13
@xhayper xhayper added the enhancement New feature or request label Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New button configuration not recognized by vscode
3 participants