AzCon: improve input, usability, reliability (4 commits) #4756
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DHowett-MSFT
added
the
Area-AzureShell
|
|
This commit replaces the AzureConnection's input handler with one that acts more like "getline()". Instead of the Read thread setting a state and WriteInput filling in the right member variable, the reader blocks on the user's input and receives it in an optional<string>. This moves the input number parsing and error case handling closer to the point where those inputs are used, as opposed to where they're collected. It also switches our input to be "line-based", which is a huge boon for typing tenant numbers >9. This fixes #3233. A simple line editor (supporting only backspace and CR) is included. It also enables echo on user input, and prints it in a nice pretty green color. It also enables input queueing: if the user types anything before the connection is established, it'll be sent once it is. Fixes #3233.
This commit colorizes parts of the AzCon's strings that include "user
options" -- things the user can type -- in yellow. This is to help with
accessibility.
The implementation here is based on a discussion with the team.
Alternative options for coloration were investigated, such as:
* Embedding escape sequences in the resource file.
This would have been confusing for translators.
The RESW file format doesn't support  escapes, so we would need
some magic post-processing.
* Embedding "markup" in the resource file (like #{93m}, ...)
This still would have been annoying for translators.
We settled on an implementation that takes resource names, colorizes
them, and string-formats them into other resources.
DHowett-MSFT
force-pushed
the
dev/duhowett/azure_2
branch
from
5bfff2b
to
daf32ee
Compare
carlos-zamora
approved these changes
Mar 2, 2020
|
|
||
| _currentInputMode = mode; | ||
|
|
||
| _TerminalOutputHandlers(L"\x1b[92m"); // Make prompted user input green |
There was a problem hiding this comment.
Should we make 92 a static constexpr int like the others above?
There was a problem hiding this comment.
It's either this or we allocate a dynamically generated string just to insert the 92 into it because it needs to be string-formatted.
I guess this whole string could be a const.
| return _currentInputMode != mode || _isStateAtOrBeyond(ConnectionState::Closing); | ||
| }); | ||
|
|
||
| _TerminalOutputHandlers(L"\x1b[m"); |
There was a problem hiding this comment.
what does this do? I don't speak VT too good yet 😋
There was a problem hiding this comment.
Lemme guess, stop coloring the output? Regardless, could you add a little comment here like you did above?
zadjii-msft
approved these changes
Mar 3, 2020
DHowett-MSFT
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request comprises four commits that improve the Azure connection.
Azure: rewrite user input handler
This commit replaces the AzureConnection's input handler with one that
acts more like "getline()". Instead of the Read thread setting a state
and WriteInput filling in the right member variable, the reader blocks
on the user's input and receives it in an optional.
This moves the input number parsing and error case handling closer to
the point where those inputs are used, as opposed to where they're
collected.
It also switches our input to be "line-based", which is a huge boon for
typing tenant numbers >9. This fixes #3233. A simple line editor
(supporting only backspace and CR) is included.
It also enables echo on user input, and prints it in a nice pretty green
color.
It also enables input queueing: if the user types anything before the
connection is established, it'll be sent once it is.
Fixes #3233.
Azure: display the user's options and additional information in color
This commit colorizes parts of the AzCon's strings that include "user
options" -- things the user can type -- in yellow. This is to help with
accessibility.
The implementation here is based on a discussion with the team.
Alternative options for coloration were investigated, such as:
This would have been confusing for translators.
The RESW file format doesn't support  escapes, so we would need
some magic post-processing.
This still would have been annoying for translators.
We settled on an implementation that takes resource names, colorizes
them, and string-formats them into other resources.
Azure: follow the user's shell choice from the online portal
Fixes #2266.
Azure: remove all credentials instead of just the first one
just a silly bug.