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

Support multiple schemas #4

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

Conversation

vhdirk
Copy link
Contributor

@vhdirk vhdirk commented Jan 18, 2024

This PR adds initial support for multiple schemas in a single gschema file.
It's pretty basic and there's probably a lot of caveats, for instance:

  • reusing flags/enums will cause these to be generated twice as aux tokenstream. Having a different key in each schema will make different flag structs/enums, which won't be compatible.

  • choices using the same key will not work

Is this something you would be willing to pull in?

Edit: I have some ideas to fix this fix the possible issues regarding flags and enums. I propose to to that in an additional PR to keep changesets small.

src/lib.rs Outdated Show resolved Hide resolved
@SeaDve
Copy link
Owner

SeaDve commented Jan 19, 2024

Thanks for the PR! I did a quick review, but I'll do a full review next week, after my finals.

@SeaDve
Copy link
Owner

SeaDve commented Jan 27, 2024

reusing flags/enums will cause these to be generated twice as aux tokenstream. Having a different key in each schema will make different flag structs/enums, which won't be compatible.

I'd like to have this fixed first before merging the MR.

choices using the same key will not work

Not sure what you meant by that

Ah, I now get what you meant. Some possible solutions I can think of are:
a. Namespace the enums (adding a prefix or moving it to a module), or;
b. Let the user handle it by suggesting to put each settings struct in their own module (add a useful error message).

@@ -0,0 +1,99 @@
<?xml version="1.0" encoding="utf-8"?>
Copy link
Owner

@SeaDve SeaDve Jan 27, 2024

Choose a reason for hiding this comment

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

I think having one of each key for each child schema would be enough for testing

@vhdirk vhdirk force-pushed the multiple-schemas branch 4 times, most recently from f035bbd to ff78919 Compare February 22, 2024 10:13
@vhdirk
Copy link
Contributor Author

vhdirk commented Feb 22, 2024

I've updated my PR so that it now promotes flags and enums to be standalone, decoupling them from individual schemas.
Since I haven't found any other (simple) way to do this, I've added a 'globals' attribute to the macro, that indicates whether or not they should be generated

@SeaDve
Copy link
Owner

SeaDve commented Feb 24, 2024

Looking good! 👍

Since I haven't found any other (simple) way to do this, I've added a 'globals' attribute to the macro, that indicates whether or not they should be generated

Is there a specifc reason why would the user prefer not to generate globals?

@vhdirk
Copy link
Contributor Author

vhdirk commented Feb 25, 2024

Is there a specifc reason why would the user prefer not to generate globals?

Well, yes. Take the enum here for instance.
When generating multiple schemas, there's no way to tell of those global structures have been defined before. proc_macro's do not have the ability to look at previously generated code, and while you can keep a thread_local with information regarding previous invocations, you can't know the context that code was generated in. So, you have to set the flag manually.

The alternative would be to just generate everything in the schema in one go, but I've been working on that and it's quite a bit of work, TBH.

@SeaDve
Copy link
Owner

SeaDve commented Mar 9, 2024

Ah, that makes sense indeed.

I wonder if it is worth it to push through something like:

gen_settings! {
    const FILE = "...";

    #[gen_settings(id = "...")]
    pub struct Settings;

    #[gen_settings(id = "...")]
    pub struct AnotherSettings;
}

It is a rough scratch of the API, but I think something like that would be less confusing and less thinking on the user side whether to generate the globals.

@SeaDve
Copy link
Owner

SeaDve commented Mar 9, 2024

We can also copy quote_spanned syntax:

gen_settings! {"path/to/schema/file"=> 
    #[gen_settings(id = "...")]
    pub struct Settings;

    #[gen_settings(id = "...")]
    pub struct AnotherSettings;
}

or

#[gen_settings(file = "...")]
#[gen_settings_skip(signature = "(ss)")] // global scope
mod settings {
    #[gen_settings(id = "...")]
    #[gen_settings_skip(signature = "(ss)")] // local scope
    pub struct Settings;

    #[gen_settings(id = "...")]
    pub struct AnotherSettings;
}

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