Support x-enum-varnames extension - #692
Conversation
x-enum-varnames can be used to have an other enum name for the corresponding value. This is used to define names of the enum items.
|
@reedom I took the liberty to rebase your branch on top of master. should I make a new pull request @deepmap-marcinr? |
|
@amery |
Excuse me, I don't get the point. |
|
@reedom for me it doesn't render the consts unless I add a path. so: paths:
'/types':
get:
responses:
'200':
description: OK
content:
'application/json':
schema:
type: array
itemsUnique: true
items:
$ref: '#/components/schemas/ObjectType'
components:
schemas:
ObjectType:
title: ObjectType
type: string
enum:
- 'A'
- 'B'
- 'C'used to produce: now it produces unless there is something else at play here |
|
It seems doesn't relate to this patch and I think that's how package: sample
generate:
models: true
compatibility:
always-prefix-enum-values: true |
|
perfect, thank you @reedom. I had the feeling I had missed something |
|
@deepmap-marcinr @reedom master...amery:oapi-codegen:feature/enum-varnames has been rebased over 1.12.2 |
stevenh
left a comment
There was a problem hiding this comment.
Looks like some debugging got left in.
| if 0 < len(schema.ExtensionProps.Extensions) { | ||
| //fmt.Fprintf(os.Stderr, "%#v\n\n", schema) | ||
| } |
There was a problem hiding this comment.
Remove old debugging?
| if 0 < len(schema.ExtensionProps.Extensions) { | |
| //fmt.Fprintf(os.Stderr, "%#v\n\n", schema) | |
| } | |
| if 0 < len(schema.ExtensionProps.Extensions) { | |
| //fmt.Fprintf(os.Stderr, "%#v\n\n", schema) | |
| } |
|
+1 to |
I just rebased and amended my branch to exclude this. thanks @stevenh |
|
Can you resolve those conflicts above, then I'll happily merge this |
@deepmap-marcinr should I make a new pull request from my branch? |
|
@deepmap-marcinr as #880 was merged, this one can be closed |
|
This looks useful. I've not merged this yet because I have a few stability concerns. Your enum naming is dependent on order, and it's really easy to mess up by reordering one list and not the other. I think a mapping would work better, in your example: components:
schemas:
Object:
properties:
category:
type: integer
enum: [0, 1, 2]
x-enum-varnames:
"1": notice
"2": warning
"3": urgentThe complications in this approach are that key names must be expressible in json, so they must be strings, even though YAML supports other types. However, I'm not going to push back on this, because the tradeoffs are complex. |
| for _, key := range []string{extEnumVarNames, extEnumNames} { | ||
| if _, ok := schema.ExtensionProps.Extensions[key]; ok { | ||
| if extEnumNames, err := extParseEnumVarNames(schema.ExtensionProps.Extensions[key]); err == nil { | ||
| enumNames = extEnumNames |
There was a problem hiding this comment.
Right now, your extension completely replaces the enum names that are autogenerated. What happens when the provided names array has a different length than possible enum values? I think we'll end up creating invalid enums. Can you, at a minimum, error out on cardinality mismatch?
|
Thank you for contributing, and I'm very sorry for taking so long to get to this PR. At this point, the code has changed so much that it's no longer relevant because this was superseded by PR #880 which was merged in Dec 2022. The |
Summary:
Support
x-enum-varnamesextension, derived from openapi-generator.With it, we can supply other enum names for the corresponding values.
After code generation you will get this result: