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

Conversation

WonderCsabo
Copy link
Member

Fixes #1394.

DayS and others added 2 commits November 9, 2014 16:43
[maven-release-plugin] copy for tag androidannotations-3.3
@WonderCsabo WonderCsabo force-pushed the 1394_preferenceChangeMoreParameterTypes branch 2 times, most recently from a0dfe62 to 13fdd1f Compare May 5, 2015 16:15
@@ -42,8 +42,13 @@
* <li>A {@link android.preference.Preference Preference} parameter to know
* which preference was targeted by this event</li>
* <li>An {@link Object} or {@link java.util.Set Set of strings} or
* {@link Boolean} or {@link String} to obtain the new value of the
* {@link android.preference.Preference Preference}</li>
* {@link Boolean} or {@link Float} or {@link Integer} or {@link Long} or
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure how it is in english, but in german we would comma seperate the words but the last. also personally i would change the order a little bit

An Object, String, Set of Strings and also Boolean, Float, Integer, Long or thier corresponding primitive types

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for separating with commas and for the order.

@WonderCsabo WonderCsabo force-pushed the 1394_preferenceChangeMoreParameterTypes branch from 13fdd1f to bc688e0 Compare May 6, 2015 07:53
@WonderCsabo
Copy link
Member Author

Thanks for the suggestion on the JavaDoc, updated.

@WonderCsabo
Copy link
Member Author

I just realized i used the new parameter validator API, but it is not available in the master branch we are merging this back. I will change this to only be on top of master and not develop.

@WonderCsabo WonderCsabo force-pushed the 1394_preferenceChangeMoreParameterTypes branch from bc688e0 to 4992869 Compare May 6, 2015 08:25
@WonderCsabo
Copy link
Member Author

Now this branch is based on master.

@WonderCsabo
Copy link
Member Author

Correcting myself: of course we have to merge back to a hotfix branch, not directly to master.

} else if (type.equals(CanonicalNameConstants.INTEGER) || type.equals(int.class.getName()) || //
type.equals(CanonicalNameConstants.FLOAT) || type.equals(float.class.getName()) || //
type.equals(CanonicalNameConstants.LONG) || type.equals(long.class.getName())) {
JClass wrapperClass = type.startsWith("java") ? holder.refClass(type) : JType.parse(holder.codeModel(), type.replace(".class", "")).boxify();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is quite hard to understand. Is there another way to get the wrapper class ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that there are two cases:
type is wrapper
type is primitive

In the first case, we can just the holder.refClass. Unfortunetaly in the second case, holder.refClass returns a low letter int class for example. JType.parse is the way to get correct primitive types, but it only expects the name, without the .class. There is no way to get "int" name from "TypeMirror", so we have to remove ".class" manually.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK then.

@yDelouis
Copy link
Contributor

yDelouis commented May 8, 2015

I have only one comment. Otherwise, the code is OK.

WonderCsabo added a commit that referenced this pull request May 9, 2015
@PreferenceChange more parameter types
@WonderCsabo WonderCsabo closed this May 9, 2015
@WonderCsabo WonderCsabo deleted the 1394_preferenceChangeMoreParameterTypes branch May 9, 2015 10:52
@WonderCsabo WonderCsabo added this to the 3.3.1 milestone May 9, 2015
@WonderCsabo
Copy link
Member Author

Rebased and merged as of 8d63943.

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

Successfully merging this pull request may close these issues.

@PreferenceChange minor parameter related changes
4 participants