Skip to content
This repository was archived by the owner on Feb 26, 2023. It is now read-only.

Support R2 class for resource values #2022

Merged
dodgex merged 1 commit into from
Jul 7, 2017
Merged

Support R2 class for resource values #2022

dodgex merged 1 commit into from
Jul 7, 2017

Conversation

@WonderCsabo
Copy link
Member

@WonderCsabo WonderCsabo commented Jul 6, 2017 •
edited
Loading

This PR adds support to ButterKnife R2 generator plugin. Please note only the SNAPSHOT version of that plugin generates all types of resources. For rationale, see the commit message.

@WonderCsabo WonderCsabo requested a review from dodgex July 6, 2017 12:39
@dodgex
Copy link
Member

dodgex commented Jul 6, 2017

Hey Csaba, thank you for the PR. I got 2 questions.

  1. What is the purpose of the R2 class? what is the advantage over R?
  2. (maybe already part of the answer to 1 but:) do we always want to use R2 if it is present?

@WonderCsabo
Copy link
Member Author

WonderCsabo commented Jul 6, 2017 •
edited
Loading

Hey,

I tried to explain it in the commit message, but maybe it was not clear, sorry.

  1. The problem is with R class is that in case of library projects, the R.. fields are not final, so we cannot use them in annotation parameters. For the same reason, we also cannot read them for validation. To work around this, we added the resName parameter everywhere, which accepts a String. But that is inconvenient to use, you can have mistakes, there is no code completion etc. The R2 class has final fields, so it can be used in annotation parameters. But to actually make it work, we also have to read the values for validation, and we can only do that from the final R2 fields. This is what this PR does.

  2. Yes, maybe we should only the this if enabled by some option. I can put this behind an option flag.

@dodgex
Copy link
Member

dodgex commented Jul 7, 2017

Thank you for explaining it again. I did not look at the commit message, so i missed the explaination there.

But I'm a bit confused... We already have @ViewById(R.id.myTextView) so I'm not sure what the problem is with R and annotation parameters...

Okay, ignore that... reading your explaination a second time revels in case of library projects.

Maybe it is enough to put this behind the library option?

@WonderCsabo
Copy link
Member Author

WonderCsabo commented Jul 7, 2017

@dodgex i think we should put this behind a new flag.

@WonderCsabo
Copy link
Member Author

WonderCsabo commented Jul 7, 2017

@dodgex updated.

@WonderCsabo
Support R2 class for resource values
75dcb3d 
The R class contains non-constant fields in case of library projects,
so we cannot use them in annotation values, nor for validaiton, since
the values are not available at compile time. To work around this,
there is a Gradle plugin to generate an R2 class, which contains
constants fields.

This commit adds a very simple integration to this plugin, by using the
R2 class instead of R if the former is available.
@dodgex
Copy link
Member

dodgex commented Jul 7, 2017

looks fine.

@dodgex dodgex merged commit f6867f8 into androidannotations:develop Jul 7, 2017
@WonderCsabo WonderCsabo deleted the supportR2Class branch July 7, 2017 12:38
WonderCsabo added a commit to WonderCsabo/androidannotations that referenced this pull request Jul 7, 2017
@WonderCsabo
Use normal R class even when validating with R2
87491b6 
The final resource values will not be the same as the compile-time values,
so we have to use the R class in the generated code, even if the user set
to use the R2 class in annotation parameters.

Follow-up fix for:
androidannotations#2022
@WonderCsabo WonderCsabo mentioned this pull request Jul 7, 2017
Merged
WonderCsabo added a commit to WonderCsabo/androidannotations that referenced this pull request Jul 7, 2017
@WonderCsabo
Use normal R class even when validating with R2
0fc78f9 
The final resource values will not be the same as the compile-time values,
so we have to use the R class in the generated code, even if the user set
to use the R2 class in annotation parameters.

Follow-up fix for:
androidannotations#2022
@WonderCsabo
Copy link
Member Author

WonderCsabo commented Jul 7, 2017

@dodgex documentation in my fork:
https://github.com/WonderCsabo/androidannotations/wiki/Library-projects#using-r2-class
https://github.com/WonderCsabo/androidannotations/wiki/CustomizeAnnotationProcessing#user2

@dodgex
Copy link
Member

dodgex commented Jul 7, 2017

looks good

@WonderCsabo WonderCsabo added this to the 4.4 milestone Jul 7, 2017
@WonderCsabo
Copy link
Member Author

WonderCsabo commented Jul 7, 2017

I merged the doc.

@LuigiPapino
Copy link

LuigiPapino commented Aug 8, 2017

This just saved me a lot of time. And will be very useful for instant apps.
It's working fine for me on a quiet complex project, with three layers (installedapp-> base -> sharedui).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@dodgex dodgex dodgex approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

3 participants

@WonderCsabo @dodgex @LuigiPapino