Refactored bottom toolbar and rprompt functionality.#1700
Conversation
work the same way as get_rprompt(). Removed default implementation of get_bottom_toolbar() from Cmd class and moved it to the getting_started example.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1700 +/- ##
==========================================
- Coverage 99.61% 99.60% -0.01%
==========================================
Files 23 23
Lines 5909 5894 -15
==========================================
- Hits 5886 5871 -15
Misses 23 23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
tleonhardt
left a comment
tleonhardt
left a comment
There was a problem hiding this comment.
Other than the two very minor issues I called out, I don't have problems with the code changes per se.
However, I have a big problem with the new default behavior from a UX perspective. Before this change, when a bottom toolbar wasn't requested, there was no visual display of a bottom bar. Now, it displays an empty gray bar which is ugly, distracting, and pointless.
Please fix it so the default behavior is like it was previously.
Added enable_rprompt flag.
kmvanbrunt
changed the title
| - **disabled_commands**: commands that have been disabled from use. This is to support commands that are only available during specific states of the application. This dictionary's keys are the command names and its values are DisabledCommand objects. | ||
| - **echo**: if `True`, each command the user issues will be repeated to the screen before it is executed. This is particularly useful when running scripts. This behavior does not occur when running a command at the prompt. (Default: `False`) | ||
| - **editor**: text editor program to use with _edit_ command (e.g. `vim`) | ||
| - **enable_bottom_toolbar**: if `True`, enables a bottom toolbar while at the main prompt. (Default: `False`) |
There was a problem hiding this comment.
The newly added enable_bottom_toolbar and enable_rprompt (as well as refresh_interval below) are documented here as instance attributes, meaning they are instance members of the cmd2.Cmd instance that developers can override at runtime. However, in cmd2.py, they are strictly initializer arguments passed directly to PromptSession and are not stored on the Cmd instance. Attempting to override them at runtime will have no effect.
I think we should create a new section documenting __init__ parameters which are not also instance attributes and move them there.
This also applies to complete_in_thread.
Alternatively, we could restore them to instance attributes stored in Cmd.
There was a problem hiding this comment.
I think restoring them as instance attributes is confusing since changing them after __init__() runs has no effect. If a dev wants to alter prompt behavior later, they can call self._create_main_session() with different values for these options.
There was a problem hiding this comment.
I'm not sure what you mean by a new section in initialization.md. That file already contains a table documenting all __init__ parameters.
2 participants
Also closes #1699