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

[consoleui] Make it easier to extend ConsolePrompt #1068

Merged
merged 2 commits into from
Sep 15, 2024

Conversation

quintesse
Copy link
Contributor

@quintesse quintesse commented Sep 10, 2024

This PR is meant as somewhat of a first step towards fixing #1051 .
When trying out different scenarios I was running into the issue that ConsolePrompt has all this useful code but it's somewhat hard to extend it because all its innards are private.
So the first commit is simply making those private fields and methods protected.

The second commit splits up the prompt() method. It's quite a big method that has both a loop and a whole bunch of of code to handle the different kinds of prompt elements that are supported. The fact that all that code is inside the loop makes it basically impossible to add any new element types or to try to handle any of them differently without basically copying and pasting the entire code to a new class.
So the second commit splits the prompt() method in two: the public prompt() still has the loop but the loop is now simplified to mostly calling a new protected method called promptElement(). This gives a subclass the possibility to override the promptElement() method to, for example, add support for a new custom element type.

They are not big changes but they make experimenting with the consoleui a whole lot easier and more powerful.

Wdyt @mattirn ?

Being able to subclass `ConsolePrompt` and have protected access to its
implementation would make it easier for devs to extends its functionality.
Copy link
Member

@gnodet gnodet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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

Successfully merging this pull request may close these issues.

3 participants