Skip to content

Remove Documentation type from Reflect #10608

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

Conversation

abgruszecki
Copy link
Contributor

Nothing but the .raw field is set when we load documentation from Tasty. Right now, it doesn't make sense to expose Documentation as a separate type.

It might make sense to expose it later when we decide how exactly we expose cooking comments.

@nicolasstucki
Copy link
Contributor

Removing the Documentation abstraction seems like a good idea. We can always add back an abstraction for it in a future version if we want to expose more functionality.

@nicolasstucki
Copy link
Contributor

@abgruszecki this PR needs to be rebased

@abgruszecki abgruszecki force-pushed the scala3doc/tasty-reflect-hide-comments branch 2 times, most recently from 7f51b39 to d26e8b6 Compare December 7, 2020 13:02
@@ -2294,14 +2294,14 @@ class QuotesImpl private (using val ctx: Context) extends Quotes, QuoteUnpickler
def pos: Option[Position] =
if self.exists then Some(self.sourcePos) else None

def documentation: Option[Documentation] =
def docstring: Option[String] =
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def docstring: Option[String] =
def docString: Option[String] =

@@ -3070,7 +3068,7 @@ trait Quotes { self: runtime.QuoteUnpickler & runtime.QuoteMatching =>
def pos: Option[Position]

/** The documentation for this symbol, if any */
def documentation: Option[Documentation]
def docstring: Option[String]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def docstring: Option[String]
def docString: Option[String]

def documentation = sym.documentation match
case Some(comment) =>
Map(ctx.sourceSet -> parseComment(comment, sym.tree))
def documentation = sym.docstring match
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def documentation = sym.docstring match
def documentation = sym.docString match

@@ -21,8 +21,8 @@ class DocumentationInspector extends TastyInspector {

override def traverseTree(tree: Tree)(owner: Symbol): Unit = tree match {
case tree: Definition =>
tree.symbol.documentation match {
case Some(doc) => println(doc.raw)
tree.symbol.docstring match {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
tree.symbol.docstring match {
tree.symbol.docString match {

@nicolasstucki nicolasstucki changed the title Remove Documentation from Tasty Reflect Remove Documentation type from Reflect Dec 7, 2020
@abgruszecki
Copy link
Contributor Author

@nicolasstucki https://www.google.com/search?hl=en&q=docstring docstring is the common spelling.

@nicolasstucki
Copy link
Contributor

It is the only method that is not camelCase in this API.

@abgruszecki
Copy link
Contributor Author

It is in camel case, it's just that it's spelled docstring.

@nicolasstucki
Copy link
Contributor

ok

Nothing but the .raw field is set when we load documentation from Tasty. It
doesn't make sense to expose Documentation as a separate data type.
@abgruszecki abgruszecki force-pushed the scala3doc/tasty-reflect-hide-comments branch from d26e8b6 to eae6d18 Compare December 7, 2020 16:35
@abgruszecki abgruszecki merged commit feb8b2e into scala:master Dec 7, 2020
@abgruszecki abgruszecki deleted the scala3doc/tasty-reflect-hide-comments branch December 7, 2020 19:10
@Kordyjan Kordyjan added this to the 3.0.0 milestone Aug 2, 2023
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.

3 participants