[refactor-prompt] restore theming support

[ixje]

What current issue(s) does this address, or what feature is it adding?
resolve the following todo of #623

after the previous item, make sure console themes still work. This requires "Format Tokens" from the prompt_toolkit package, but it should only be applied when used in the CLI. Meaning; the business logic should not be coupled to a PromptInterface() instance because that's the one who creates the theme tokens. solution idea: supply print function as param.

How did you solve this problem?
replace built-in print with prompt_print in the classes below Commands as follows:

from neo.Prompt.PromptPrinter import prompt_print as print

This was the most straight forward way to replace all print calls in command code. One advantage is that we can easily swap out the print call for something custom. One place we do this is for testing where we swap out the prompt_toolkit.print_formatted_text with the buildint print such that all our patch('sys.stdout') in test-cases still work.

In the old prompt some part of the command execution would be themed (e.g. red coloured) whereas others not because they used regular prints. Now everything related to a command response is printed themed.

How did you make sure your solution works?
make test and some manual testing

Are there any special changes in the code that we should be aware of?
see the "how solved" section.

Please check the following, if applicable:

• Did you add any tests?
• Did you run make lint?
• Did you run make test?
• Are you making a PR to a feature branch or development rather than master?
• Did you add an entry to CHANGELOG.rst? (if not, please do)

[jseagrave21]

One minor grammatical suggestion. Otherwise, my only question is where is the token_style "number" used? In the original implementation, "number" was used perhaps the most frequently, just for one example see

neo-python/neo/bin/prompt.py

Lines 588 to 589 in e378012

	tokens = [("class:number", out)]
	print_formatted_text(FormattedText(tokens), style=self.token_style)

from show_state which is similar to most of the show commands. Am I missing where, the output of the show commands is now in "number" format?

For example,
I don't think the output from show state should look how it currently does because of the default command text formatting:

[jseagrave21]

Should be "sets up" not "setups"

[ixje]

👍
I was mentally pretty tired at this point. It seems to always show first in language mistakes that I should know 😓 .

[jseagrave21]

Hey no worries! I only wish I could be of more help. It seems like you made a big push the other day!

[jseagrave21]

As a separate feedback, while playing with this new branch I found this error appears when trying to show help for a command with no params such as show state help:

neo> show state help

Show the status of the node

Usage: show state

Could not execute command: max() arg is an empty sequence
  File "/usr/lib/python3.7/threading.py", line 885, in _bootstrap
    self._bootstrap_inner()/2038
  File "/usr/lib/python3.7/threading.py", line 917, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.7/threading.py", line 865, in run
    self._target(*self._args, **self._kwargs)
  File "/mnt/c/users/jseag/neo/neo-python-ixje/venv/lib/python3.7/site-packages/twisted/_threads/_threadworker.py", line 46, in work
    task()
  File "/mnt/c/users/jseag/neo/neo-python-ixje/venv/lib/python3.7/site-packages/twisted/_threads/_team.py", line 190, in doWork
    task()
  File "/mnt/c/users/jseag/neo/neo-python-ixje/venv/lib/python3.7/site-packages/twisted/python/threadpool.py", line 250, in inContext
    result = inContext.theWork()
  File "/mnt/c/users/jseag/neo/neo-python-ixje/venv/lib/python3.7/site-packages/twisted/python/threadpool.py", line 266, in <lambda>
    inContext.theWork = lambda: context.call(ctx, func, *args, **kw)
  File "/mnt/c/users/jseag/neo/neo-python-ixje/venv/lib/python3.7/site-packages/twisted/python/context.py", line 122, in callWithContext
    return self.currentContext().callWithContext(ctx, func, *args, **kw)
  File "/mnt/c/users/jseag/neo/neo-python-ixje/venv/lib/python3.7/site-packages/twisted/python/context.py", line 85, in callWithContext
    return func(*args,**kw)
  File "/mnt/c/users/jseag/neo/neo-python-ixje/neo/bin/prompt.py", line 216, in run
    traceback.print_stack()
Traceback (most recent call last):
  File "/mnt/c/users/jseag/neo/neo-python-ixje/neo/bin/prompt.py", line 200, in run
    cmd.handle_help(arguments)
  File "/mnt/c/users/jseag/neo/neo-python-ixje/neo/Prompt/CommandBase.py", line 147, in handle_help
    self.__sub_commands[item].handle_help(arguments[1:])
  File "/mnt/c/users/jseag/neo/neo-python-ixje/neo/Prompt/CommandBase.py", line 143, in handle_help
    self.__print_absolute_cmd_help()
  File "/mnt/c/users/jseag/neo/neo-python-ixje/neo/Prompt/CommandBase.py", line 117, in __print_absolute_cmd_help
    longest_param_name = max(min_indent, max(len(p.name) for p in self.command_desc().params))
ValueError: max() arg is an empty sequence

