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.

Sign up for GitHub

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

Jump to bottom

Remove spinner from all progress (allow auth prompts to use the terminal) #439

Merged
merged 4 commits into from
Sep 22, 2020
Merged

Remove spinner from all progress (allow auth prompts to use the terminal) #439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4d57a8f
gitauth: ensure auth prompts can be shown over TTY
mjcheetham Sep 22, 2020
4183579
console: remove progress spinners from all commands
mjcheetham Sep 22, 2020
6323364
clone: add final "Complete" message after clone
mjcheetham Sep 22, 2020
6ac3f9e
Update readme with new output
mjcheetham Sep 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions Readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,14 @@ Clone parameters:
Local Cache: C:\.scalarCache
Destination: C:\_git\ForTests
FullClone: False
Authenticating...Succeeded
Querying remote for config...Succeeded
Authenticating...
Querying remote for config...
Using cache server: None (https://dev.azure.com/gvfs/ci/_git/ForTests)
Cloning...Succeeded
Fetching commits and trees from origin (no cache server)...Succeeded
Configuring Watchman...Succeeded.
Validating repo...Succeeded
Cloning...
Fetching commits and trees from origin (no cache server)...
Configuring Watchman...
Validating repo...
Complete!
```

Then, navigate into the repository's `src` directory.
Expand Down
71 changes: 6 additions & 65 deletions Scalar.Common/ConsoleHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@ public enum ActionResult
public static bool ShowStatusWhileRunning(
Func<bool> action,
string message,
TextWriter output,
bool showSpinner,
int initialDelayMs = 0)
TextWriter output)
{
Func<ActionResult> actionResultAction =
() =>
Expand All @@ -29,88 +27,31 @@ public static bool ShowStatusWhileRunning(
ActionResult result = ShowStatusWhileRunning(
actionResultAction,
message,
output,
showSpinner,
initialDelayMs: initialDelayMs);
output);

return result == ActionResult.Success;
}

public static ActionResult ShowStatusWhileRunning(
Func<ActionResult> action,
string message,
TextWriter output,
bool showSpinner,
int initialDelayMs)
TextWriter output)
{
ActionResult result = ActionResult.Failure;
bool initialMessageWritten = false;

try
{
if (!showSpinner)
{
output.Write(message + "...");
initialMessageWritten = true;
result = action();
}
else
{
ManualResetEvent actionIsDone = new ManualResetEvent(false);
bool isComplete = false;
Thread spinnerThread = new Thread(
() =>
{
int retries = 0;
char[] waiting = { '\u2014', '\\', '|', '/' };

while (!isComplete)
{
if (retries == 0)
{
actionIsDone.WaitOne(initialDelayMs);
}
else
{
output.Write("\r{0}...{1}", message, waiting[(retries / 2) % waiting.Length]);
initialMessageWritten = true;
actionIsDone.WaitOne(100);
}

retries++;
}

if (initialMessageWritten)
{
// Clear out any trailing waiting character
output.Write("\r{0}...", message);
}
});
spinnerThread.Start();

try
{
result = action();
}
finally
{
isComplete = true;

actionIsDone.Set();
spinnerThread.Join();
}
}
output.WriteLine(message + "...");
initialMessageWritten = true;
result = action();
}
finally
{
switch (result)
{
case ActionResult.Success:
if (initialMessageWritten)
{
output.WriteLine("Succeeded");
}

break;

case ActionResult.CompletedWithErrors:
Expand Down
11 changes: 7 additions & 4 deletions Scalar.Common/Git/GitProcess.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,8 @@ public virtual bool TryGetCredential(
Result gitCredentialOutput = this.InvokeGitAgainstDotGitFolder(
GenerateCredentialVerbCommand("fill"),
stdin => stdin.Write($"url={repoUrl}\n\n"),
parseStdOutLine: null);
parseStdOutLine: null,
userInteractive: true);

if (gitCredentialOutput.ExitCodeIsFailure)
{
Expand Down Expand Up @@ -614,11 +615,11 @@ private Process GetGitProcess(
}
}

processInfo.EnvironmentVariables["GIT_TERMINAL_PROMPT"] = "0";
processInfo.EnvironmentVariables["GCM_VALIDATE"] = "0";

if (!userInteractive)
{
processInfo.EnvironmentVariables["GIT_TERMINAL_PROMPT"] = "0";
processInfo.EnvironmentVariables["GCM_INTERACTIVE"] = "Never";
}

Expand Down Expand Up @@ -844,7 +845,8 @@ private Result InvokeGitAgainstDotGitFolder(
string command,
Action<StreamWriter> writeStdIn,
Action<string> parseStdOutLine,
string gitObjectsDirectory = null)
string gitObjectsDirectory = null,
bool userInteractive = true)
{
// This git command should not need/use the working directory of the repo.
// Run git.exe in Environment.SystemDirectory to ensure the git.exe process
Expand All @@ -857,7 +859,8 @@ private Result InvokeGitAgainstDotGitFolder(
writeStdIn: writeStdIn,
parseStdOutLine: parseStdOutLine,
timeoutMs: -1,
gitObjectsDirectory: gitObjectsDirectory);
gitObjectsDirectory: gitObjectsDirectory,
userInteractive: userInteractive);
}

public class Result
Expand Down
3 changes: 1 addition & 2 deletions Scalar.Upgrader/UpgradeOrchestrator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,7 @@ protected bool LaunchInsideSpinner(Func<bool> method, string message)
return ConsoleHelper.ShowStatusWhileRunning(
method,
message,
this.output,
this.output == Console.Out && !ScalarPlatform.Instance.IsConsoleOutputRedirectedToFile());
this.output);
}

private JsonTracer CreateTracer()
Expand Down
9 changes: 9 additions & 0 deletions Scalar/CommandLine/CloneVerb.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,15 @@ private Result DoClone(string fullEnlistmentRootPathParameter, string normalized
cloneResult = this.TryRegisterRepo();
}

if (cloneResult.Success)
{
this.Output.WriteLine("Complete!");
}
else
{
this.Output.WriteLine("Complete with errors.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps "Process finished with errors." ?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Your wording is fine. Ignore me and merge.

Copy link
Contributor

Choose a reason for hiding this comment

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

You're probably having a nice, quiet evening. I'll merge ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! We can always change this at a later date if needs be.

}

return cloneResult;
}

Expand Down
4 changes: 1 addition & 3 deletions Scalar/CommandLine/ScalarVerb.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,7 @@ protected bool ShowStatusWhileRunning(
return ConsoleHelper.ShowStatusWhileRunning(
action,
message,
this.Output,
showSpinner: !this.Unattended && this.Output == Console.Out && !ScalarPlatform.Instance.IsConsoleOutputRedirectedToFile(),
initialDelayMs: 0);
this.Output);
}

protected GitAuthentication.Result TryAuthenticate(ITracer tracer, ScalarEnlistment enlistment, out string authErrorMessage)
Expand Down