Skip to content

Fix #6270: Allow change of color setting in TASTy Reflect #6276

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 1 commit into from
Apr 10, 2019

Conversation

nicolasstucki
Copy link
Contributor

@nicolasstucki nicolasstucki commented Apr 10, 2019

This provides a way to enabled and disable color when printing. The default is set to the current compiler setting.

Now, if someone wants a show that never prints the colors it can be implemented with

implicit def (expr: scala.quoted.Expr[T]) showColorless[T] (implicit reflect: Reflection): String = {
  import reflect._
  expr.unseal.showCode(the[Context].withoutColors)
}

Note that Tree.showCode will change name to Tree.show to match Expr.show.

Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

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

Otherwise, LGTM

/** Show a source code like representation of this type */
/** Show a source code like representation of this type
* Will print Ansi colors if ctx.printColors is enabled.
*/
def show(implicit ctx: Context): String =
unseal.showCode
Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to default to no colors other than ctx.printColors as that's what users want most of the time.

Otherwise, users will be forced to write x.show(the[Context].withColors) instead of x.show.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can do either with a default color: Boolean = false or have two methods:

  • show
  • showColored

Both are more friendly than x.show(the[Context].withColors)

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 cannot a default the is different for the compiler setting or every other use case will break.

As shown in the example it is easy to add your own version of show where you disregard the default settings.

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 can always add some variant later

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 it's better to have it now as we go -- we will forget all these minor usability issues. If such usability issues accumulate, the user experience with Tasty Reflect will degrade.

@nicolasstucki nicolasstucki merged commit 9ab10e7 into scala:master Apr 10, 2019
@ghost ghost removed the stat:needs review label Apr 10, 2019
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.

2 participants