[v2] Initial groundwork for wizard improvements #5728
Conversation
Codecov Report
@@ Coverage Diff @@
## wizard-improvements #5728 +/- ##
=======================================================
- Coverage 93.47% 93.43% -0.05%
=======================================================
Files 254 258 +4
Lines 20184 20283 +99
=======================================================
+ Hits 18867 18951 +84
- Misses 1317 1332 +15
Continue to review full report at Codecov.
|
| if app_assertion: | ||
| failure_message_format = ( | ||
| f'Incorrect action on key press "{key}": ' | ||
| '{message}' |
There was a problem hiding this comment.
Need to add f on this iine as well
There was a problem hiding this comment.
Oh that omission was on purpose. Mainly the {message} is filled out later when the AssertionError is propagated.
|
|
||
| def _create_all_prompts(self, definition): | ||
| prompts = [] | ||
| for _, step_definition in definition['plan'].items(): |
There was a problem hiding this comment.
Probably using .values() would be more clear
| from prompt_toolkit.document import Document | ||
| from prompt_toolkit.key_binding import KeyBindings | ||
| from prompt_toolkit.layout.containers import ( | ||
| Window, VSplit, Dimension |
There was a problem hiding this comment.
[NIT] I believe it fits in one line
| self.container = self._get_answer_container() | ||
|
|
||
| def _get_answer_buffer(self): | ||
| return Buffer(name=f'{self._value_name}_answer_buffer') |
There was a problem hiding this comment.
What the reason of giving name to this Buffer if we never use this name?
I was wondering if it makes sense to name the buffer just f'{self._value_name} and make keybindings global instead of injecting them into each WizardPrompt
Something like
def submit_current_answer(event):
current_buffer = event.app.current_buffer.name
event.app.values[current_buffer.name] = .current_buffer.text
@kb.add('tab')
@kb.add('enter')
def next_prompt(event):
submit_current_answer(event)
event.app.layout.focus_next()
@kb.add('s-tab')
def last_prompt(event):
submit_current_answer(event)
event.app.layout.focus_last()There was a problem hiding this comment.
I think I originally wanted to use the name but it was something that got missed after trying a couple of different approaches. Honestly the key tabbing logic is going to be going away in the next PR, specifically I'm going to introduce an abstraction that walks though the wizard prompts telling us what the current prompt is and what the next prompt will be. I do like the idea of making the keybindings global now though so I'll introduce a keybindings module to encompass the current logic and build off of it.
| return VSplit( | ||
| [ | ||
| WizardPromptDescription( | ||
| answer_buffer, |
There was a problem hiding this comment.
I'm a bit concern about passing the reference to the buffer between containers, maybe it would be safer just to path its name
There was a problem hiding this comment.
Yeah I'm just going to switch this to use the value name.
New prompttoolkit abstractions will be added underneath this package
The layout currently just: * Displays all prompts no matter if they should not be shown * Allows you to type a answer to each prompt * Allows you to tab through and back prompts * Highlights prompt that has the current focus
This allows us to try out the new wizard format without having to touch the actual wizard commands. In order to run just type: aws cli-dev wizard-dev --run-wizard file://path/to/wizard.yml
This makes it easier to share with other features that will need it such as wizards
To do this, it required updating the application stubber with the following changes: * Added the ability to make assertions instead of only being able to rely on True/False. This makes the error messages a bit more verbose and easier to make more complex assertions * Added the ability to set the text of the current focused buffer. This allows for text to be inputted. There are still more tests needed to be added here but they should be added once the UI and wizard spec stabilizes.
It involved: * Updating test cases to use the updated stubber methods * Change all of the assertions to include some assertion in them * Make more helper methods to make it easier to update assertions in the future
Specifically: * Updated some styling on imports * Added a new keybindings module to represent keybindings for the entire application and moved keybindings from individual prompts to it * Reference buffers by value name to simplify required reference values
kyleknap
force-pushed
the
improved-wizard-init
branch
from
a281916
to
69046e4
Compare
[v2] Initial groundwork for wizard improvements
[v2] Initial groundwork for wizard improvements
This pull request adds the initial groundwork for wizard improvements. This change allows the prompts to be displayed as a basic form-based UI that you can type input into. To test it, I updated the
cli-dev wizard-devto start using this new scaffolding. So you can try it out by inputting:I tried to break this as best as I could into separate commits to accurately represent what changes I made but to summarize:
uipackage to host anything related to prompt toolkit UI.cli-dev wizard-devcommand to use this UIFuture work:
There is still a bit of work that is needed for the UI. In terms of immediate plans, I will next be adding: