Init colorama only in Windows legacy terminal #238
Conversation
jiasli
changed the title
| @@ -101,6 +101,8 @@ def __init__(self, | |||
|
|
|||
| self.only_show_errors = self.config.getboolean('core', 'only_show_errors', fallback=False) | |||
| self.enable_color = self._should_enable_color() | |||
| # Init colorama only in Windows legacy terminal | |||
| self._should_init_colorama = self.enable_color and os.name == 'nt' and not is_modern_terminal() | |||
There was a problem hiding this comment.
Why not (self.enable_color or _KNACK_TEST_FORCE_ENABLE_COLOR) and ...? Does that mean knack test always use colorama no matter it's modern terminal or legacy terminal?
There was a problem hiding this comment.
Why not
(self.enable_color or _KNACK_TEST_FORCE_ENABLE_COLOR) and ...?
The action scope of mocked _KNACK_TEST_FORCE_ENABLE_COLOR is CLI.invoke. If _KNACK_TEST_FORCE_ENABLE_COLOR is used in CLI.__invoke__, CLI.invoke won't be affected and won't have colorama enabled.
Lines 36 to 37 in cd59031
Does that mean knack test always use colorama no matter it's modern terminal or legacy terminal?
Yes.
There was a problem hiding this comment.
Dropped _KNACK_TEST_FORCE_ENABLE_COLOR. Now strip control sequences by knack itself:
Lines 57 to 58 in e5e0529
| def assert_in_multi_line(sub_string, string): | ||
| # assert sub_string is in string, with all whitespaces, line breaks and control sequences ignored | ||
| assert _remove_whitespace(sub_string) in _remove_control_sequence(_remove_whitespace(string)) |
There was a problem hiding this comment.
This function does the job of removing control sequences which are previously removed by colorama.
Init colomara only on Windows, because
_should_init_coloramacan be furtherly disabled by Azure CLI