Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd2 3.0.0 Discussion #1351

Open
kmvanbrunt opened this issue Oct 24, 2024 · 11 comments
Open

cmd2 3.0.0 Discussion #1351

kmvanbrunt opened this issue Oct 24, 2024 · 11 comments

Comments

@kmvanbrunt
Copy link
Member

kmvanbrunt commented Oct 24, 2024

Now that cmd2 2.5.0 is out, I'd like to start thinking about cmd2 3.0.0. So here are a few features I'd like to see based on feedback we've received over the years.

  • Full rich and rich-argparse integration.

    • I wrote the table and related formatting code in early 2020 to solve a problem that all existing table creators at the time had. None of them preserved text styles across multiple lines of a table cell. That was also right around the time rich was developing their table functionality so I wasn't aware of their library. In the years since, rich has become the de facto library for formatting text in the terminal and it's widely used and well supported. So I think it's safe at this point to offload all of our text formatting to rich. This approach was also discussed in Regarding KISS and direction of the project #1251 .

    • I have been testing our stuff with rich-argparse and I'm finding the switch very easy.

  • Make async alerts and prompt changes event driven.

    • Currently developers must obtain self.terminal_lock to asynchronously print an alert. In cmd2 3.0 they will call a function like add_alert() which adds to a queue and cmd2 will handle printing alerts when the prompt is on screen. Async prompt updates will provide a similar method.
  • Consider moving some built-in commands to CommandSets or mixin classes.

    • What commands are best suited for this?
  • Remove macros.

    • They were a neat idea at the time, but as 1350 points out, the lack of custom tab completion makes them difficult and confusing to use. Removing macros also eliminates the need for self._input_line_to_statement(). We could just call self._complete_statement() directly.
  • Modernize build system to be based on pyproject.toml #1334

    • Moving the ext_test plugin might help here as well. It currently has its own build system and is required to run our unit tests. Can it become a part of the main cmd2 code or at least not be needed to run our unit tests?
@kmvanbrunt kmvanbrunt added this to the 3.0.0 milestone Oct 24, 2024
@tleonhardt
Copy link
Member

tleonhardt commented Oct 24, 2024

I think replacing most of cmd2's custom text formatting code with a combination of rich and rich-argparse is a fantastic idea. It will reduce the amount of custom code that we need to maintain for cmd2 and allow us to bring in some additional functionality while presenting our users with the now idiomatic features available within rich.

I'm intrigued by the idea of making async alerts and prompt changes event driven, but I'd want to understand more about the architecture for how this would work and how it may interact or interfere with other event loops. For this sort of thing, I think we should consider integrating with Textual as this provides a path to true terminal-based UIs.

@tleonhardt
Copy link
Member

@anselor @kotfu Do either of you have any thoughts or opinions on any of the above?

@anselor
Copy link
Contributor

anselor commented Oct 25, 2024

I think I understand what @kmvanbrunt is getting at about alerts. As I recall, right now it's the caller's responsibility to figure out what state the prompt is in and either immediately print an alert or come up with a mechanism to queue up the alert for later display. Rearchitecting so that we consume the alert and determine in cmd2 when/how to display the alert. In the most basic case, if the terminal is busy executing a command, then the alert gets queued. If the terminal is at the prompt then we do our prompt redraw thing. This opens the door for more advanced alerting methods that could take advantage of something like textual to have a dedicated GUI-like way of displaying alerts.

I agree with moving the plugins back into cmd2 proper and integrate it with unit tests. It would make sense to expand our unit tests to handle cases with different combinations of commandsets / plugins loaded. The build system update will go a long way towards helping that.

Definitely agree with trying to move more commands into pre-canned CommandSets.

Definitely agree on rich. At this point it's quite mature.

@kotfu
Copy link
Member

kotfu commented Oct 26, 2024

I like the idea of migrating to rich and rich-argparse for text output, it's mature and widely used. I'm supportive of the other changes listed above too (async improvements, removing macros, etc).

Here's a couple other suggestions that are much bigger features which I'm happy to work on if there is enough enthusiasm for them.

  1. I expect it's a common use case for developers using cmd2 to add their own additional Settables. For many use cases, it also would be useful have built-in support for persisting those settings between invocations of your cmd2-based tools. I think this can be done in a way that:
  • is off by default
  • works on all platforms (*nix, windows, macos)
  • is easy for a developer to adopt if they just want cmd2 to take care of it (ie you don't have to worry about file format, location, reading or writing to/from the file, etc)
  • can be extended to store other configuration data in the same file as the settings (i.e. if you want to store a list of servers, or a list of SQL snippets to be used by your app, or whatever)
  • a developer can override/re-implement various parts of the configuration if they want more control, ie a different file format, or you want to parse some existing config file/format, or you need to check multiple files (i.e. one in /etc and one in ~/.config or something similar)
  1. Let's take rich to it's ultimate conclusion: built-in support for colored themes. I did this in my own cmd2-based app: https://tomcatmanager.readthedocs.io/en/stable/interactive/themes.html. Well, mostly did it. There are some outputs/errors generated by cmd2 that don't use poutput/perr, and therefore I couldn't "intercept" the output and apply the proper theme colors. I would implement this in such a way that:
  • it's off by default
  • a theme definition can be used to control both cmd2 output and the output of your cmd2-based app
  • allows cmd2 developers to override the theme file locations, names, etc,
  • allows cmd2 developers to maintain their own online theme gallery, where you just tell cmd2 the url of your gallery and then it has commands to browse, search, and install themes from the gallery

@anselor
Copy link
Contributor

anselor commented Oct 26, 2024

@kotfu

I think, going back to @kmvanbrunt's point on moving commands into CommandSets, we can move settables into a CommandSet. This opens the door for these types of behavior to be modular and swappable so if there's a fundamentally different approach to managing settables it's just a matter of swapping which implementation you want to use.

Agreed on the output functions. Over the course of a couple years I've been generalizing the output functions a bit. If we can focus a bit on that as well as change all of our internal code to use them, then that opens the door for mixins to override those for specific behaviors. Including possibly making it easier to embed inside of some sort of console UI (that could also be built with rich).

@kmvanbrunt
Copy link
Member Author

kmvanbrunt commented Oct 27, 2024

@kotfu I like both of your ideas.

I'm almost done integrating cmd2 with rich-argparse. Once I create the pull request, I'd love feedback from everyone to make sure it turned out well. After it gets merged, we'll need to integrate rich with the rest of the application.

These are the most obvious areas to update:

  1. output functions
  2. tables and text formatting
  3. tab completion output
    -This one will get interesting because some output is printed by readline/pyreadline3. I suppose we can use a rich Console to render the text and then send that text to readline.

Something like this for pyreadline3?

with console.capture() as capture:
    console.print(tab_completion_output)
    
readline.rl.mode.console.write(capture.get())

I prefer doing 2 and possibly 3 if that's OK with everyone.

@kotfu Since you've already done item 1 in tomcatmanager, would you like to handle this along with adding themes?

@kmvanbrunt
Copy link
Member Author

I have one more idea for cmd 3.0.0 and it's a large change.

There is a known race condition between a cmd2 async printing thread and GNU readline both drawing the prompt at the same time. Unfortunately, I can't add thread protection to GNU readline's code for obvious reasons. Additionally on Windows, pyreadline3 is buggy and it isn't being updated often.

cmd2 users have filed issues (#1315, #1319, #1331) for all of these problems.

Therefore, I propose switching from readline to prompt_toolkit. It seems to do everything we need and it's cross-platform. This solves a few issues.

  1. No more calling into the readline shared library on Linux and Mac.
  2. No more OS-specific tab completion code.
  3. Eliminate our async alert code because prompt_toolkit is capable of printing messages while a user is at the prompt.
  4. Eliminate our async prompt updating code since prompt_toolkit is capable of redrawing its prompt.

Switching to prompt_toolkit also means we will no longer inherit from cmd since it relies on readline.

What are everyone's thoughts on this?

@tleonhardt
Copy link
Member

tleonhardt commented Nov 1, 2024

The idea of basing cmd2 on prompt-toolkit and getting rid of all readline usage is intriguing. As long as we maintain full compatibility with cmd I don't see any downsides - i.e. we don't need to inherit or depend on cmd, but I think a user should still be able to replace from cmd import Cmd with from cmd2 import Cmd and have everything "just work".

I see benefits including:

  • No issues with readline not working well on some Windows or Mac setups
  • Advanced features built-in such as syntax highlighting, mouse support, etc.
  • The optional bottom status bar that can be used to persistently display status and/or alerts

However I do see this as likely a rather large and probably backwards incompatible breaking change. I think we might want to consider releasing some features in stages in minor releases prior to this. For example, the rich-argparse stuff we just merged in today seems like it isn't a major breaking change that could likely be released as 2.6.0.

I also think if we do go the prompt-toolkit route we probably want to start with a minimal usage where we are just using it as a drop-in readline replacement and not using all of the UI elements, at least to begin with.

@kmvanbrunt
Copy link
Member Author

I've started converting cmd2 to use rich for its output. @anselor, the print_to() method you added is making this a lot easier.

@tleonhardt
Copy link
Member

#1376 Removes support for Python 3.8 and makes 3.9 the minimum required version since 3.8 is now past EOL.

@tleonhardt
Copy link
Member

#1377 Makes use of type hinting generics added to collections so we no longer need to import things like List from typing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants