Conversation
benjajaja
left a comment
There was a problem hiding this comment.
I would just like to be able to set the default value, and also the suffix: we use "not_set" or similar in our codebase, but also probably not 100% consistently.
Also in general I would be inclined to call it "default" instead of unknown.
| lowercaseLookup bool | ||
| caseInsensitive bool | ||
| marshal bool | ||
| unknownCase bool |
There was a problem hiding this comment.
| unknownCase bool | |
| unknownCase string | |
| unknownCaseValue string | |
| unknownCaseSuffix string |
I would make this as customizable as possible here. Allow setting a default value, and the suffix. For example in re:cap BE, the value is usually NOT_SET.
(We can't just use unknownCase string, in case someone wants to use an empty string as default).
There was a problem hiding this comment.
I thought about this, but was wondering how to have a different syntax between renaming and setting the default case:
type Gender string
// This renames between type and value:
// ENUM(male, female, unknown=not_set)
// Maybe something like this?
// ENUM(male, female) default=not_setThis is a bit more work because we need to have extra parsing but doable.
What difference do you make between unknownCase and unknownCaseValue? And by suffix you mean the naming of the default type value right?
There was a problem hiding this comment.
You could just add it to the CLI args for now.
unknownCase:bool-> enable if trueunknownCaseValue:*string-> set default value if not nilunknownCaseSuffix:*string-> replace theUnknownsuffix if not nil
Actually, the args would have to be *string, as "" would be a valid use case for the value, and unknownCaseSuffix is optional.
I'm not sure if "suffix" is actually needed, what we need (would be great) to make work is our use case that we have elsewhere:
// ENUM(first_item)
// should produce:
const OurEnumFirstItem OutEnum = "first_item"
const OurEnumNotSet OutEnum = "NOT_SET"mixed upper/lower, and snake_case for the values.
There was a problem hiding this comment.
But anyway don't let me drag this out too much, we can also change this later.
There was a problem hiding this comment.
The CLI args apply for all enums defined in the file though, so I think if we have customization of the value and suffix, I think we should do it at the enum level 🤔
There was a problem hiding this comment.
Right, that way it's more flexible and the //go generate ... trigger can be the same across files 👍
|
Overall this looks great! Looking forward to using it elsewhere in our codebase. |
No description provided.