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.

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

[C]: Fix enum values #9537

Merged
merged 2 commits into from
May 27, 2021
Merged

[C]: Fix enum values #9537

merged 2 commits into from
May 27, 2021

Conversation

zhemant
Copy link
Contributor

@zhemant zhemant commented May 21, 2021

enumvalue is being modified to replace all "-" with "_". This is changing the original enum values.

E.g. If an enum is defined as follows:
enum:
- a-test
- b-rest
- c-best

The values are changed to a_test, b_rest, c_best. This should not happen as it results in issues if the same spec is referenced by different people with different code-gen.
The enumvalue is still required to replace all "-" with "_" as this is used in typedef.

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh
    ./bin/utils/export_docs_generators.sh
    
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master, 5.1.x, 6.0.x
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@ityuhui @michelealbano @wing328

@@ -8,13 +8,13 @@

{{#isEnum}}
char* {{classFilename}}_{{classname}}_ToString({{projectName}}_{{classVarName}}_{{enumName}}_e {{classname}}) {
char *{{classname}}Array[] = { "NULL"{{#allowableValues}}{{#enumVars}}, "{{{value}}}"{{/enumVars}}{{/allowableValues}} };
char *{{classname}}Array[] = { "NULL"{{#allowableValues}}, {{#values}}"{{.}}"{{^-last}},{{/-last}}{{/values}}{{/allowableValues}} };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is below line OK ?

char *{{classname}}Array[] = { "NULL"{{#allowableValues}}{{#values}}, "{{.}}"{{/values}}{{/allowableValues}} };

Sorry I don't have a Swagger doc with enum values, so I cannot try it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line you suggest is not correct. It does not add comma in between values.

With the current line last we get comma in between:

{ "NULL"{{#allowableValues}}, {{#values}}"{{.}}"{{^-last}},{{/-last}}{{/values}}{{/allowableValues}} };
 char* statusArray[] =  { "NULL", "available-t1","pending-t2","sold-t3" };

If we remove the ^last tag , then it does not add a comma in between values.

{ "NULL"{{#allowableValues}}{{#values}}, "{{.}}"{{/values}}{{/allowableValues}} };
char* statusArray[] =  { "NULL", "available-t1""pending-t2""sold-t3" };

Snippet from debug logs how it looks:

      "allowableValues" : {
        "enumVars" : [ {
          "name" : "AVAILABLET1",
          "isString" : false,
          "value" : "available_t1"
        }, {
          "name" : "PENDINGT2",
          "isString" : false,
          "value" : "pending_t2"
        }, {
          "name" : "SOLDT3",
          "isString" : false,
          "value" : "sold_t3"
        } ],
        "values" : [ "available-t1", "pending-t2", "sold-t3" ]
      },

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw you can use this doc for trying as well: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore.yaml

You will have to update enum on line 719 to following:

enum:            
  - available-t1            
  - pending-t2            
  - sold-t3

You can check pet.c for generated enum's.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @zhemant

I tried

char *{{classname}}Array[] = { "NULL"{{#allowableValues}}{{#values}}, "{{.}}"{{/values}}{{/allowableValues}} };

with https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore.yaml (adding "-" to enum values)

And got the right generated code:

char *statusArray[] =  { "NULL", "available-t1", "pending-t2", "sold-t3" };

So I think my proposal is OK too.

BTW: do you want to change

modules/openapi-generator/src/main/resources/C-libcurl/api-body.mustache

too ? It has the same code for enum type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

weird, I will try again to generate with your suggestion.

Yes, I will update the enum in api-body as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ityuhui I apologise for before, I tried with wrong syntax. I tried { "NULL"{{#allowableValues}},{{#values}}"{{.}}"{{/values}}{{/allowableValues}} }; instead of { "NULL"{{#allowableValues}}{{#values}}, "{{.}}"{{/values}}{{/allowableValues}} };
I am also able to generate same result as you now.
I have updated it now in body and API mustache

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!
and could you please tell me how to get the debug log ? e.g.

      "allowableValues" : {
        "enumVars" : [ {
          "name" : "AVAILABLET1",
          "isString" : false,
          "value" : "available_t1"
        }, {
          "name" : "PENDINGT2",
          "isString" : false,
          "value" : "pending_t2"
        }, {
          "name" : "SOLDT3",
          "isString" : false,
          "value" : "sold_t3"
        } ],
        "values" : [ "available-t1", "pending-t2", "sold-t3" ]
      },

It's very helpful for template code.

Copy link
Contributor Author

@zhemant zhemant May 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I use the following command to generate debug logs for Models and push them into a file for analysis:

java -DdebugModels -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -i modules/openapi-generator/src/test/resources/3_0/petstore.yaml -g c -o out/pet-new --enable-post-process-file  > out/pet.txt

You can replace -DdebugModels with -DdebugOperations and other flags.

-DdebugModels is for printing template logs for all models
-DdebugOperations is for printing template logs for all API's

You can find more debug flags here: https://github.com/openapitools/openapi-generator/wiki/FAQ#how-to-debug-openapi-generator.

I hope this is what you were looking for.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is exactly what I want. Thank you very much !

Copy link
Contributor

@ityuhui ityuhui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wing328 wing328 added this to the 5.2.0 milestone May 27, 2021
@wing328 wing328 merged commit 5227e06 into OpenAPITools:master May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants