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

Split filename-template for both entities and paths #60

Closed
Tracked by #101
LePips opened this issue Jul 26, 2022 · 4 comments · Fixed by #108
Closed
Tracked by #101

Split filename-template for both entities and paths #60

LePips opened this issue Jul 26, 2022 · 4 comments · Fixed by #108

Comments

@LePips
Copy link
Contributor

LePips commented Jul 26, 2022

Using the below files, everything is freely available:

Command used:

create-api generate --output . --config createapi-config.yaml --package JellyfinAPI -s jellyfin-openapi-stable.json"

With the operations paths style I get conflicting filenames between entities and paths. I actually need to use the operations paths style because rest results in another error because the spec has a path literally called "Path" and that creates a different issue. You can simply change the paths style and generate to see the error.

Anyways, the only way to properly avoid these file conflicts is to use --entityname-template "$0Model" which properly renames all entity files and the corresponding entities, however I would not like to have every entity to be that same name due to convenience. I think a flag change for the --filename-template <filename-template> would be more appropriate to split into two flags:

  • --entity-filename-template <entity-filename-template>
  • --paths-filename-template <paths-filename-template>

These flags would create a further separation between the two types and aid in these types of conflicts. If this gets greenlit I'll be more than happy to implement to get comfortable with this project.

@liamnichols
Copy link
Member

Thanks for raising this @LePips!

Anyways, the only way to properly avoid these file conflicts is to use --entityname-template "$0Model" which properly renames all entity files and the corresponding entities, however I would not like to have every entity to be that same name due to convenience

Yeah I agree. It's not common to suffix model files with FooModel.swift. It would make much more sense to add the suffix to the paths (i.e GetFooRequest.swift or similar).

I think a flag change for the --filename-template <filename-template> would be more appropriate to split into two flags

Generally this seems like a reasonable idea, but the filename template is also used on some smaller extension files that occasionally get generated too, so not everything falls into the 'paths' or 'entities' bucket.

In general, I did want to move the --filename-template (amongst other options) into the config file rather than command line arguments (#52) so I wonder if there would be a better way to model more specific options in the config file itself? But without thinking a bit more, I don't really have an answer to that yet.

As an alternative workaround, had you considered trying to rename the conflicting operations in the configuration file? Does that work how I'm thinking it would?

rename:
  operations:
    myConflictingOperationName: someOtherName

@LePips
Copy link
Contributor Author

LePips commented Jul 27, 2022

flag vs config

I'm all for having everything in the config. I'm also happy to aid in your work if you need any.

smaller extension files

We can just apply these names if they apply to an entity or path from the schema, not files with extensions. I will still have to take a deeper look at how file name generation is done.

rename workaround

I'm not knowledgeable of how that flag works specifically, but it does not fix the issue. Even then, it would be problematic to go through every conflict in a large schema like in my case. I'm way too lazy.

@LePips
Copy link
Contributor Author

LePips commented Aug 2, 2022

Finally looking at the file generation, might I propose a change to these extension files and general generated directory structure?

Both entities and paths sometimes create their necessary helper methods/namespaces/objects and they live within the Entities/Paths files respectively. I think it would be more organizational if they lived one level higher alongside the folders for better access and the folders are purely for schema generated files. These helper files could be EntityHelpers.swift/PathHelpers.swift but it may also be better to split among their purpose, like if all possible extensions were necessary:

// filename creation subject to change
Entities/
Paths/
Paths+Namespace.swift
Entities+AnyJSON

This would mean that the two config options (instead of flags) entityFilenameTemplate/pathsFilenameTemplate only affect schema generated files as the helper/extension files live one level higher.

As a reference, this is what the OpenAPI generator does.

@liamnichols
Copy link
Member

That sounds like a good approach 👍

it may also be better to split among their purpose

This is the best approach, it matches the direction we want to go with #74. But don't worry about merging them if --split wasn't used. So extensions will have the following rules:

  1. Always placed at the top level
  2. Always split
  3. Don't support filename templates
    • Lets just make sure that we're not using any common names like String+Utils.swift
    • Maybe we can prefix or suffix them consistently if necessary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants