Fix up colored log support

 * do not use pdict
 * do not enable color support by default. once we have the new
   getopt version, we can add a new type of command line flag
   for that.
 * fix Dialyzer warnings
 * use atom instead of boolean
 * use better name for internal function
 * do not try (and fail) to access rebar's app env in retest test

3 files changed, 36 insertions, 44 deletions

diff --git a/ebin/rebar.app b/ebin/rebar.app
index 985dbb9..710f8b2 100644
--- a/ebin/rebar.app
+++ b/ebin/rebar.app
@@ -69,7 +69,7 @@
          {log_level, warn},
 
          %% Log colored
-         {log_colored, true},
+         {log_colored, uncolored},
 
          %% any_dir processing modules
          {any_dir_modules, [
diff --git a/inttest/logging/logging_rt.erl b/inttest/logging/logging_rt.erl
index 11cb0cd..2709a84 100644
--- a/inttest/logging/logging_rt.erl
+++ b/inttest/logging/logging_rt.erl
@@ -38,25 +38,9 @@ files() ->
 
 run(_Dir) ->
     SharedExpected = "==> logging_rt \\(compile\\)",
+    {ERROR, WARN, INFO, DEBUG} = log_labels(),
     %% provoke ERROR due to an invalid app file
     retest:log(info, "Check 'compile' failure output~n"),
-    {ERROR, WARN, INFO, DEBUG} =
-        case application:get_env(rebar, log_colored) of
-            {ok, true} ->
-                {
-                 "\\e\\[1m\\e\\[31mERROR: \\e\\[0m",
-                 "\\e\\[33mWARN: \\e\\[0m",
-                 "\\e\\[32mINFO: \\e\\[0m",
-                 "\\e\\[34mDEBUG: \\e\\[0m"
-                };
-            _ ->
-                {
-                 "ERROR: ",
-                 "WARN: ",
-                 "INFO: ",
-                 "DEBUG: "
-                }
-        end,
     ok = check_output("./rebar compile -q", should_fail,
                       [SharedExpected, ERROR],
                       [WARN, INFO, DEBUG]),
@@ -76,6 +60,14 @@ run(_Dir) ->
                       [ERROR, INFO]),
     ok.
 
+log_labels() ->
+    {
+      "(\\e\\[1m\\e\\[31mERROR: \\e\\[0m|ERROR: )",
+      "(\\e\\[33mWARN: \\e\\[0m|WARN: )",
+      "(\\e\\[32mINFO: \\e\\[0m|INFO: )",
+      "(\\e\\[34mDEBUG: \\e\\[0m|DEBUG: )"
+    }.
+
 check_output(Cmd, FailureMode, Expected, Unexpected) ->
     case {retest:sh(Cmd), FailureMode} of
         {{error, _}=Error, should_succeed} ->
diff --git a/src/rebar_log.erl b/src/rebar_log.erl
index 91fb659..abc2ac5 100644
--- a/src/rebar_log.erl
+++ b/src/rebar_log.erl
@@ -43,6 +43,9 @@
 %% Public API
 %% ===================================================================
 
+%% TODO: Once we have the new getopt version, use a flag that
+%% takes an optional arg but has a default value and fetch the
+%% setting via rebar_config, after it has been set in rebar:main.
 init(Config) ->
     Verbosity = rebar_config:get_global(Config, verbose, default_level()),
     case valid_level(Verbosity) of
@@ -50,27 +53,17 @@ init(Config) ->
         ?WARN_LEVEL  -> set_level(warn);
         ?INFO_LEVEL  -> set_level(info);
         ?DEBUG_LEVEL -> set_level(debug)
-    end,
-    LogColored = rebar_config:get_global(Config, log_colored, true),
-    set_log_colored(LogColored).
-
+    end.
 
 set_level(Level) ->
-    erlang:put(rebar_log_level, Level).
-
-set_log_colored(true) ->
-    erlang:put(rebar_log_colored, true),
-    ok;
-set_log_colored(_LogColored) ->
-    erlang:put(rebar_log_colored, false),
-    ok.
+    ok = application:set_env(rebar, log_level, Level).
 
 log(Level, Str, Args) ->
     log(standard_io, Level, Str, Args).
 
 log(Device, Level, Str, Args) ->
-    LogLevel = erlang:get(rebar_log_level),
-    LogColored = erlang:get(rebar_log_colored),
+    {ok, LogLevel} = application:get_env(rebar, log_level),
+    {ok, LogColored} = application:get_env(rebar, log_colored),
     case should_log(LogLevel, Level) of
         true ->
             io:format(Device, log_prefix(Level, LogColored) ++ Str, Args);
@@ -101,33 +94,40 @@ should_log(error, error) -> true;
 should_log(error, _)     -> false;
 should_log(_, _)         -> false.
 
-log_prefix(Level, _Colored = false) ->
+log_prefix(Level, uncolored) ->
     log_prefix(Level);
-log_prefix(Level, _Colored = true) ->
-    color_from_level(Level) ++ log_prefix(Level) ++ reset_color().
+log_prefix(Level, colored) ->
+    color_for_level(Level) ++ log_prefix(Level) ++ reset_color().
 
 log_prefix(debug) -> "DEBUG: ";
 log_prefix(info)  -> "INFO:  ";
 log_prefix(warn)  -> "WARN:  ";
 log_prefix(error) -> "ERROR: ".
 
-color_from_level(debug) ->
+color_for_level(debug) ->
     color_foreground(blue);
-color_from_level(info) ->
+color_for_level(info) ->
     color_foreground(green);
-color_from_level(warn) ->
+color_for_level(warn) ->
     color_foreground(yellow);
-color_from_level(error) ->
+color_for_level(error) ->
     color_bold() ++ color_foreground(red).
 
-color_foreground(black)   -> "\e[30m";
+%% -type color() :: 'black' | 'red' | 'green' | 'yellow'
+%%                | 'blue' | 'magenta' | 'cyan' | 'white'.
+%% -spec color_foreground(color()) -> string().
+%%
+%% To silence Dialyzer, disable following colors, because they're
+%% unused as of right now: black, magenta, cyan, white
+%%
+%% color_foreground(black)   -> "\e[30m";
+%% color_foreground(magenta) -> "\e[35m";
+%% color_foreground(cyan)    -> "\e[36m";
+%% color_foreground(white)   -> "\e[37m";
 color_foreground(red)     -> "\e[31m";
 color_foreground(green)   -> "\e[32m";
 color_foreground(yellow)  -> "\e[33m";
-color_foreground(blue)    -> "\e[34m";
-color_foreground(magenta) -> "\e[35m";
-color_foreground(cyan)    -> "\e[36m";
-color_foreground(white)   -> "\e[37m".
+color_foreground(blue)    -> "\e[34m".
 
 color_bold()  -> "\e[1m".
 reset_color() -> "\e[0m".