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

Look for default layout in parents if not specified #119

Merged
merged 2 commits into from
Feb 11, 2017
Merged

Look for default layout in parents if not specified #119

merged 2 commits into from
Feb 11, 2017

Conversation

geralt-encore
Copy link
Contributor

@geralt-encore geralt-encore commented Feb 9, 2017

#118
The logic ended up more complex than I expected initially. If you have any ideas how to do it easily - I'll gladly fix it.

@elihart
Copy link
Contributor

elihart commented Feb 9, 2017

Nice! It is a bit complex, but I don't think there is much getting around that. I just merged #120 which adds checks around the annotation.layout() calls. To prevent having to do that mess I refactored what you have to only check the layout in one place. I'm also a fan of recursion :P

private EpoxyModelClass findClassAnnotationWithLayout(TypeElement classElement)
      throws EpoxyProcessorException {
    if (!isEpoxyModel(classElement)) {
      return null;
    }

    EpoxyModelClass annotation = classElement.getAnnotation(EpoxyModelClass.class);
    if (annotation == null) {
      return null;
    }

    try {
      int layoutRes = annotation.layout();
      if (layoutRes != 0) {
        return annotation;
      }
    } catch (AnnotationTypeMismatchException e) {
      throwError("Invalid layout value in %s annotation. (class: %s). %s: %s",
          EpoxyModelClass.class,
          classElement.getSimpleName(),
          e.getClass().getSimpleName(),
          e.getMessage());
      return null;
    }

    TypeElement superclassElement = getSuperClassElement(typeUtils, classElement);
    EpoxyModelClass annotationOnSuperClass = findClassAnnotationWithLayout(superclassElement);

    // Return the last annotation value we have so the proper error can be thrown if needed
    return annotationOnSuperClass != null ? annotationOnSuperClass : annotation;
  }

@geralt-encore
Copy link
Contributor Author

I'll look into your implementation! Just wanted to be sure that I understood the problem and did it correctly.

@Test
public void testGenerateDefaultLayoutMethodFailsIfLayoutNotSpecified() {
JavaFileObject model = JavaFileObjects
.forResource("GenerateDefaultLayoutMethodNoLayout.java");
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for all these tests!

@elihart
Copy link
Contributor

elihart commented Feb 9, 2017

yep, you understood perfectly, thanks for working on this!

Copy link
Contributor

@elihart elihart left a comment

Choose a reason for hiding this comment

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

Perfect! Thanks :)

@elihart elihart merged commit fc816b2 into airbnb:master Feb 11, 2017
@geralt-encore geralt-encore deleted the parent-default-layout branch February 18, 2017 16:12
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.

2 participants