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

Conversation

Ezard
Copy link
Contributor

@Ezard Ezard commented Sep 24, 2023

Fixes HTML report paths not being clickable

Closes #470

@Ezard Ezard changed the title Update HTML report path to be correct on all platforms Update HTML report path to be clickable Sep 25, 2023
@@ -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

@shanshin shanshin merged commit 58e2c95 into Kotlin:main Oct 10, 2023
@shanshin
Copy link
Collaborator

Thanks for the effort!

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.

HTML report paths are not clickable
2 participants