Skip to content

Commit 1c4489c

Browse files
committed
Add file path escaping to Scaladoc
As Jekyll (and in extension GitHub pages) makes starting a file/folder name with a couple characters illegal, those character are now being replaced with a "$charname" format. To avoid having $underlinedocs, $underlineblog in url by default, those default static site directories are stripped of the underline character. Some tests concerning links were adjusted to accomodate the changes. Example testcase for illegal jekyll chars in Scaladoc was also added.
1 parent d698cba commit 1c4489c

File tree

11 files changed

+49
-19
lines changed

11 files changed

+49
-19
lines changed
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package _docs.tests
1+
package docs.tests
22

33
class Adoc:
44
def foo = 123
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package example
2+
3+
package jekyllEscapes:
4+
object #~
5+
object ~>~

scaladoc/src/dotty/tools/scaladoc/renderers/Locations.scala

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,9 @@ trait Locations(using ctx: DocContext):
4747
case "<empty>" :: tail => "_empty_" :: tail
4848
case other => other
4949
if ctx.args.apiSubdirectory then "api" :: fqn else fqn
50-
cache.put(dri, path)
51-
path
50+
val escapedPath = escapePath(path)
51+
cache.put(dri, escapedPath)
52+
escapedPath
5253
case cached => cached
5354

5455
private def unknownPage(dri: DRI): String =

scaladoc/src/dotty/tools/scaladoc/renderers/Renderer.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ abstract class Renderer(rootPackage: Member, val members: Map[DRI, Member], prot
101101
val all = navigablePage +: redirectPages
102102
// We need to check for conflicts only if we have top-level member called docs
103103
val hasPotentialConflict =
104-
rootPackage.members.exists(m => m.name.startsWith("_docs"))
104+
rootPackage.members.exists(m => m.name.startsWith("docs"))
105105

106106
if hasPotentialConflict then
107107
def walk(page: Page): Unit =

scaladoc/src/dotty/tools/scaladoc/site/StaticSiteContext.scala

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,15 @@ class StaticSiteContext(
2323
val docsPath = root.toPath.resolve("_docs")
2424
val blogPath = root.toPath.resolve("_blog")
2525

26-
val relativizeFrom = if args.apiSubdirectory then docsPath else root.toPath
26+
def relativizeAndStrip(path: Path): Path =
27+
if args.apiSubdirectory then
28+
docsPath.relativize(path)
29+
else
30+
val relativised = root.toPath.relativize(path).toString()
31+
// we remove all the '_' from _docs, _blog ... as having
32+
// $underlinedocs in the url by default will look quite ugly
33+
val withUnderlineRemoved = relativised.replaceFirst("^\\_", "")
34+
Paths.get(withUnderlineRemoved)
2735

2836
val siteExtensions = Set(".html", ".md")
2937

@@ -91,7 +99,7 @@ class StaticSiteContext(
9199
}
92100

93101
def driFor(dest: Path): DRI =
94-
val rawFilePath = relativizeFrom.relativize(dest)
102+
val rawFilePath = relativizeAndStrip(dest)
95103
val pageName = dest.getFileName.toString
96104
val dotIndex = pageName.lastIndexOf('.')
97105

scaladoc/src/dotty/tools/scaladoc/tasty/SymOps.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ class SymOpsWithLinkCache:
259259
else
260260
(sym.className, sym.anchor)
261261

262-
val location = (sym.packageNameSplitted ++ className).map(escapeFilename(_))
262+
val location = (sym.packageNameSplitted ++ className).map(filename => if filename != ".." then escapeFilename(filename) else filename)
263263

264264
val externalLink = {
265265
import reflect._

scaladoc/src/dotty/tools/scaladoc/util/escape.scala

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,19 @@ object Escape:
99
.replace("/", "$div")
1010
.replace("\\", "$bslash")
1111
if escaped != filename then escaped + "$" else escaped
12+
13+
def escapePath(path: Seq[String]) = path.escapeIllegalJekyllChars
14+
15+
/**
16+
* Jekyll (also used by GitHub Pages) by default makes a couple characters
17+
* illegal to use in file name beginnings. Those can be disabled manually,
18+
* however as suggested by #14612 and like supported in Scala 2 scaladoc
19+
* it's better to have those characters escaped here by default.
20+
*/
21+
extension (path: Seq[String]) private def escapeIllegalJekyllChars: Seq[String] =
22+
path.map(
23+
_.replaceAll("^~", "\\$tilde")
24+
.replaceAll("^\\_", "\\$underline")
25+
.replaceAll("^\\.", "\\$dot")
26+
.replaceAll("^\\#", "\\$hash")
27+
)

scaladoc/test/dotty/tools/scaladoc/ReportingTest.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,11 @@ class ReportingTest:
7676
val docsRoot = testDocPath.resolve("conflicts-pages").toString
7777
checkReportedDiagnostics(_.copy(
7878
docsRoot = Some(docsRoot),
79-
tastyFiles = tastyFiles("tests", rootPck = "_docs")
79+
tastyFiles = tastyFiles("tests", rootPck = "docs")
8080
)){ diag =>
8181
assertNoWarning(diag)
8282
val Seq(msg) = diag.errorMsgs.map(_.toLowerCase)
83-
Seq("conflict","api", "static", "page", "_docs/tests/adoc.html")
83+
Seq("conflict","api", "static", "page", "docs/tests/adoc.html")
8484
.foreach( word =>
8585
Assert.assertTrue(s"Error message: $msg should contains $word", msg.contains(word))
8686
)

scaladoc/test/dotty/tools/scaladoc/renderers/LocationTests.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ class LocationTests:
4444
locations.escapedAbsolutePathWithAnchor(new DRI(location, anchor))
4545

4646
assertEquals(
47-
"scala/%23::.html#abcde",
48-
pathWithAnchor("scala.#::", "abcde")
47+
"scala/$hash%23::.html#abcde",
48+
pathWithAnchor("scala.##::", "abcde")
4949
)
5050

5151
assertEquals(

scaladoc/test/dotty/tools/scaladoc/site/NavigationTest.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,5 +44,5 @@ class NavigationTest extends BaseHtmlTest:
4444
)),
4545
))
4646

47-
testNavMenu("_docs/Adoc.html", topLevelNav)
47+
testNavMenu("docs/Adoc.html", topLevelNav)
4848
}

