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.

Sign up for GitHub

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

Jump to bottom

Add new button configuration schema #196

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

Add new button configuration schema #196

wants to merge 3 commits into from

Conversation

atakiya
Copy link

@atakiya atakiya commented Mar 25, 2023 •
edited

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

@atakiya
Added config validation and defaults for vscord.status.buttons
549875f 
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)
@atakiya
Moved comments from markdownDescription example to schema
6d7dc52
@atakiya
Removed uri format from button urls
9929b2c 
Forgot they could also be parameter expansion strings, which would throw a warning
@atakiya atakiya mentioned this pull request Mar 25, 2023
Open
nick22985
nick22985 reviewed Mar 26, 2023
View reviewed changes
Copy link
Collaborator

@nick22985 nick22985 left a comment •
edited

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.

@nick22985 nick22985 mentioned this pull request Mar 27, 2023
Closed
@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
Reviewers

@nick22985 nick22985 nick22985 left review comments

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

New button configuration not recognized by vscode
3 participants
@atakiya @nick22985 @xhayper