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

Expose types in worksheet #1752

Closed
kpbochenek opened this issue May 15, 2020 · 2 comments · Fixed by #1990
Closed

Expose types in worksheet #1752

kpbochenek opened this issue May 15, 2020 · 2 comments · Fixed by #1990
Assignees
Labels
improvement Not a bug or a feature, but something general we can improve
Milestone

Comments

@kpbochenek
Copy link
Contributor

kpbochenek commented May 15, 2020

I took a look at possibility to expose types in worksheets and before starting a work I would like to get opinion what is your point of view on that.

For all the worksheet evaluation responsible is dependency mdoc.
We simply call val worksheet = mdoc.evaluateWorksheet(input.path, input.value)
All logic what to display is handled by mdoc, we only get String as summary of each evaluated line.

Currently we get output:
2020-05-15-100014_563x408_scrot

Just tried quick hack to see if we have any types available during evaluation and as PoC it looks like that:
2020-05-15-100244_694x424_scrot

Changes should mostly be done in mdoc in how it generates a summary for each line.
To not change any current behaviour I thought we could add mdoc.evaluateWorksheetWithTypes to interface which will add types to output in a format we will be happy with.
About how to know when to execute evaluateWorksheet and when evaluateWorksheetWithType I thought we could use lenses for that.
Inspired by dottyIDE:
2020-05-15-100721_329x99_scrot
We could have on top a lens "Show Types" that would be changed to "Hide Types" and back when you click it.
We could keep Map[worksheet_path, show_types] in Metals and based on that decide if user wants to see types or not.

Opinions? :)

@olafurpg
Copy link
Member

Thank you for this detailed writeup!

MDoc has access to pprint.TPrint[T] instances for all bound variables here
https://github.com/scalameta/mdoc/blob/7a74f604a3c2099070bec601eee77dced659b2b1/runtime/src/main/scala/mdoc/document/Document.scala#L28

The printer for variables is configurable via Settings here https://github.com/scalameta/mdoc/blob/7a74f604a3c2099070bec601eee77dced659b2b1/mdoc/src/main/scala/mdoc/internal/cli/Settings.scala#L147

Changes should mostly be done in mdoc in how it generates a summary for each line. To not change any current behaviour I thought we could add mdoc.evaluateWorksheetWithTypes

I would treat this as a setting which is configurable via some withX method here https://github.com/scalameta/mdoc/blob/7a74f604a3c2099070bec601eee77dced659b2b1/mdoc-interfaces/src/main/scala/mdoc/interfaces/Mdoc.java#L10

We could have on top a lens "Show Types" that would be changed to "Hide Types" and back when you click it.

I think that's a good idea if we want to make this functionality configurable. Do we have evidence to support that we don't want this feature enabled by default without an option to hide types? Users who want to see the full runtime value can always hover over the statement (or use shortcut to trigger hover).

Just tried quick hack to see if we have any types available during evaluation and as PoC it looks like that:

nit: I would use Map[Int, String] = <value> as a prefix instead of Map[Int, String: <value> since the colon is always used before the type in Scala.

@kpbochenek kpbochenek self-assigned this May 19, 2020
@kpbochenek kpbochenek added improvement Not a bug or a feature, but something general we can improve and removed question labels May 19, 2020
@kpbochenek
Copy link
Contributor Author

kpbochenek commented May 19, 2020

Referenced changes in mdoc: scalameta/mdoc#335

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Not a bug or a feature, but something general we can improve
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants