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

Add name parameter to @SharedPref #1918

Merged
WonderCsabo merged 2 commits into from
Dec 11, 2016
Merged

Add name parameter to @SharedPref #1918

WonderCsabo merged 2 commits into from
Dec 11, 2016

Conversation

@dodgex
Copy link
Member

@dodgex dodgex commented Dec 11, 2016

This PR implements the feature request of #1675.

With this PR it is possible to provide a custom name for the preference file of a @SharedPref interface.

@dodgex dodgex requested a review from WonderCsabo December 11, 2016 10:05
@WonderCsabo
Copy link
Member

WonderCsabo commented Dec 11, 2016

@dodgex please change the PR title, it is pretty ugly how it was generated from the branch name.

WonderCsabo
WonderCsabo suggested changes Dec 11, 2016
View reviewed changes
Copy link
Member

@WonderCsabo WonderCsabo left a comment

Choose a reason for hiding this comment

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

LGTM!

...nnotations/src/main/java/org/androidannotations/internal/core/handler/SharedPrefHandler.java Outdated
boolean hasCustomName = !name.trim().isEmpty();
EnumSet<SharedPref.Scope> allowedScopes = EnumSet.of(SharedPref.Scope.ACTIVITY, SharedPref.Scope.UNIQUE);
if (hasCustomName && !allowedScopes.contains(scope)) {
validation.addWarning("SharedPref#name() is only supported for Scope.ACTIVITY and Scope.UNIQUE.");
Copy link
Member

@WonderCsabo WonderCsabo Dec 11, 2016

Choose a reason for hiding this comment

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

I would add an error() here. Better failing at compile time than spending hours debugging this at runtime.

...nnotations/src/main/java/org/androidannotations/internal/core/handler/SharedPrefHandler.java
SharedPref.Scope scope = sharedPrefAnnotation.value();
String name = sharedPrefAnnotation.name();
boolean hasCustomName = !name.trim().isEmpty();
EnumSet<SharedPref.Scope> allowedScopes = EnumSet.of(SharedPref.Scope.ACTIVITY, SharedPref.Scope.UNIQUE);
Copy link
Member

@WonderCsabo WonderCsabo Dec 11, 2016

Choose a reason for hiding this comment

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

Nice!

Copy link
Member Author

@dodgex dodgex Dec 11, 2016

Choose a reason for hiding this comment

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

:)

@dodgex
Copy link
Member Author

dodgex commented Dec 11, 2016 •
edited
Loading

@WonderCsabo I changed from warning to error and now i get a test failure due to a class that has nothing to do with the test?!

java.lang.AssertionError: Expected no errors, found ..\workspace\androidannotations\AndroidAnnotations\androidannotations-core\androidannotations\target\test-classes\org\androidannotations\generation\WindowFeatureAppCompatActivity.java:25: error: package android.support.v7.app does not exist
public class WindowFeatureAppCompatActivity extends android.support.v7.app.AppCompatActivity {

any idea? :(

@WonderCsabo WonderCsabo changed the title 1675 add name parameter to shared pref Add name parameter to @SharedPref Dec 11, 2016
@WonderCsabo
Copy link
Member

WonderCsabo commented Dec 11, 2016

@dodgex about the test failure: i am just guessing, but maybe ensureOutputDirectoryIsEmpty() deletes something which another test relies on?

@dodgex
Copy link
Member Author

dodgex commented Dec 11, 2016

Don't know... i copied that from GenerateJavaDocTest. The error appeas when i add the validation message as error and disappears when i add it as warning...

@dodgex dodgex added this to the 4.3 milestone Dec 11, 2016
@WonderCsabo WonderCsabo merged commit 7b54b2f into androidannotations:develop Dec 11, 2016
@dodgex dodgex deleted the 1675_add_name_parameter_to_shared_pref branch December 11, 2016 13:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@WonderCsabo WonderCsabo WonderCsabo approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

2 participants

@dodgex @WonderCsabo