Remove spinner from all progress (allow auth prompts to use the terminal)#439
Merged
derrickstolee merged 4 commits intomicrosoft:mainfrom Sep 22, 2020
Merged
Conversation
Contributor
derrickstolee
left a comment
There was a problem hiding this comment.
We talked offline about some things. Just don't forget the Readme.md changes.
Ensure authentication prompts shown by Git and any configured credential helpers can write to the terminal/TTY and capture information. Explicitly pass the userInteractive=true argument down the call stack to the construction of the Git process, and move the setting of GIT_TERMINAL_PROMPT=0 into the if-not-user-interactive block.
Remove all progress spinners from all commands. Git may need to prompt for authentication or other prompts during a command invocation, and the spinner interferes with this.
After the clone completes, add a final success (or failure) message.
977d7cf to
6323364
Compare
derrickstolee
approved these changes
Sep 22, 2020
Contributor
derrickstolee
left a comment
There was a problem hiding this comment.
One small nit. Looks great!
| } | ||
| else | ||
| { | ||
| this.Output.WriteLine("Complete with errors."); |
Contributor
There was a problem hiding this comment.
Perhaps "Process finished with errors." ?
Member
Author
There was a problem hiding this comment.
Do we want to also change "Complete" to something else? I was just trying to be consistent in the "done/complete/finished" wording. Happy to go with whatever.
Contributor
There was a problem hiding this comment.
Your wording is fine. Ignore me and merge.
Contributor
There was a problem hiding this comment.
You're probably having a nice, quiet evening. I'll merge ;)
Member
Author
There was a problem hiding this comment.
Thanks! We can always change this at a later date if needs be.
chrisd8088
added a commit
to chrisd8088/scalar
that referenced
this pull request
Oct 11, 2020
The last callers of the IsConsoleOutputRedirectedToFile() methods in the ScalarPlatform classes were removed in commit 4183579 in PR microsoft#439. Therefore we can remove this method, which was never properly implemented on POSIX anyway, per microsoft/VFSForGit#1355.
chrisd8088
added a commit
to chrisd8088/scalar
that referenced
this pull request
Oct 11, 2020
The last callers of the IsConsoleOutputRedirectedToFile() methods in the ScalarPlatform classes were removed in commit 4183579 in PR microsoft#439. Therefore we can remove this method (which was never properly implemented on POSIX anyway per microsoft/VFSForGit#1355) as well as the native Windows system call wrappers and enums which were used only by the WindowPlatform implementation of this method.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Ensure authentication prompts shown by Git and any configured credential
helpers can write to the terminal/TTY and capture information.
We don't really need the fancy spinner to show "Authenticating...";
this is only cosmetic.
Also explicitly pass-down the userInteractive=true argument down the
call stack to the construction of the Git process, and move the setting
of GIT_TERMINAL_PROMPT=0 into the if-not-user-interactive block.