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

Update HTML report path to be clickable #471

Merged
merged 2 commits into from
Oct 10, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ package kotlinx.kover.gradle.plugin.tasks.reports
import org.gradle.api.file.*
import org.gradle.api.provider.Property
import org.gradle.api.tasks.*
import java.io.File
import java.net.URI

@CacheableTask
internal abstract class KoverHtmlTask : AbstractKoverReportTask() {
Expand All @@ -28,7 +30,14 @@ internal abstract class KoverHtmlTask : AbstractKoverReportTask() {
}

fun printPath(): Boolean {
logger.lifecycle("Kover: HTML report for '$projectPath' file://${reportDir.get().asFile.canonicalPath}/index.html")
val clickablePath = URI(
Copy link
Collaborator

@shanshin shanshin Oct 2, 2023

Choose a reason for hiding this comment

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

It looks like such code gives the same result:

        val clickablePath = URI(
            "file",
            "",
            reportDir.get().asFile.resolve("index.html").canonicalPath,
            null
        ).toASCIIString()

Copy link
Collaborator

@shanshin shanshin Oct 2, 2023

Choose a reason for hiding this comment

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

Perhaps it's even better to try like this:

        val clickablePath = URI(
            "file",
            "",
            reportDir.get().file("index.html").asFile.toURI().path,
            null
        ).toASCIIString()

Copy link
Contributor Author

@Ezard Ezard Oct 2, 2023

Choose a reason for hiding this comment

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

@shanshin I agree with the first one - the second one crashes for me, because the path ends up being something like C:/path/to/file.html, but URI requires a leading slash - canonicalPath is the bit that adds this slash in

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, the first one appears to crash for me too 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reportDir.get().asFile.resolve("index.html").toURI().path

seems to work, as an alternative to

File(reportDir.get().asFile.canonicalPath, "index.html").toURI().path

Copy link
Collaborator

Choose a reason for hiding this comment

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

reportDir.get().asFile.resolve("index.html").toURI().path
Ok, this is also a good variant if it works stably on Windows

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Found some issues with that version on Windows :(

My recommendation would be to just use the original version in this PR - I took that straight from the Gradle codebase, so I assume that that's very battle-tested and reliable

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok.
Could you please specify what problems have appeared?
This may come in handy in the future in other places

"file",
"",
File(reportDir.get().asFile.canonicalPath, "index.html").toURI().path,
null,
null,
).toASCIIString()
logger.lifecycle("Kover: HTML report for '$projectPath' $clickablePath")
return true
}
}