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

Impossible to create two same colors #31

Closed
Igor-Palaguta opened this issue Oct 13, 2015 · 8 comments
Closed

Impossible to create two same colors #31

Igor-Palaguta opened this issue Oct 13, 2015 · 8 comments

Comments

@Igor-Palaguta
Copy link
Contributor

In colors file I have two identical colors with different name

LightGrayColor: #a8abb1
HighlightedLinkColor: #a8abb1

swiftgen generates two cases with same value, and swift compiler fails with error "Raw value for enum is not unique"

@Igor-Palaguta
Copy link
Contributor Author

If we remove inheritance from UInt32, and we create something like this:

enum Name {
   var colorValue: UInt32 {
      switch self {
      case LightGrayColor:
          return 0xa8abb1ff
      case HighlightedLinkColor:
          return 0xa8abb1ff
      }
   }
}

If you fine with it, I can create PR

@AliSoftware
Copy link
Collaborator

Interesting, I didn't think about such a case.

Don't rush on doing the PR though: I'm currently working on a big PR #30 to refactor SwiftGen using Commander, and my current refactor there would make it hard to merge your PR afterwards.
Additionally that PR would allow us to easily add options to the CLI, like one that could let users decide if they allow multiple colors or not for example, using some flag like --allow-duplicates.

Another change I wanted to do afterwards would be to use Stencil (#24) so that people can use custom templates for the generated code. That change will make it way easier to change the generated code, as it will be stored as a clear and readable Stencil template, instead of mixed and hard-coded within the code.

Once those two big PRs are done, we could then imagine keeping the current template with enum Color : UInt32 for simplicity and readability's sake, and another your template that allows duplicate values, for people like you who need those.

As a side note, I think using a dictionary [Color: UInt32] for the var colorValue: UInt32 computed variable would be more efficient and probably faster than doing a big switch there, as each case will only return an UInt32 and do nothing else and dictionaries are optimized for fast lookup.

@AliSoftware
Copy link
Collaborator

@Igor-Palaguta Good news: I just merged the Stencil branch (#24) which was a major refactor.

As a result, the masterbranch of SwiftGen now uses templates, which means you could easily make a PR to fix this now! 😉

@Igor-Palaguta
Copy link
Contributor Author

Very Cool! Thank you

Don't know where is good place for custom templates, I created for my project:

  1. https://gist.github.com/Igor-Palaguta/65c20ca626dd41119e0f#file-strings-stencil
    Preserves undescores in identifiers using swift_Identifier
  2. https://gist.github.com/Igor-Palaguta/23a7b9a540a6933d1d60#file-colors-stencil
    Supports colors with same code

@AliSoftware
Copy link
Collaborator

I think we could at least add these templates with the other ones, so that they'd be installed with homebrew and all.

Then we could imagine an option flag from the CLI to make it easy to manage and reference them easily instead of having to specify the whole path.

First step

  • Provide a swiftgen xxx --templateName templateName or swiftgen xxx -n templateName option to allow the user to specify a template by name. This would be equivalent to specify --template with a full path, but will automatically prepend the name with TEMPLATES_RELATIVE_PATH + append the name with .stencil if not already. That way, it would be easier to use an alternate template that would be part of the ones provided with SwiftGen

Next step

This move the concept further (and could be done in a separated PR later)

  • Add a CUSTOM_TEMPLATES_PATH constant that will point to ~/Library/Application Support/SwiftGen/templates
  • Provide a swiftgen templates --list command to list all the templates installed in the default location (the ones in TEMPLATES_RELATIVE_PATH) and the ones installed in CUSTOM_TEMPLATES_PATH if any
  • Provide a swiftgen templates --add FILE command to copy a template into CUSTOM_TEMPLATES_PATH.
  • Make swiftgen templates with no option simply print the CUSTOM_TEMPLATES_PATH path to stdout.
  • Make swiftgen xxx -n templateName search the named template in CUSTOM_TEMPLATES_PATH first before fallback into TEMPLATES_RELATIVE_PATH

This way people could install custom templates and use them way more easily!

@AliSoftware
Copy link
Collaborator

  1. https://gist.github.com/Igor-Palaguta/65c20ca626dd41119e0f#file-strings-stencil
    Preserves undescores in identifiers using swift_Identifier

I think we should actually make that the default template. We should also maybe rename that filter swift_identifier, all lowercase, btw (because when underscores are not considered invalid chars and are preserved, we don't uppercase the next letter if it's lowercase)

@AliSoftware
Copy link
Collaborator

Ok, i started a branch snakeCase that would allow to convert this_kind_of_string to thisKindOfString so that the difference between swiftIdentifier and swift_identifier won't probably be useful anymore.

One can then either just use |swiftIdentifier (for your style of coding, keeping MENU_HOME_TITLE style) or |swiftIdentifier|snakeToCamelCase (for people prefering menuHomeTitle). I think it would be more understandable than the distinction between swiftIdentifier vs. swift_identifier.

Then we could have two templates for each of the 4 tools, depending on each people's style of coding.

@AliSoftware
Copy link
Collaborator

As of now in master, the only filter for generating a Swift identifier is now called swiftIdentifier (no more swift_identifier), and I added a separate filter snakeToCamelCase to convert this_is_an_identifier to ThisIsAnIdentifier.

The default template uses snakeToCamelCase for various stuff (but not all), so you could duplicate it into a custom template for your use case which would simply not use that snakeToCamelCase filter.

I'm keeping this issue open until #42 is done (at least the first part allowing people to specify a template by name) and you could then integrate your templates so they get installed with SwiftGen by default.

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

No branches or pull requests

2 participants