Skip to content

utils: Replace activity progress report messages with a live timer #1967

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

MicroFish91
Copy link
Contributor

@MicroFish91 MicroFish91 commented Apr 11, 2025

We had discussed adding a live activity timer and hiding progress reports from the activity log description. This would help accomplish that.

Corresponding Resource Groups PR: microsoft/vscode-azureresourcegroups#1119

(Note: Progress reports would still be used commands leveraging the Azure Wizard without registering an activity, see below.)

await vscode.window.withProgress(options, task);

@MicroFish91 MicroFish91 changed the title utils: Replace activity progress messages with a live timer utils: Replace activity progress report messages with a live timer Apr 11, 2025
@MicroFish91 MicroFish91 marked this pull request as ready for review April 15, 2025 01:58
@MicroFish91 MicroFish91 requested a review from a team as a code owner April 15, 2025 01:58
@@ -24,6 +24,12 @@ export class ExecuteActivity<TContext extends types.ExecuteActivityContext = typ
}
}

protected override report(progress?: { message?: string; increment?: number }): void {
const message: string | undefined = this.context.activityChildren?.length ? this.timerMessage : progress?.message;
this._onProgressEmitter.fire({ ...this.getState(), message });
Copy link
Contributor Author

@MicroFish91 MicroFish91 Apr 25, 2025

Choose a reason for hiding this comment

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

Made a new change here. I decided to override the report method in the concrete class. Two reasons for this:

  1. The concrete class is what actually holds the ExecuteActivityContext which I need
  2. The abstract class base implementation doesn't seem coupled to the Wizard, so I didn't want to pollute its implementation with Azure Wizard-y things.

The way this behaves:

  • If an activity has children, we just show a timer as the description. We offload the responsibility of showing what's being executed to the children
  • If no children, we still should indicate what's happening, so let's default back to any progress report messages in the description

I kept the timer on the abstract class level because its probably useful for any variation of concrete activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants