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.

Sign up for GitHub

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

Jump to bottom

De-dupe with the fully qualified function name #315

Merged
merged 8 commits into from
May 8, 2023
Merged

De-dupe with the fully qualified function name #315

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
906844a
De-dupe with the fully qualified function name
vinaygaba Apr 3, 2022
63788e5
Fix indentation
pmecho Apr 5, 2023
13228f5
Fix detekt
pmecho Apr 5, 2023
f8c4359
Renames in tests
pmecho Apr 6, 2023
1c8d31a
Remove temp code
pmecho Apr 6, 2023
e2a3746
Revert "Renames in tests"
pmecho Apr 6, 2023
fcd9a66
Only use the element name for file prefix
pmecho Apr 6, 2023
9002840
Updating test code
pmecho Apr 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 10 additions & 8 deletions showkase-processor/src/main/java/com/airbnb/android/showkase/processor/ShowkaseProcessor.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,12 +275,12 @@ class ShowkaseProcessor @JvmOverloads constructor(
}

private fun Collection<ShowkaseMetadata.Component>.dedupeAndSort() = this.distinctBy {
// It's possible that a composable annotation is annotated with both Preview &
// It's possible that a composable annotation is annotated with both Preview &
// ShowkaseComposable(especially if we add more functionality to Showkase and they diverge
// in the customizations that they offer). In that scenario, its important to dedupe the
// composables as they will be processed across both the rounds. We first ensure that
// only distict method's are passed onto the next round. We do this by deduping on
// the combination of packageName, the wrapper class when available(otherwise it
// only distict method's are passed onto the next round. We do this by deduping on
// the combination of packageName, the wrapper class when available(otherwise it
// will be null) & the methodName.
if (it.componentIndex != null) {
"${it.packageName}_${it.enclosingClassName}_${it.elementName}_${it.componentIndex}"
Expand All @@ -290,16 +290,18 @@ class ShowkaseProcessor @JvmOverloads constructor(
}
}
.distinctBy {
// We also ensure that the component groupName and the component name are unique so
// that they don't show up twice in the browser app.
// We also ensure that the component groupName and the component name are unique so
// that they don't show up twice in the browser app. This also de-duplicates based
// on the fully qualified function name to support categorization with additional
// fields (e.g. tags, extraMetadata, etc) on custom browsers.
if (it.componentIndex != null) {
"${it.showkaseName}_${it.showkaseGroup}_${it.showkaseStyleName}_${it.componentIndex}"
"${it.fqPrefix}_${it.showkaseName}_${it.showkaseGroup}_${it.showkaseStyleName}_${it.componentIndex}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

we already do the ${it.packageName}_${it.enclosingClassName}_${it.elementName} dedpuing above. Why do we need to do it again here 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vinaygaba If I don't include it here as well it will remove ones that only differ by tags or extraMetadata. I tried to change this to do them in one distinctBy step but that messed up where a @Preview annotation has the same group/style as the @ShowkaseComposable annotation so I left it as two steps.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see!

} else {
"${it.showkaseName}_${it.showkaseGroup}_${it.showkaseStyleName}"
"${it.fqPrefix}_${it.showkaseName}_${it.showkaseGroup}_${it.showkaseStyleName}"
}
}
.sortedBy {
"${it.packageName}_${it.enclosingClassName}_${it.elementName}"
it.fqPrefix
}

private fun processColorAnnotation(roundEnvironment: XRoundEnv): Set<ShowkaseMetadata> {
Expand Down
4 changes: 4 additions & 0 deletions ...-processor/src/main/java/com/airbnb/android/showkase/processor/models/ShowkaseMetadata.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ internal sealed class ShowkaseMetadata {
abstract val insideWrapperClass: Boolean
abstract val insideObject: Boolean

/** A fully qualified prefix for use when de-duplicating components. **/
val fqPrefix: String
get() = "${packageName}_${enclosingClassName}_$elementName"

data class Component(
override val element: XElement,
override val packageName: String,
Expand Down
6 changes: 4 additions & 2 deletions ...c/main/java/com/airbnb/android/showkase/processor/writer/ShowkaseCodegenMetadataWriter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,11 @@ internal class ShowkaseCodegenMetadataWriter(private val environment: XProcessin
&& showkaseMetadata.componentIndex != null
&& showkaseMetadata.componentIndex > 0
) {
"${showkaseMetadata.showkaseGroup}_${showkaseMetadata.showkaseName}_${showkaseMetadata.componentIndex}"
"${showkaseMetadata.fqPrefix}_${showkaseMetadata.showkaseGroup}" +
"_${showkaseMetadata.showkaseName}_${showkaseMetadata.componentIndex}"
} else {
"${showkaseMetadata.showkaseGroup}_${showkaseMetadata.showkaseName}"
"${showkaseMetadata.fqPrefix}_${showkaseMetadata.showkaseGroup}" +
"_${showkaseMetadata.showkaseName}"
}
val methodName = if (showkaseMetadata is ShowkaseMetadata.Component
&& showkaseMetadata.showkaseStyleName != null
Expand Down
8 changes: 4 additions & 4 deletions showkase-processor/src/main/java/com/airbnb/android/showkase/processor/writer/WriterUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ internal fun CodeBlock.Builder.addShowkaseBrowserComponent(
} else {
"_${showkaseMetadata.showkaseName}"
}
var componentKey = (showkaseMetadata.packageName +
var componentKey = (showkaseMetadata.fqPrefix +
"_${showkaseMetadata.enclosingClassName}" +
"_${showkaseMetadata.showkaseGroup}" +
componentName +
Expand Down Expand Up @@ -323,10 +323,10 @@ internal fun generatePropertyNameFromMetadata(
val name =
if (metadata.componentIndex != null && metadata.componentIndex > 0
) {
"${metadata.packageName}_${metadata.showkaseGroup}_" +
"${metadata.fqPrefix}_${metadata.showkaseGroup}_" +
"${metadata.showkaseName}_${metadata.componentIndex}"
} else {
"${metadata.packageName}_${metadata.showkaseGroup}_${metadata.showkaseName}"
"${metadata.fqPrefix}_${metadata.showkaseGroup}_${metadata.showkaseName}"
}
val propertyName = if (metadata.showkaseStyleName != null) {
"${name}_${metadata.showkaseStyleName}"
Expand All @@ -336,7 +336,7 @@ internal fun generatePropertyNameFromMetadata(
propertyName
}
else -> {
"${metadata.packageName}_${metadata.showkaseGroup}_${metadata.showkaseName}"
"${metadata.fqPrefix}_${metadata.showkaseGroup}_${metadata.showkaseName}"
.filter { it.isLetterOrDigit() }
}
}
Expand Down