The Wayback Machine - https://web.archive.org/web/20210810204032/https://github.com/microsoft/terminal/pull/10755
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.

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

Combine progress states in the tab, taskbar #10755

Merged
merged 10 commits into from Aug 10, 2021
Merged

Conversation

@zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented Jul 22, 2021

Summary of the Pull Request

background-progress-000

This PR causes the Terminal to combine taskbar states at the tab and window level, according to the MSDN docs for SetProgressState.

This allows the Terminal's taskbar icon to continue showing progress information, even if you're in a pane/tab that doesn't have progress state. This is helpful for cases where the user may be running a build in one tab, and working on something else in another.

References

PR Checklist

  • Closes #10090
  • I work here
  • Tests added/passed
  • [n/a] Requires documentation to be updated

Detailed Description of the Pull Request / Additional comments

This also fixes a related bug where transitioning from the "error" or "warning" state directly to the "indeterminate" state would cause the taskbar icon to get stuck in a bad state.

Validation Steps Performed

progress.cmd
@echo off
setlocal enabledelayedexpansion

set _type=3
if (%1) == () (
    set _type=3
) else (
    set _type=%1
)



if (%_type%) == (0) (
    <NUL set /p =�]9;4�
    echo Cleared progress
)
if (%_type%) == (1) (
    <NUL set /p =�]9;4;1;25�
    echo Started progress (normal, 25^)
)
if (%_type%) == (2) (
    <NUL set /p =�]9;4;2;50�
    echo Started progress (error, 50^)
)
if (%_type%) == (3) (
    @rem start indeterminate progress in the taskbar
    @rem this `<NUL set /p =` magic will output the text _without a newline_

    <NUL set /p =�]9;4;3�
    echo Started progress (indeterminate, {omitted})
)
if (%_type%) == (4) (
    <NUL set /p =�]9;4;4;75�
    echo Started progress (warning, 75^)
)
@github-actions

This comment has been hidden.

Copy link

@github-actions github-actions bot commented on 2703447 Jul 22, 2021

@check-spelling-bot Report

Unrecognized words, please review:

  • precentage
Previously acknowledged words that are now absent SPACEBAR Unregister
To accept these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands

... in a clone of the git@github.com:microsoft/terminal.git repository
on the dev/migrie/b/progress-bugs branch:

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spelling/expect/alphabet.txt
.github/actions/spelling/expect/expect.txt
.github/actions/spelling/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spelling/expect/2703447e1c10deabeb6eebde93ae1ba333dab959.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
}

comment_json=$(mktemp)
curl -L -s -S \
  --header "Content-Type: application/json" \
  "https://api.github.com/repos/microsoft/terminal/comments/53832867" > "$comment_json"
comment_body=$(mktemp)
jq -r .body < "$comment_json" > $comment_body
rm $comment_json

patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")
  

patch_add=$(perl -e '$/=undef;
$_=<>;
s{<details>.*}{}s;
s{^#.*}{};
s{\n##.*}{};
s{(?:^|\n)\s*\*}{}g;
s{\s+}{ }g;
print' < "$comment_body")
  
update_files
rm $comment_body
git add -u
✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. You can copy the contents of each perl command excluding the outer ' marks and dropping any '"/"' quotation mark pairs into a file and then run perl file.pl from the root of the repository to run the code. Alternatively, you can manually insert the items...

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spelling/allow/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spelling/allow/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spelling/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spelling/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

🗜️ If you see a bunch of garbage

If it relates to a ...

well-formed pattern

See if there's a pattern that would match it.

If not, try writing one and adding it to a patterns/{file}.txt.

Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

Note that patterns can't match multiline strings.

binary-ish string

Please add a file path to the excludes.txt file instead of just accepting the garbage.

File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

@github-actions

This comment has been hidden.

src/cascadia/TerminalApp/TaskbarState.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/TaskbarState.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/TerminalPage.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/TerminalTab.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@PankajBhojwani PankajBhojwani left a comment

Looks great!

@msftbot
Copy link
Contributor

@msftbot msftbot bot commented Aug 10, 2021

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.
@msftbot msftbot bot merged commit a14b6f8 into main Aug 10, 2021
9 checks passed
9 checks passed
@github-actions
Spell checking Spell checking
Details
@github-actions
Spell checking Spell checking
Details
@azure-pipelines
Terminal CI Build #0.0.2108.0911 succeeded
Details
@azure-pipelines
Terminal CI (Audit Mode Static Analysis Build x64) Audit Mode Static Analysis Build x64 succeeded
Details
@azure-pipelines
Terminal CI (Build x64 Build x64 Release) Build x64 Build x64 Release succeeded
Details
@azure-pipelines
Terminal CI (Build x86 Build x86 Release) Build x86 Build x86 Release succeeded
Details
@azure-pipelines
Terminal CI (Code Health Scripts Proper Code Formatting Check) Code Health Scripts Proper Code Formatting Check succeeded
Details
@msftbot
auto-merge.config.enforce No dynamic merge policies are applicable.
@microsoft-cla
license/cla All CLA requirements met.
Details
@msftbot msftbot bot deleted the dev/migrie/b/progress-bugs branch Aug 10, 2021
@@ -690,37 +709,39 @@ namespace winrt::TerminalApp::implementation
// - <none>
void TerminalTab::_UpdateProgressState()
{
if (const auto& activeControl{ GetActiveTerminalControl() })
const auto state{ GetCombinedTaskbarState() };

This comment has been minimized.

@DHowett

DHowett Aug 10, 2021
Member

gentle nit: we could compute the "max state" one time and store it, here, because this is the listener for all progress state changes (right?). That would make the call in TerminalPage free (it wouldn't need to min_element the group of winrt objects)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment