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

Create "Autobuild fields from enum type param" #181

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Geokureli
Copy link

@Geokureli Geokureli commented Nov 6, 2024

There weren't any great cookbook examples of the following things

  • Using @:autoBuild to generate fields. Even the manual page for autoBuild doesn't actually do anything practical
  • Getting type params from a type, specifically an EnumValue
  • Get all values from an enum and inject them into code

I would also like to do this with @:genericBuild as there aren't many great resources for that either, but first, this may be a bit too meaty for a manual. I'm wondering if I should break it down into smaller ones

@Simn
Copy link
Member

Simn commented Nov 6, 2024

What's the purpose of a set of grades? That seems like a pretty odd use-case.

@Geokureli
Copy link
Author

Geokureli commented Nov 6, 2024

What's the purpose of a set of grades? That seems like a pretty odd use-case.

It doesn't make sense for grades, but I was planning to think of a better example, eventually. I just started Typing A,B,C and thought "maybe notes of a chord", but didn't want to add sharps/flats, so I put grades.

In reality, I would probably use something like this for a tagging system, since tag order doesn't matter and there's no need for multiple instances of the same tag

@Simn
Copy link
Member

Simn commented Nov 6, 2024

A set of chords would be strange as well. A set of notes though would make some sense in a program that determines a chord. For brevity just A-G would suffice here.

It ultimately doesn't matter but I always prefer when these examples make some sense.

@Geokureli
Copy link
Author

Went with the DnD 5e damage types, and chose a weapon with multiple types of damage

}

static function addFields(enumType:EnumType, fields:Array<Field>) {
final enumCT = Context.getType(enumType.module + "." + enumType.name).toComplexType();
Copy link
Contributor

@kLabz kLabz Nov 7, 2024

Choose a reason for hiding this comment

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

Build macros should avoid triggering typing (Context.getType here), especially in cases like that where it's avoidable. Best to keep bad practices out of cookbook

Copy link
Author

Choose a reason for hiding this comment

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

I'm still pretty new to macros, and their best practices. Why should this be avoided, and what is the better way to do this?

Copy link
Author

Choose a reason for hiding this comment

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

Oh I see, this line is no longer needed as the field isn't used

Comment on lines +18 to +33
try {
return switch Context.getLocalType() {
// Extract the type parameter
case TInst(getEnumType(_.get()) => enumType, _):
// Generate fields
addFields(enumType, fields);

fields;
case found:
throw 'Expected TInst, found: $found';
}
} catch(e) {
// Catch thrown errors and point them to the failing class
Context.error(e.message, Context.currentPos());
return Context.getBuildFields();
}
Copy link
Author

Choose a reason for hiding this comment

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

I've found that this is my preferred way of handling macro errors, using throw everywhere, catching them and converting them to a Context.error that points to the line that triggered the macro. This is more informative to the dev utilizing the macro, but less informative to devs adding to or maintaining the macro, which I fix by temporarily commenting out the try/catch.

Would it be better or simpler to just use Context.error everywhere and remove the try? Alternatively should I trim down the error handling altogether and just have a simpler but perhaps more fragile example that just needs to work for the given usage example?

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