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

fix: obey hover markup kind capability #6303

Merged
merged 11 commits into from
Apr 23, 2024
Merged

Conversation

kasiaMarek
Copy link
Contributor

@kasiaMarek kasiaMarek commented Apr 12, 2024

resolves: #6293

@kasiaMarek kasiaMarek marked this pull request as draft April 12, 2024 13:59
@kasiaMarek kasiaMarek marked this pull request as ready for review April 17, 2024 15:01
@kasiaMarek kasiaMarek requested a review from tgodzik April 17, 2024 15:02
* Returns the type of the expression at the given position along with the
* symbol of the referenced symbol.
*/
public CompletableFuture<Optional<HoverSignature>> hover(OffsetParams params, ContentType contentType) {
Copy link
Contributor

Choose a reason for hiding this comment

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

could we instead of adding a new method add it to PresentationCompilerConfig ?

@@ -29,22 +30,23 @@ import scala.meta.pc.SymbolDocumentation
* Handles both javadoc and scaladoc.
*/
class Docstrings(index: GlobalSymbolIndex) {
val cache = new TrieMap[String, SymbolDocumentation]()
val cache = new TrieMap[(String, ContentType), SymbolDocumentation]()
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
val cache = new TrieMap[(String, ContentType), SymbolDocumentation]()
val cache = new TrieMap[Content, SymbolDocumentation]()

Maybe create a case class for it? Or:

sealed trait Content extends Any
class Markdown(val text: String) extends AnyVal with Content
class Plain(val text: String) extends AnyVal with Content

def toPlaintext(b: Body): String =
Option(b).map(body => printBlocks(body.blocks)).mkString("\n")

def toPlaintext(c: Comment, docstring: String): String = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Any chance to share some of MarkdownGenerator implementation?

Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

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

Great work! One minor thing, otherwise we are good to merge.

class Plain(val text: String) extends AnyVal with Content

object Content {
def from(text: String, contentType: ContentType): AnyVal with Content =
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 from(text: String, contentType: ContentType): AnyVal with Content =
def from(text: String, contentType: ContentType): Content =

would be enough

@kasiaMarek kasiaMarek merged commit fd8a191 into scalameta:main Apr 23, 2024
24 of 25 checks passed
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.

Support plaintext Hover responses
2 participants