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

Use section name as namespace #273

Closed
wants to merge 2 commits into from
Closed

Use section name as namespace #273

wants to merge 2 commits into from

Conversation

archonitex
Copy link

Quick PR for Feature Request #272 to use section as namespace for the keys

@scelis
Copy link
Owner

scelis commented Apr 9, 2019

Thanks for this! I am trying to figure out if this is something I want to add to Twine as we try not to add features that will only be used by a very small percentage of users. If we do add it, I think we will want unit tests for sure to help ensure the feature doesn't break in the future. I'm happy to leave this open for a bit to see if anyone else wants to make a case for adding it.

@scelis
Copy link
Owner

scelis commented Apr 9, 2019

One other concern: We have to be cognizant of special characters for the various formatters. I think Android doesn't like having periods in string keys and that it might break R.string functionality. Does anyone know if this is true?

@archonitex
Copy link
Author

@scelis That's fair! I personally feel as more and more people are moving towards using code generation tools like Swiftgen this would become more popular.

I don't know what long term plans you have for the library but maybe we can rethink this feature and think up a more "plugin" style where you could add "hooks" to process and massage data as it is being processed by Twine. That way people could build their own "Twine Plugins" to modify the keys or values before they are exported. Food for thought!

If you're interested in having the feature merge into the project I would gladly step up and write the unit test suite for it and maybe make it a bit more robust.

Let me know!

@jonasrottmann
Copy link
Contributor

jonasrottmann commented Apr 19, 2019

One other concern: We have to be cognizant of special characters for the various formatters. I think Android doesn't like having periods in string keys and that it might break R.string functionality. Does anyone know if this is true?

Yes, completely right 👍🏻
Android resource identifier names have to obey the rules of identifiers in Java.
Only [A-Z], [a-z], [0-9], $ and _ are allowed and first character must not be a digit.
See 3.8. Identifiers for more info.

@archonitex
Copy link
Author

@scelis Would this feature be a better fit as a plugin / custom formatter that I can add and distribute separately? Does Twine have the ability for plugins / custom formatters?

@scelis
Copy link
Owner

scelis commented May 14, 2019

Hey! Sorry that this is stagnating and thanks again for your patience and your contributions. I think there are a few possible paths from here:

  1. Update the current PR and change the separator character to one that is more universally accepted for string identifiers. I'm still not 100% convinced that this is a feature that will have enough usage to warrant being in Twine, but it's also simple enough that I'm ok adding it.

  2. Add support for this type of functionality using a plugin. There is some very basic plugin support in Twine (see this documentation and plugin.rb). Note, however, that this should probably not be implemented as a custom formatter. Instead, I think we would want to add plugin support for custom transformation of both string keys and string values as they are being written to localization files. Support for this does not currently exist in Twine and would need to be implemented.

  3. Do this kind of thing as a pre-processor step in whatever you are using for your build or deploy. One of the goals of Twine is for the Twine text file to be super easy to read and write, even for scripts outside of the Twine ecosystem. In this particular usecase, your build process could read the Twine file out of your repository and re-write the contents to prepend the section name to every string key. Then, your script that executes twine would use this new, processed file instead of the original one. This is the sort of way I anticipate many one-off features being implemented until there are enough people wanting the feature to warrant adding it to Twine proper.

@scelis
Copy link
Owner

scelis commented Jul 20, 2020

Closing due to inactivity. Please feel free to reopen!

@scelis scelis closed this Jul 20, 2020
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 this pull request may close these issues.

3 participants