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.

Sign up for GitHub

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

Jump to bottom

adding enum support for golang #5631

Closed
wants to merge 3 commits into from
Closed

adding enum support for golang #5631

wants to merge 3 commits into from

Conversation

marcusljx
Copy link
Contributor

@marcusljx marcusljx commented May 12, 2017 •
edited
Loading

PR checklist

  • Read the contribution guidelines.
  • Ran the shell/batch script under ./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)
  • Filed the PR against the correct branch: master for non-breaking changes and 2.3.0 branch for breaking (non-backward compatible) changes.

Description of the PR

Fixes #4459

  • Added generation of enums according to standard golang enum style.
  • Modified description template to use token name at beginning (standard go docstring convention)

@wing328
Copy link
Contributor

wing328 commented May 12, 2017

@marcusljx thanks for the PR but the CI reports the following errors:

# _/home/travis/build/swagger-api/swagger-codegen/samples/client/petstore/go/go-petstore
go-petstore/enum_class.go:18: syntax error: unexpected -, expecting )
go-petstore/enum_class.go:19: non-declaration statement outside function body
go-petstore/enum_class.go:19: syntax error: unexpected name, expecting semicolon or newline
FAIL	_/home/travis/build/swagger-api/swagger-codegen/samples/client/petstore/go [build failed]
[ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.2.1:exec (go-test) on project Goswagger: Command execution failed. Process exited with an error: 2 (Exit value: 2) -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :Goswagger

Please take a look when you've time.

cc @guohuang @antihax @neilotoole

@wing328 wing328 added this to the v2.3.0 milestone May 12, 2017
@marcusljx
Copy link
Contributor Author

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?

wing328
wing328 reviewed May 12, 2017
View reviewed changes
samples/client/petstore/go/go-petstore/enum_class.go Outdated

// List of EnumClasss
const (
_abc EnumClass = "_abc"
Copy link
Contributor

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"?

Copy link
Contributor

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.

Copy link
Contributor Author

@marcusljx marcusljx May 12, 2017 •
edited
Loading

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.

Copy link
Contributor Author

@marcusljx marcusljx May 12, 2017 •
edited
Loading

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?

Copy link
Contributor

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("_$", "");
...
    }

Copy link
Contributor Author

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.

Copy link
Contributor

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

antihax
antihax reviewed May 12, 2017
View reviewed changes
Copy link
Contributor

@antihax antihax left a 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.

samples/client/petstore/go/go-petstore/docs/FakeApi.md Outdated
@@ -68,7 +68,7 @@ Name | Type | Description | Notes
**float** | **float32**| None |
**string_** | **string**| None |
**binary** | **string**| None |
**date** | **string**| None |
Copy link
Contributor

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?

Copy link
Contributor

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?

Copy link
Contributor Author

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

@marcusljx
Copy link
Contributor Author

Okay I think I made a mistake with the forking+branching. I will close this and push another PR (properly this time I hope)

@marcusljx marcusljx closed this May 13, 2017
@marcusljx marcusljx mentioned this pull request May 13, 2017
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antihax antihax antihax left review comments

@wing328 wing328 wing328 left review comments

Assignees
No one assigned
Labels
Client: Go Enhancement: Feature Feature: Enum
Projects
None yet
Milestone
v2.3.0
Development

Successfully merging this pull request may close these issues.
3 participants
@marcusljx @wing328 @antihax