lampepfl / dotty-feature-requests

Historical feature requests. Please create new feature requests at https://github.com/lampepfl/dotty/discussions/new?category=feature-requests
31 stars 2 forks source link

Automatically turn off colored output when not run in a terminal #39

Open smarter opened 7 years ago

smarter commented 7 years ago

We should use the same detection mechanism now used by scalac: https://github.com/scala/scala/pull/5663/files#diff-54246962bf5d9d33d2a44254c9422eb3R157

felixmulder commented 7 years ago

Doesn't work in sbt when tried on my machine.

smarter commented 7 years ago

What doesn't work?

felixmulder commented 7 years ago

The detection used by:

+  private[scala] def coloredOutputEnabled: Boolean = propOrElse("scala.color", "auto") match {
+    case "auto" => System.console() != null && !isWin
+    case a if a.toLowerCase() == "true" => true
+    case _ => false
+  }
felixmulder commented 7 years ago

I think it has to do with the redirects on the sbt level.

Varunram commented 7 years ago

This still a bug? Any steps for reproduction? Thanks!

smarter commented 7 years ago

Run the following in a shell:

mkdir try
echo "dummy" > try/dummy.scala
./bin/dotc try/dummy.scala 2> out.txt

if you open out.txt in a text editor, you'll see escape sequences to represent the color, this is because the default value of the -color flag is always if we use -color:never then no escape code is part of the file:

./bin/dotc -color:never try/dummy.scala 2> out.txt

But it would be better if we could add a -color:auto setting that would be the default and only enable colors when we're outputting to a terminal. This is what the System.console() != null && !isWin check that scalac does is for.

Varunram commented 7 years ago

Wouldn't it be better to modify the current always code to include our new auto definition and simply rename always as auto?

smarter commented 7 years ago

No, it's useful to keep -color:always because the detection of terminal done by -color:auto might not always work.

Varunram commented 7 years ago

Well, I'm facing a few problems and I don't know what to do. Here's a part of my patch. The conclusion I came to was that it could be a problem in Highlighting.scala I've attached my pretty basic patch below. Would love your views. Thanks for your time!

diff --git a/compiler/src/dotty/tools/dotc/printing/Highlighting.scala b/compiler/src/dotty/tools/dotc/printing/Highlighting.scala
index 3bda7fb7a..b29551cb5 100644
--- a/compiler/src/dotty/tools/dotc/printing/Highlighting.scala
+++ b/compiler/src/dotty/tools/dotc/printing/Highlighting.scala
@@ -15,7 +16,12 @@ object Highlighting {

     def show(implicit ctx: Context) =
       if (ctx.settings.color.value == "never") text
-      else highlight + text + Console.RESET
+      else {
+        if(System.console() != null)
+        highlight + text + Console.RESET
+        else
+        text
+      }

     override def toString =
       highlight + text + Console.RESET
@@ -30,7 +36,12 @@ object Highlighting {
   abstract class Modifier(private val mod: String, text: String) extends Highlight(Console.RESET) {
     override def show(implicit ctx: Context) =
       if (ctx.settings.color.value == "never") ""
-      else mod + super.show
+      else{
+        if(System.console() != null)
+        mod + super.show
+        else 
+        ""
+      } 
   }
Varunram commented 7 years ago

CC: @felixmulder @smarter Thanks! Edit: I forgot to mention that the auto works courtesy other changes, just that this doesn't