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

feat: enhance multiple capabilities file format & fix mixed permissions schema #9079

Merged
merged 2 commits into from
Mar 5, 2024

Conversation

amrbashir
Copy link
Member

@amrbashir amrbashir commented Mar 5, 2024

This enhances the DX of defining multiple definitions in a single JSON file, previously the only way was:

{
  "capabilities": [ {...}, {...} ] 
}

now we can also support

[
  {...},
  {...}
]

toml however can't take advantage of this due to the nature of toml but doesn't matter that much

[[capabilities]]
identifier = "id"

[[capabilities]]
identifier = "another-one"

@amrbashir amrbashir requested a review from a team as a code owner March 5, 2024 02:51
@amrbashir
Copy link
Member Author

amrbashir commented Mar 5, 2024

This PR also fixes generation of PermissionEntry which made the generated schema not accept the following valid permission array:

{
  "permissions": ["perm1", { "identifier": "perm2" }]
}

Even though mixed permission array are parsed correctly in toml, the Event Better TOML extension in vscode still flags the following tomls as incorrect:

identifier = "allow-http"
windows = ["main"]

[[permissions]]
identifier = "allow-read"

and

identifier = "allow-http"
windows = ["main"]
permissions = ["app:allow-app-hide", { identifier = "allow-read" }]

This maybe a bug in Event Better TOML extension but I am not of aware of any other tool I can use to test and compare

Edit: tried to use taplo-cli to validate the schema and it worked fine, so not sure where the problem is. Anyways, since the format is supported by us and the schema generated is correct, this IDE problem shouldn't concern us.

Edit2: after closing and re-opening vscode, the validation seems to accept it and work fine so maybe after this PR schema fix, users won't have to deal with this at all.

@lucasfernog
Copy link
Member

the Even Better TOML extension schema validation isn't perfect, at least autocompletion doesn't always work. That's why we use JSON by default for capabilities.

@lucasfernog
Copy link
Member

a change file would be nice here i think

@amrbashir
Copy link
Member Author

Added

@lucasfernog lucasfernog merged commit bb23511 into dev Mar 5, 2024
24 checks passed
@lucasfernog lucasfernog deleted the feat/multiple-capabilities-file-format branch March 5, 2024 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants