-
Notifications
You must be signed in to change notification settings - Fork 214
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
Fixes Issue About Generated Strings Rendering #1307
Conversation
@@ -97,8 +99,10 @@ public class PaparazziPlugin : Plugin<Project> { | |||
val reportOutputDir = project.extensions.getByType(ReportingExtension::class.java).baseDirectory.dir("paparazzi/${variant.name}") | |||
val snapshotOutputDir = project.layout.projectDirectory.dir("src/test/snapshots") | |||
|
|||
val generateResValuesProvider = project.tasks.named("generate${variantSlug}ResValues") as TaskProvider<GenerateResValues> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any better API to retrieve the generated res folder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! I'd cherry pick this small piece of logic here: https://github.com/cashapp/paparazzi/pull/1305/files#diff-197fe329c4084290675667dd6b6194a6db2af00d5410525ad2d64bf4591d9b82
...with one small tweak: move that logic into the provider in order to defer "getting" the Provider contents until task execution.
in other words,
task.projectResourceDirs.set(project.provider {
val packagedResDir = InternalArtifactType.PACKAGED_RES
.getIntermediateOutputPath(buildDirectory, variant.name)
localResourceDirs.relativize(projectDirectory).plus(projectDirectory.relativize(packagedResDir))
})
This will change yet again once #1136 lands, but I like this as a temp solution to close #1067 and release sooner as part of 1.3.3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems InternalArtifactType.GENERATED_RES.[getOutputPath|getIntermediateOutputPath]
does not give the right path
Expected: build/generated/res/resValues/debug
Actual: build/[generated|intermediates]/generated_res/debug
Looking into getOutputPath or getIntermediateOutputPath, what it does inside is just to join paths
FileUtils.join(
getOutputDir(buildDirectory.get().asFile),
variantIdentifier,
*paths,
forceFilename.ifEmpty { getFileSystemLocationName() }
)
So it is static and different from actual generated res path. And generated res path could be set via registerGeneratedResFolders
, so we should always read it from GenerateResValues
task rather than constructing the static path programatically
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh you're right. I was confusing the two output directory types, good catch!
We still should try and avoid task references where possible, since that peeks into plugin implementation detail, but not a blocker for this. Moving to AGP 8 apis, will change this logic to automatically be included as part of localResourceDirs.
@@ -97,8 +99,10 @@ public class PaparazziPlugin : Plugin<Project> { | |||
val reportOutputDir = project.extensions.getByType(ReportingExtension::class.java).baseDirectory.dir("paparazzi/${variant.name}") | |||
val snapshotOutputDir = project.layout.projectDirectory.dir("src/test/snapshots") | |||
|
|||
val generateResValuesProvider = project.tasks.named("generate${variantSlug}ResValues") as TaskProvider<GenerateResValues> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! I'd cherry pick this small piece of logic here: https://github.com/cashapp/paparazzi/pull/1305/files#diff-197fe329c4084290675667dd6b6194a6db2af00d5410525ad2d64bf4591d9b82
...with one small tweak: move that logic into the provider in order to defer "getting" the Provider contents until task execution.
in other words,
task.projectResourceDirs.set(project.provider {
val packagedResDir = InternalArtifactType.PACKAGED_RES
.getIntermediateOutputPath(buildDirectory, variant.name)
localResourceDirs.relativize(projectDirectory).plus(projectDirectory.relativize(packagedResDir))
})
This will change yet again once #1136 lands, but I like this as a temp solution to close #1067 and release sooner as part of 1.3.3.
sample/src/main/java/app/cash/paparazzi/sample/ResourcesDemo.kt
Outdated
Show resolved
Hide resolved
sample/src/main/java/app/cash/paparazzi/sample/ResourcesDemoView.kt
Outdated
Show resolved
Hide resolved
bc727b1
to
fa7fe38
Compare
…resource loading mechanism
92deaaf
to
56cd985
Compare
@@ -97,8 +99,10 @@ public class PaparazziPlugin : Plugin<Project> { | |||
val reportOutputDir = project.extensions.getByType(ReportingExtension::class.java).baseDirectory.dir("paparazzi/${variant.name}") | |||
val snapshotOutputDir = project.layout.projectDirectory.dir("src/test/snapshots") | |||
|
|||
val generateResValuesProvider = project.tasks.named("generate${variantSlug}ResValues") as TaskProvider<GenerateResValues> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh you're right. I was confusing the two output directory types, good catch!
We still should try and avoid task references where possible, since that peeks into plugin implementation detail, but not a blocker for this. Moving to AGP 8 apis, will change this logic to automatically be included as part of localResourceDirs.
Co-authored-by: John Rodriguez <john.rodriguez@gmail.com>
Related to #1067
Fixes issue that generated strings are not rendered correctly in new resource loading mechanism