-
Notifications
You must be signed in to change notification settings - Fork 696
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
Reduce status bar spam, log only minimal message to the status bar #4366
Changes from all commits
184b87b
a5bc15f
bbe6365
ba65ff9
0d34fab
5fc5c3f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -213,13 +213,18 @@ private void HandleErrorsAndWarnings(ILogMessage logMessage) | |
|
||
private static bool ShouldReportProgress(ILogMessage logMessage) | ||
{ | ||
// Only show messages with VerbosityLevel.Normal. That is, info messages only. | ||
// Only show messages with VerbosityLevel.Minimal. | ||
// Do not show errors, warnings, verbose or debug messages on the progress dialog | ||
// Avoid showing indented messages, these are typically not useful for the progress dialog since | ||
// they are missing the context of the parent text above it | ||
return RestoreOperationProgressUI.Current != null | ||
&& GetMSBuildLevel(logMessage.Level) == MSBuildVerbosityLevel.Normal | ||
&& logMessage.Message.Length == logMessage.Message.TrimStart().Length; | ||
&& GetMSBuildLevel(logMessage.Level) == MSBuildVerbosityLevel.Minimal | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the |
||
&& !IsStringIndented(logMessage); | ||
|
||
static bool IsStringIndented(ILogMessage logMessage) | ||
{ | ||
return logMessage.Message.Length > 0 && char.IsWhiteSpace(logMessage.Message[0]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 Waaaaay better! |
||
} | ||
} | ||
|
||
/// <summary> | ||
|
@@ -468,71 +473,6 @@ private static LogLevel GetLogLevel(MSBuildVerbosityLevel level) | |
} | ||
} | ||
|
||
internal class WaitDialogProgress : RestoreOperationProgressUI | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unused class, just kept creating extra refs to some methods. |
||
{ | ||
private readonly ThreadedWaitDialogHelper.Session _session; | ||
private readonly JoinableTaskFactory _taskFactory; | ||
|
||
private WaitDialogProgress( | ||
ThreadedWaitDialogHelper.Session session, | ||
JoinableTaskFactory taskFactory) | ||
{ | ||
_session = session; | ||
_taskFactory = taskFactory; | ||
UserCancellationToken = _session.UserCancellationToken; | ||
} | ||
|
||
public static async Task<RestoreOperationProgressUI> StartAsync( | ||
IAsyncServiceProvider asyncServiceProvider, | ||
JoinableTaskFactory jtf, | ||
CancellationToken token) | ||
{ | ||
token.ThrowIfCancellationRequested(); | ||
|
||
await jtf.SwitchToMainThreadAsync(token); | ||
|
||
var waitDialogFactory = await asyncServiceProvider.GetServiceAsync< | ||
SVsThreadedWaitDialogFactory, IVsThreadedWaitDialogFactory>(); | ||
|
||
var session = waitDialogFactory.StartWaitDialog( | ||
waitCaption: Resources.DialogTitle, | ||
initialProgress: new ThreadedWaitDialogProgressData( | ||
Resources.RestoringPackages, | ||
progressText: string.Empty, | ||
statusBarText: string.Empty, | ||
isCancelable: true, | ||
currentStep: 0, | ||
totalSteps: 0)); | ||
|
||
return new WaitDialogProgress(session, jtf); | ||
} | ||
|
||
public async override ValueTask DisposeAsync() | ||
{ | ||
await _taskFactory.SwitchToMainThreadAsync(); | ||
_session.Dispose(); | ||
} | ||
|
||
public override async Task ReportProgressAsync( | ||
string progressMessage, | ||
uint currentStep, | ||
uint totalSteps) | ||
{ | ||
await _taskFactory.SwitchToMainThreadAsync(); | ||
|
||
// When both currentStep and totalSteps are 0, we get a marquee on the dialog | ||
var progressData = new ThreadedWaitDialogProgressData( | ||
progressMessage, | ||
progressText: string.Empty, | ||
statusBarText: string.Empty, | ||
isCancelable: true, | ||
currentStep: (int)currentStep, | ||
totalSteps: (int)totalSteps); | ||
|
||
_session.Progress.Report(progressData); | ||
} | ||
} | ||
|
||
internal class StatusBarProgress : RestoreOperationProgressUI | ||
{ | ||
private static object Icon = (short)Constants.SBAI_General; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -259,7 +259,7 @@ public static async Task<RestoreSummary> CommitAsync(RestoreResultPair restoreRe | |
var log = summaryRequest.Request.Log; | ||
|
||
// Commit the result | ||
log.LogInformation(Strings.Log_Committing); | ||
jeffkl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
log.LogVerbose(Strings.Log_Committing); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I seriously considered removing this. It literally just says "Comitting restore" which is not extremely helpful but it is a progress indicator. For now, I just moved it verbose, but we can totally change it if others strongly lean one way or another. |
||
await result.CommitAsync(log, token); | ||
|
||
if (result.Success) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this logger is a testing utility that's only built under debug, so changes here don't affect the product.