Update comments

knack/cli.py

@@ -22,7 +22,9 @@
 
 logger = get_logger(__name__)
 
-KNACK_TEST_FORCE_ENABLE_COLOR = False
+# Temporarily force color to be enabled even when out_file is not stdout.
+# This is only intended for testing purpose.
+_KNACK_TEST_FORCE_ENABLE_COLOR = False
 
 
 class CLI(object):  # pylint: disable=too-many-instance-attributes
@@ -100,14 +102,15 @@ def __init__(self,
 
         self.only_show_errors = self.config.getboolean('core', 'only_show_errors', fallback=False)
 
-        # Color is only enabled when all scenarios are met:
+        # Color is only enabled when all conditions are met:
         #     1. [core] no_color config is not set
         #     2. stdout is a tty
+        #         Otherwise, if the downstream command doesn't support color, Knack will fail with
+        #         BrokenPipeError: [Errno 32] Broken pipe, like `az --version | head --lines=1`
+        #         https://github.com/Azure/azure-cli/issues/13413
         #     3. stderr is a tty
+        #         Otherwise, the output in stderr won't have LEVEL tag
         #     4. out_file is stdout
-        # Otherwise, if the downstream command doesn't support color, Knack will fail with
-        # BrokenPipeError: [Errno 32] Broken pipe, like `az --version | head --lines=1`
-        # https://github.com/Azure/azure-cli/issues/13413
         conditions = (not self.config.getboolean('core', 'no_color', fallback=False),
                       isatty(sys.stdout), isatty(sys.stderr), self.out_file is sys.stdout)
         self.enable_color = all(conditions)
@@ -221,7 +224,7 @@ def invoke(self, args, initial_invocation_data=None, out_file=None):
         exit_code = 0
         try:
             out_file = out_file or self.out_file
-            if out_file is sys.stdout and self.enable_color or KNACK_TEST_FORCE_ENABLE_COLOR:
+            if out_file is sys.stdout and self.enable_color or _KNACK_TEST_FORCE_ENABLE_COLOR:
                 import colorama
                 colorama.init()
                 # point out_file to the new sys.stdout which is overwritten by colorama
@@ -263,7 +266,7 @@ def invoke(self, args, initial_invocation_data=None, out_file=None):
         finally:
             self.raise_event(EVENT_CLI_POST_EXECUTE)
 
-            if self.enable_color or KNACK_TEST_FORCE_ENABLE_COLOR:
+            if self.enable_color or _KNACK_TEST_FORCE_ENABLE_COLOR:
                 import colorama
                 colorama.deinit()
 

tests/util.py

@@ -33,7 +33,7 @@ def wrapper(self):
         cli_logger.handlers.clear()
 
         sys.stdout = sys.stderr = self.io = StringIO()
-        with mock.patch("knack.cli.KNACK_TEST_FORCE_ENABLE_COLOR", True):
+        with mock.patch("knack.cli._KNACK_TEST_FORCE_ENABLE_COLOR", True):
             func(self)
         self.io.close()
         sys.stdout = original_stdout