Overriding ansi.style_error()

[kotfu]

While working on documentation for using ansi color support, I noticed that ansi.py defines three convenience methods for styling output:

• style_success()
• style_warning()
• style_error()

These methods are all partial objects created with functools.partial() which supply certain arguments to style(). These methods all behave similarly, I'll give an example for style_error(), but the concept applies to the other methods too.

ansi.style_error() adds ANSI escape sequences to style the message as red text. Say I am creating my own cmd2 based app, and I am following our recommendations to use perror() to output error messages. But I decide in my app I want my error messages to have a yellow background with red text. There are two ways for me to accomplish my objective:

1. override the perror() method and change the styling applied to the message. The problem with this method is that there are a couple of places in argparse_completer.py and argparse_custom.py that call ansi.style_error() directly, so those error messages won't match my application's custom error message style.

2. redefine ansi.style_error() so it uses my custom error message color scheme. This makes error messages consistent, but is very brittle and can make my application hard to debug.

Should we consider a different implementation of the "standardizing error, warning, and success text styling" idea that makes it easier for an app developer to override the text coloring we choose to use in cmd2?

One possible approach would be to move style_error() from a function in ansi.py to a method on Cmd2.cmd.

[kmvanbrunt]

The design was intended for developers to choose option 2. But yes, it can be brittle. Your ticket caused me to find a bug with our usage of these styles. See PR #820.

I don't mind changing our approach, but ansi was meant to be a fairly independent module. So I'm not sure about moving these functions into cmd2.Cmd. For some software projects I work on, there are cmd2-based CLI portions as well as typical command line utilities. These command line utilities make use of the ansi.style wrappers without needing to instantiate a Cmd object.

[kotfu]

Makes sense to keep ansi somewhat independent. After further review, it appears that the argparse_completer and argparse_custom modules also use ansi, and can also be used independently of cmd2.Cmd. It appears we have created a circular problem in our current implementation:

1. We want ansi to be usable as a stand-alone module
2. We want argparse_completer and argparse_custom to have ANSI styled output, and to be usable independent of cmd2.Cmd. When used within cmd2.Cmd these should be well-behaved and consistent with other cmd2.Cmd usage.
3. We want an easy, intuitive way for other developers to consistently override the color schemes for success, warnings, and errors, and if they do so, have all error messages generated using their custom scheme.

The current implementation allows argparse_completer and argparse_custom to be used independent of cmd2.Cmd. But when they are used within cmd2.Cmd, they do not honor the allow_ansi setting; they always generate ANSI escape sequences. It's weird to write documentation recommending that people call the perror() method to consistently output error messages, when we don't do it ourselves. It seems likely that a developer would assume that if they re-implemented perror() they would pick up all of our internally generated errors, and they would not.

I think the problem to be solved is to find a way to keep ansi, argparse_completer, and argparse_custom usable independent of cmd2.Cmd, but when incorporated into cmd2.Cmd, they are well-behaved and honor the cmd2 conventions. I define cmd2 conventions relevant to this use case as: 1) honoring allow_ansi setting, and 2) all internal error output gets sent through perror().

I think we should debate what "well-behaved" means, and I don't have strong feelings about how to prioritize some of these discrepancies we have now. I do feel pretty strongly that we should have consistent behavior, which we don't currently have.

[kmvanbrunt]

Only ansi and argparse_custom can be used independently.

argparse_completer defines the AutoCompleter class which has a cmd2.Cmd member called cmd2_app. AutoCompleter uses this member to access tab-completion settings and passes it as the self parameter to choices and completer methods. AutoCompleter is conceptually a member of cmd2.Cmd defined in another file.

But all of these modules respect allow_ansi because they call ansi_aware_write(). In a cmd2 app, run set allow_ansi never and then cause an argparse error. You'll see it no longer prints in red.

[kotfu]

I see now how the allow_ansi property on cmd2.Cmd passes the setting through to the ansi module. It feels weird to me to have an instance attribute which is really a module attribute in a different module. This could go horribly wrong if you instantiated multiple cmd2.Cmd objects, but I think that's an unlikely edge case. Now that I'm familiar with how this all works, give me a bit to let it settle. I may not think it's as awful as my initial reaction indicated.

However, an alternative implementation which would fix this, and still allow stand alone use of ansi.py might be to put all the functions in ansi.py into a new ANSIWriter class. cmd2.Cmd.__init__() would instantiate this class and save it as a private attribute of the instance. It would then pass through the allow_ansi setting to an attribute of this ANSIWriter instance, (instead of assigning it to an attribute of the module). Stand-alone users of the ansi capabilities would likewise instantiate ANSIWriter, which would still be independent of cmd2.Cmd, and still could have the allow_ansi functionality as an attribute.

[tleonhardt]

I potentially like the idea of an ANSIWriter class which cmd2.Cmd.__init()__ would instantiate an instance of.

[kmvanbrunt]

This approach won't work for the do_* function Cmd2ArgumentParserobjects since they are cmd2.Cmd class members.

This is one reason why ansi.allow_ansi is intended to be an application-wide setting and not an instance setting.

[kotfu]

This hole goes deeper than I expected. It appears Cmd2ArgumentParser works via a global variable and/or adding . I'll wait until someone writes the documentation on how that all works and then revisit the color output section.