Change enum prefix detection strategy #145
Conversation
It will now find the common prefix among all case names instead of relying on the enum definition name.
|
This needs a few more tweaks to compile, I'll get to them later on today. |
|
Ok, this should be ready for a review now. I think this is a nice improvement in the naming of enums but it impacts a lot of stuff. It introduces one slight naming regression for MethodFlags cases because the inconsistent prefix on that enum means we don't identify a great prefix (more context here: #23) — Relevant part of the diff: - public static let methodFlagNormal = MethodFlags (rawValue: 1)
- public static let methodFlagEditor = MethodFlags (rawValue: 2)
- public static let methodFlagConst = MethodFlags (rawValue: 4)
- public static let methodFlagVirtual = MethodFlags (rawValue: 8)
- public static let methodFlagVararg = MethodFlags (rawValue: 16)
- public static let tatic = MethodFlags (rawValue: 32)
- public static let methodFlagObjectCore = MethodFlags (rawValue: 64)
- public static let `default` = MethodFlags (rawValue: 1)
+ public static let flagNormal = MethodFlags (rawValue: 1)
+ public static let flagEditor = MethodFlags (rawValue: 2)
+ public static let flagConst = MethodFlags (rawValue: 4)
+ public static let flagVirtual = MethodFlags (rawValue: 8)
+ public static let flagVararg = MethodFlags (rawValue: 16)
+ public static let flagStatic = MethodFlags (rawValue: 32)
+ public static let flagObjectCore = MethodFlags (rawValue: 64)
+ public static let flagsDefault = MethodFlags (rawValue: 1)The impact is that the |
|
I think it is ok to special-case the default case to get that name in place. I also think that we should special case the keys, because I am not a fan of "_1" as an enum value vs say "key1". |
Adjust linker settings since dynamic_lookup is macOS-specific
|
Sounds good, I'll get those changes in soon. |
Instead of prefixing with "_" when the name would be invalid, lets keep the original prefix.
|
Ok should be good now; anytime there would be an invalid swift name, it does the same thing it would have done before, so there is no change on those cases; e.g. we get Here's the diff in API: https://gist.github.com/PadraigK/af78939c3e597635d6db5dc98e7b6c64 |
…n virtual calls, we need to dereference the value passed
It will now find the common prefix among all case names instead of relying on the enum definition name.
Instead of prefixing with "_" when the name would be invalid, lets keep the original prefix.
| func commonPrefix() -> String { | ||
| map(\.name).commonPrefix()?.dropAfterLastUnderscore() ?? "" | ||
| } |
There was a problem hiding this comment.
Is commonPrefix often invoked to warrant providing this in an extension?
There was a problem hiding this comment.
It is used in 2 or 3 places. I did it like this to ensure consistent use at call sites, since things like default method parameter mappings need to follow the same pattern as the case definition.
There was a problem hiding this comment.
No idea how to mark this conversation as resolved since the button to do so isn’t on my end, but consider this conversation “resolved” for all intents and purposes.
Co-authored-by: Marquis Kurt <software@marquiskurt.net>
|
Thank you so much for this contribution, this a significant usability win. Merging it. |
Fixes issue: #144
The generator will now find the common prefix among all case names instead of relying on the enum definition name.
You can see the changes to the generated API in this gist: https://gist.github.com/PadraigK/af78939c3e597635d6db5dc98e7b6c64