Skip to content

Scala3doc: Bugfixes related to symbol names and extension methods #10334

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

Merged
merged 5 commits into from
Nov 23, 2020

Conversation

pikinier20
Copy link
Contributor

closes #10329
closes #227

@@ -168,7 +168,7 @@ case class TastyParser(qctx: QuoteContext, inspector: DokkaBaseTastyInspector, c

private def errorMsg[T](a: Any, m: => String, e: Throwable): Option[T] =
val msg = try m catch case e: Throwable => a.toString
println(s"ERROR: tree is faling: msg")
println(s"ERROR: tree is faling: $msg")
Copy link
Contributor

Choose a reason for hiding this comment

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

good catch!

Copy link
Contributor

@romanowski romanowski left a comment

Choose a reason for hiding this comment

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

Logic looks ok, I've asked for some stylistic changes

@@ -17,13 +17,13 @@ class ImplicitMembersExtensionTransformer(ctx: DokkaContext) extends Documentabl

def expandMember(outerMembers: Seq[Member])(c: Member): Member =
val companion = c match
case classlike: DClass => ClasslikeExtension.getFrom(classlike).flatMap(_.companion).flatMap(classlikeMap.get)
case classlike: DClass => ClasslikeExtension.getFrom(classlike).flatMap(_.companion).map(classlikeMap)
Copy link
Contributor

Choose a reason for hiding this comment

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

This change may not be safe (it will throw exception when classlikeMap does not contains companion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it to safe call (Map.get) and log when classlikeMap is missing companion

@@ -248,6 +248,14 @@ class ScalaPageContentBuilder(

def signature(d: Documentable) = addChildren(signatureProvider.signature(d).asScala.toList)

private def buildSignature(d: Documentable, s: Signature) = signatureProvider.asInstanceOf[ScalaSignatureProvider].signature(d, s)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can move signature to ScalaSignatureProvider object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are using contentBuilder field from ScalaSignatureProvider class which will be hard to obtain in object

@@ -31,6 +31,11 @@ class ScalaSignatureProvider(contentConverter: CommentsToContentConverter, logge
def driLink(text: String, dri: DRI): SignatureBuilder = ContentNodeBuilder(builder.driLink(text, dri))
}

def signature(d: Documentable, s: Signature) = signatureContent(d){ builder =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use Member here?

Copy link
Contributor

@abgruszecki abgruszecki left a comment

Choose a reason for hiding this comment

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

@pikinier20 I fixed indentation & mixed brace/braceless style & whitespace issues for you.

I also removed the outdated code for handling extension names.

The PR LGTM now.

@romanowski romanowski merged commit 31566b7 into scala:master Nov 23, 2020
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.

Scala3doc: extension methods are not grouped properly Frontend bugs
3 participants