scaladoc/test/dotty/tools/scaladoc/site/SiteGeneratationTest.scala

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,13 @@ class SiteGeneratationTest extends BaseHtmlTest:
3232
}
3333

3434
def testDocPages()(using ProjectContext) =
35-
checkFile("_docs/Adoc.html")(title = "Adoc", header = "Header in Adoc", parents = Seq(projectName))
36-
checkFile("_docs/dir/index.html")(title = "A directory", header = "A directory", parents = Seq(projectName))
37-
checkFile("_docs/dir/nested.html")(
35+
checkFile("docs/Adoc.html")(title = "Adoc", header = "Header in Adoc", parents = Seq(projectName))
36+
checkFile("docs/dir/index.html")(title = "A directory", header = "A directory", parents = Seq(projectName))
37+
checkFile("docs/dir/nested.html")(
3838
title = "Nested in a directory", header = "Nested in a directory", parents = Seq(projectName, "A directory"))
3939

4040
def testDocIndexPage()(using ProjectContext) =
41-
checkFile("_docs/index.html")(title = projectName, header = s"$projectName in header")
41+
checkFile("docs/index.html")(title = projectName, header = s"$projectName in header")
4242

4343
def testApiPages(
4444
mainTitle: String = "API",
@@ -66,13 +66,13 @@ class SiteGeneratationTest extends BaseHtmlTest:
6666
testDocIndexPage()
6767
testApiPages()
6868

69-
withHtmlFile("_docs/Adoc.html"){ content =>
69+
withHtmlFile("docs/Adoc.html"){ content =>
7070
content.assertAttr("p a","href", "../tests/site/SomeClass.html")
7171
}
7272

7373
withHtmlFile("tests/site/SomeClass.html"){ content =>
7474
content.assertAttr(".breadcrumbs a","href",
75-
"../../_docs/index.html", "../../index.html", "../site.html", "SomeClass.html"
75+
"../../docs/index.html", "../../index.html", "../site.html", "SomeClass.html"
7676
)
7777
}
7878
}
@@ -98,7 +98,7 @@ class SiteGeneratationTest extends BaseHtmlTest:
9898
@Test
9999
def staticLinking() = withGeneratedSite(testDocPath.resolve("static-links")){
100100

101-
withHtmlFile("_docs/Adoc.html"){ content =>
101+
withHtmlFile("docs/Adoc.html"){ content =>
102102
content.assertAttr("p a","href",
103103
"dir/html.html",
104104
"dir/name...with..dots..html",

0 commit comments

Comments
 (0)