johncarl81 / transfuse

:syringe: Transfuse - A Dependency Injection and Integration framework for Google Android
http://androidtransfuse.org/
Apache License 2.0
220 stars 28 forks source link

It'd be nice to be able to use the R.java constants for the Activity name and icon. #146

Closed tylerchesley closed 8 years ago

tylerchesley commented 9 years ago

I understand why strings are used now because it makes it easier to generate the manifest. But it would be easier to refactor if we could use the R constants.

johncarl81 commented 9 years ago

I like the way you think. As you may know, currently we can use the app constants, just in string form:

@Activity(label = "@string/app_name", icon = "@drawable/icon")

This would be cool:

@Activity(labelId = R.string.app_name, iconId = R.drawable.icon)

To get there we would need some sort of lookup from the R values (ints) to the field name. This could be challenging. Also, we would need to deal with the ambiguous label, icon, etc definitions. A solution here might be to only allow R constants, but I fear this is a bit heavy handed.

Thoughts?

tylerchesley commented 9 years ago

I don't know if you can use this at annotation processing time but there is a helper method on the Resources class that will give you the string's entry name. I dug into the source code to see how they're doing it but it looks like they're delegating to some native code.

https://developer.android.com/reference/android/content/res/Resources.html#getResourceEntryName(int)

My other, possibly naive, idea is to just take the constant name after the R.string portion as the field name, e.g. R.string.app_name -> "@string/app_name". As far as I know, that constant name will always match the entry name even if refactored.

johncarl81 commented 9 years ago

I actually spoke too soon. I forgot I implemented a lookup to find the R.* invocations by id in the RResourceMapping.java class. An instance of this class is populated by Transfuse for each round that the annotation processor runs.

We would just have to create a String literal instead of a JInvocation.

Also, I have a branch of Transfuse that's near completion locally that will allow anyone to extend Transfuse with plugins. This is right up that alley as we could write the manifest entry using a different annotation if appropriate.

tylerchesley commented 9 years ago

Let me know. I'm more than happy to contribute when you're ready. I have a few other ideas I'd like to implement too.

johncarl81 commented 9 years ago

@tylerchesley, you know the concerns around moving off of String based manifest fields (where applicable), but if you want to move forward with a PR, go for it. There may be something to enforcing that the user puts resources into the proper place.

If you have other ideas I'm excited to hear about and discuss them as well.

I encourage you to dig into the code. We have a number of open issues, that may be challenging but I would certainly appreciate the help.