so, I recommend this change in CommandBase.py:

replace

def __print_absolute_cmd_help(self):
        print(f"\n{self.command_desc().short_help.capitalize()}")
        params = ""
        for p in self.command_desc().params:
            params += f"{p.formatted_name()} "
        print(f"\nUsage: {self.__command_with_parents()} {params}\n")

        min_indent = 15
        longest_param_name = max(min_indent, max(len(p.name) for p in self.command_desc().params))
        for p in self.command_desc().params:
            print(p.to_str(longest_param_name))

with:

def __print_absolute_cmd_help(self):
        print(f"\n{self.command_desc().short_help.capitalize()}")
        params = ""
        for p in self.command_desc().params:
            params += f"{p.formatted_name()} "
        print(f"\nUsage: {self.__command_with_parents()} {params}\n")

        min_indent = 15
        try:
            longest_param_name = max(min_indent, max(len(p.name) for p in self.command_desc().params))
            for p in self.command_desc().params:
                print(p.to_str(longest_param_name))
        except ValueError:
            pass

this achieves the desired results with minimal changes.

[jseagrave21]

A final separate issue I noticed is that the wallet command does not have an adequate help description. I recommend changing command_desc in CommandWallet to:

def command_desc(self):
        return CommandDesc('wallet', 'show wallet details or manage wallets\n\n'
                           f"{' ':>20} Usage Examples:\n"
                           f"{' ':>23} wallet\n"
                           f"{' ':>23} wallet COMMAND\n")

this change illustrates that wallet is a standalone command (and in my opinion, it is probably the most used command).

[ixje]

I don't think the output from show state should look how it currently does because of the default command text formatting:

What would you have in mind it should look like?

My thoughts on the current approach were;
there are 2 types of information being printed on the console. 1) information triggered by us and information triggered in the background by networking code, VM execution etc. The former could be limited to cli command responses and are (currently) in red, the latter are in whatever colour the logger (debug,info,error) prints them. This gives a clear distinction for the user what they caused and what not.

I know it's a bit limited in the presentation, but I think that can be handled/improved with #624

[ixje]

A final separate issue I noticed is that the wallet command does not have an adequate help description. I recommend changing command_desc in CommandWallet to:

we can't do that because that will give

neo> help

Commands:
   config          - configure internal settings
   sc              - develop smart contracts
   search          - search the blockchain
   show            - show various node and blockchain data
   wallet          - show wallet details or manage wallets

                     Usage Examples:
                        wallet
                        wallet COMMAND

if we ever get a base group command with e.g. a z then you'd get something

neo> help

Commands:
   config          - configure internal settings
   sc              - develop smart contracts
   search          - search the blockchain
   show            - show various node and blockchain data
   wallet          - show wallet details or manage wallets

                     Usage Examples:
                        wallet
                        wallet COMMAND
    z-group        - my custom command

Not too pretty imo. The best I can come up with is

neo> wallet help

Usage: wallet COMMAND (or "wallet" to show the wallet contents)

This requires changing all double underscore methods/attributes in CommandBase to single underscore such that we can override handle_help() for CommandWallet. I'll ping Lysander on slack to see if he has ideas (can't tag him here)

[ixje]

addressed the show state help issue in a separate PR #792

[LysanderGG]

Reply to #788 (comment)

I can think of a few solutions:

1. have one (or several) subfunctions returning a string that handle_help would print, instead of directly printing.
   For this particular case, we could have:

def _usage_str(self):
    return f"Usage: {self.__command_with_parents()} COMMAND"

def handle_help(self, arguments):
        item = get_arg(arguments)
        if item == 'help':
            if len(self.__sub_commands) > 0:
                # show overview of subcommands and their purpose
                print(f"\n{self._usage_str()}\n")
                print(f"{self.command_desc().short_help.capitalize()}\n")
                ...

Then overriding _usage_str for CommandWallet should handle this problem.

2. We could restore the original idea of having a short help message and a longer help message.
   If the long help message is not defined, it would be the same as the short one.

The longer help could be used outside of listings.
For this particular problem, we would only need to provide a long help for CommandWallet.

3. Almost same as 2. but instead of adding a member to CommandDesc, we could define the following command that could be overwritten:

def longer_help(self):
    return self.command_desc().short_help

I don't think one is better than another for now 🤔
Maybe 1. is good enough for now and simple enough.

[jseagrave21]

What would you have in mind it should look like?

I was just referring back to the original presentation, where most outputs were in "number" format. In this case, I think the thinking was there were three types instead of two: input, ouput, and other. So all commands, seen when using help, would be red since they are commmands, but all outputs would be in "number" format, except for unrequested outputs like you mentioned (e.g. errors, etc.) .

I know it's a bit limited in the presentation, but I think that can be handled/improved with #624

Maybe it is better to table this discussion til then?