-
Notifications
You must be signed in to change notification settings - Fork 6k
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
adding enum support for golang #5631
Conversation
@marcusljx thanks for the PR but the CI reports the following errors:
Please take a look when you've time. |
Hi. Yes the reason is because golang does not allow tokens beginning with special characters. Are there any text manipulation lambda functions in swagger-codegen that I can apply on the enum template section? |
|
||
// List of EnumClasss | ||
const ( | ||
_abc EnumClass = "_abc" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marcusljx let's continue the conversation with the code here. I believe the starting "_abc" is causing the issue.
What would be the correct value looks like? "ABC"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are several functions you will need to override in the Go generator. Please have a look at https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/PhpClientCodegen.java#L648-L706 for reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the full line should look like:
ABC EnumClass = "_abc"
In the next line, EFG
should be the correct token.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry @wing328 I'm not exactly sure how to use those functions in the moustache template. Could you shed some light about how to use them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The generator would use them. I think what is being suggested is to include a similar override in the GoClientCodegen.java and GoServerCodegen.java
Just briefly reading, something like this may help.
@Override
public String toEnumVarName(String name, String datatype) {
...
String enumName = sanitizeName(underscore(name).toUpperCase());
enumName = enumName.replaceFirst("^_", "");
enumName = enumName.replaceFirst("_$", "");
...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added it but it still doesn't change anything in the generator. I'm not sure if I'm using the generator fields/sections correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please have a look at https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/php/model_generic.mustache#L69-L84
You can also use the -DdebugModels
flag to find out what mustache tags are available: https://github.com/swagger-api/swagger-codegen/wiki/FAQ#how-to-debug-swagger-codegen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe possible that the go files were rebuilt with an outdated codegen jar.
@@ -68,7 +68,7 @@ Name | Type | Description | Notes | |||
**float** | **float32**| None | | |||
**string_** | **string**| None | | |||
**binary** | **string**| None | | |||
**date** | **string**| None | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Odd; how was this changed back to time.Time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marcusljx did you make the PR based on master instead of 2.3.0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?? I don't think so. I forked the repo and only did my edits on 2.3.0
Okay I think I made a mistake with the forking+branching. I will close this and push another PR (properly this time I hope) |
PR checklist
./bin/
to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.sh
and./bin/security/{LANG}-petstore.sh
if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates)2.3.0
branch for breaking (non-backward compatible) changes.Description of the PR
Fixes #4459