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

Adds the list of choices to the choice type #37

Merged
merged 1 commit into from
Jun 30, 2024

Conversation

jneug
Copy link
Contributor

@jneug jneug commented Jun 29, 2024

This allows access from other packages / templates to the list of choices available for this type.

THis allows access from other packages / templates to the list of choices available for this type.
@jneug
Copy link
Contributor Author

jneug commented Jun 29, 2024

As dicussed in #36.

Copy link
Member

@tingerrr tingerrr left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Although I wonder if this should be available as a field on all base-types with a value of none or (), to keep the internal structure of base-type consistent, what do you think @JamesxX?

@jamesrswift
Copy link
Member

On the one side, consistency would be nice, on the other, descriptive fields (especially when the shape of the data might change between schemas) might be the way to go

@jneug
Copy link
Contributor Author

jneug commented Jun 30, 2024

I'm not sure how this is supposed to work. A "choices" key wouldn't make much sense for other types than #choice. Or what do you mean by keeping it consistent? Types like #dictionary, #array and #either already expose additional keys that other types don't.

I can think of two kinds of types: "basic types" that check the type of the provided value against a list of allowed types and "complex types" that add some logic (assertions, pre-/post-transforms).

For these "complex types" unique keys might help identify them more reliably than the name. If a choices field exists, I'm dealing with some kind of choice type and not an array, while descendants-schema suggests the reverse.

@jamesrswift
Copy link
Member

Good points and well made. When I get home tonight I'll go through each of the schema generators and see what needs changing

@jamesrswift
Copy link
Member

In meantime, no reason why this shouldn't be merged. I'll keep open because there's some nice discussion on field names which will be important still

@jamesrswift jamesrswift merged commit a26c02f into typst-community:main Jun 30, 2024
1 check passed
@jamesrswift
Copy link
Member

One thing I'd like to change now that choices are stored in a field is for the assertion to read that field rather than keep it's own copy. It's a design I've tried to implement in the other generators so that (in this example) choices can still be changed after schema generation

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.

3 participants