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

implement table type access modifiers #313

Merged
merged 13 commits into from
Oct 13, 2024
Merged

Conversation

jackdotink
Copy link
Contributor

Implements table type access modifiers:

type foo = {
    read bar: number
    write baz: string,
}

@@ -19,6 +19,8 @@ pub enum TypeInfo {
Array {
/// The braces (`{}`) containing the type info.
braces: ContainedSpan,
/// The access modifer of the array, `read` in `{ read number }`.
access: Option<TokenReference>,
Copy link
Owner

Choose a reason for hiding this comment

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

Add #[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))]

Copy link
Owner

Choose a reason for hiding this comment

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

You didn't do this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added serde_option or whatever it is to the entire enum. If you'd rather have this, I can do that instead.

Copy link
Owner

Choose a reason for hiding this comment

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

Do you mean display_option? That's for display: the thing I gave is to not include it in serde if it's unset.

@Kampfkarren
Copy link
Owner

Need tests for this as well

Copy link
Contributor

@DataM0del DataM0del left a comment

Choose a reason for hiding this comment

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

This should require the syntax property access modifiers RFC to be implemented into the luau-lang/luau repository.
I don't think it's implemented yet, and I tried the example provided in the PR description and ran it with Luau, but it gave me a syntax error.

@jackdotink
Copy link
Contributor Author

@DataM0del try again because it works for me

@DataM0del
Copy link
Contributor

@DataM0del try again because it works for me

I'll update my Luau version. I wish though that Luau would have a --version command but the PR for that isn't moving because of complaints about how you have to change the version macro value to the current version or etc.

@DataM0del
Copy link
Contributor

image
I updated. It works now.

@DataM0del
Copy link
Contributor

@DataM0del try again because it works for me

Actually, it doesn't work because of the trailing comma.

pub struct TypeField {
pub(crate) access: Option<TokenReference>,
Copy link
Owner

Choose a reason for hiding this comment

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

This also needs the serde thing

@Kampfkarren Kampfkarren merged commit 511d4fb into Kampfkarren:main Oct 13, 2024
2 checks passed
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.

4 participants