Dottydoc: Show implicit modifier in implicit function types #4954
Conversation
| @@ -28,42 +28,4 @@ object references { | |||
| } | |||
| final case class NoLink(title: String, target: String) extends MaterializableLink | |||
|
|
|||
| implicit class ReferenceShower(val ref: Reference) extends AnyVal { | |||
There was a problem hiding this comment.
As an outsider: This PR mostly makes sense to me, but I'm not sure why ReferenceShower is removed here.
Not that I'm attached to it — it seems pretty-printing won't insert necessary extra parentheses — seems we'd have gotten A & B => C from both (A & B) => C and A & (B => C). But is this handled better by the new code? It seems harder to get right once you flatten everything into nested maps, as asJava seems to do.
For the record, PlainPrinter/RefinedPrinter start handling this correctly in #4072 (tho the basic infrastructure was in place): recursive calls must pass the current precedence and insert parentheses around nodes with a lower precedence than the current one.
But I wonder if DottyDoc should reimplement this logic instead of reusing RefinedPrinter, since each deviation will confuse users. The only different requirement seems that RefinedPrinter uses a pretty-printing library designed to insert line-breaks where needed, but flattening that to String without extra line-breaks should be feasible.
There was a problem hiding this comment.
I removed it because the functionality is actually implemented twice: in ReferenceShower and RenderReference.
Instead of generating a string from the reference, we keep it in "java form" and use the tag to actually render it.
There was a problem hiding this comment.
That makes sense, but the precedence bug seems still an issue; I've confirmed it (tho without this PR) and opened #4966.
Duhemm
force-pushed
the
fix/dottydoc-implicit-function-type
branch
from
a97b7a0 to
5e61866
Compare
|
Rebased. @Blaisorblade Do you mind reviewing this PR, since you already had a look? |
There was a problem hiding this comment.
LGTM. The main change looks good, and dropping ReferenceShower makes sense to keep more information for later rendering. The harder question was #4966, but that is orthogonal. Approving.
|
@Duhemm wanted to merge this, but the CI hasn't passed yet, so I can't. What's up with it? Can you please take care of it or reassign it? |
Duhemm
force-pushed
the
fix/dottydoc-implicit-function-type
branch
from
342066e to
e735f3d
Compare
No description provided.