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

Add taskbar progress #2158

Merged
merged 8 commits into from
Oct 27, 2022
Merged

Add taskbar progress #2158

merged 8 commits into from
Oct 27, 2022

Conversation

timcassell
Copy link
Collaborator

@timcassell timcassell commented Oct 18, 2022

I initially tried to use a nuget package, but it does not support netstandard2.0. So I copied the relevant code from https://github.com/contre/Windows-API-Code-Pack-1.1 to Helpers/Taskbar to get this working (thanks @contre and @aybe).

Also thanks to ivan_pozdeev on StackOverflow for grabbing the proper console window handle.

image

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

The screen shot that you have provided looks very promising, but how about the license? Can we just copy paste the code?

@timcassell thank you for your contribution!

@timcassell
Copy link
Collaborator Author

@adamsitnik The license: https://github.com/contre/Windows-API-Code-Pack-1.1/blob/master/LICENSE

I was reading about it, apparently it was originally written by Microsoft a while ago and then they disappeared it, and other people picked up the code and uploaded to nuget (Source). So I don't think the license is an issue, but I'm no legal expert. ;)

Also, your change request makes sense, I'll do that.

@timcassell
Copy link
Collaborator Author

In the future we could even use this to alert warnings or errors if we want.

@Khaos66
Copy link

Khaos66 commented Oct 18, 2022

I just commented on #2102. You don't need another NuGet. This is a simple ComImport.
P.S. feel free to copy my code

@timcassell
Copy link
Collaborator Author

Thanks @Khaos66! I implemented your code, it's so much less than I had before!

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

The code looks good to me, but I've tested it and it does not work on my Windows 10 and 11 machines, both using cmd and Windows Terminal:

image

@timcassell
Copy link
Collaborator Author

@adamsitnik I pushed a change to get the console window's ancestor. Unfortunately, I only have a Win7 machine to test on, so could you let me know if that fixes it?

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

@timcassell I've given it a try and it does not work.

I've followed https://dzone.com/articles/using-windows-78-taskbar and created a small console app using https://www.nuget.org/packages/Microsoft-WindowsAPICodePack-Shell:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>netcoreapp3.1</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.WindowsAPICodePack-Shell" Version="1.1.0" />
  </ItemGroup>
</Project>
using Microsoft.WindowsAPICodePack.Taskbar;
using System.Threading;

namespace TaskbarProgress
{
    internal class Program
    {
        static void Main(string[] args)
        {
            if (TaskbarManager.IsPlatformSupported)
            {
                TaskbarManager tbManager = TaskbarManager.Instance;

                for (int i = 0; i < 100; i++)
                {
                    tbManager.SetProgressValue(i, 100);

                    Thread.Sleep(100);
                }
            }
        }
    }
}

it fails as well...

Unhandled exception. System.InvalidOperationException: A valid active Window is needed to update the Taskbar.
   at Microsoft.WindowsAPICodePack.Taskbar.TaskbarManager.get_OwnerHandle()
   at Microsoft.WindowsAPICodePack.Taskbar.TaskbarManager.SetProgressValue(Int32 currentValue, Int32 maximumValue)
   at TaskbarProgress.Program.Main(String[] args) in C:\Users\adsitnik\source\repos\TaskbarProgress\Program.cs:line 18

I am sorry but I don't have more time to spend on this.

@timcassell
Copy link
Collaborator Author

timcassell commented Oct 19, 2022

@adamsitnik I got the same error when I was testing initially, which was why I found it necessary to call the GetConsoleWindow(). This was the code I had working:

using Microsoft.WindowsAPICodePack.Taskbar;
using System;
using System.Runtime.InteropServices;
using System.Threading;

namespace TaskbarProgress
{
    internal class Program
    {
        [DllImport("kernel32.dll")]
        private static extern IntPtr GetConsoleWindow();

        static void Main(string[] args)
        {
            if (TaskbarManager.IsPlatformSupported)
            {
                IntPtr consoleWindowHandle = GetConsoleWindow();
                TaskbarManager tbManager = TaskbarManager.Instance;

                tbManager.SetProgressState(TaskbarProgressBarState.Normal, consoleWindowHandle);
                for (int i = 0; i < 100; i++)
                {
                    tbManager.SetProgressValue(i, 100, consoleWindowHandle);

                    Thread.Sleep(100);
                }
                tbManager.SetProgressState(TaskbarProgressBarState.NoProgress, consoleWindowHandle);
            }
        }
    }
}

I am sorry but I don't have more time to spend on this.

Understandable. I might be able to get my hands on a win 11 machine in a couple days.

@Khaos66
Copy link

Khaos66 commented Oct 19, 2022

Strange. I used this code on all of the windowses. Win 7, 8, 10, 11.
You need the right window handle however. I've not used it with a console app before. Sorry

@adamsitnik
Copy link
Member

You need the right window handle however.

That is correct. I saw that all the sys-calls are succeeding (returning 0), but we are most likely using the wrong window handle.

Exposed SetState and started with Indeterminate.
@timcassell
Copy link
Collaborator Author

I was able to see it working in both cmd and PowerShell on Win11, but it doesn't work with Terminal. Not sure why, I even tried walking the GetAncestor with Parent flag until I hit the Desktop window, but still no luck. We might be able to get it to work by pushing an actual Terminal command for it, but not sure if we want to do that (or if we can even detect that we're running in Terminal). microsoft/terminal#3004

image
image
image

Also, Win11 is weird, when I report 0 progress, it fills the whole bar instead of having an empty bar (Win7 doesn't do this). So I left the state as Indeterminate until the first benchmark is done.

@Khaos66
Copy link

Khaos66 commented Oct 21, 2022

Yes win11 does that so you know that the window is open. Because no bar unter the icon means the app is pinned to the taskbar but not running ;)

I guess that Windows Terminal uses sub-processes instead of embedded windows. It could just redirect all inputs/outputs for you.
You might want to check the parent process instead of the parent window?
Still, I'm not an expert here, it's just a guess

@timcassell

This comment was marked as outdated.

@timcassell
Copy link
Collaborator Author

timcassell commented Oct 27, 2022

Thanks to help from folks on the Windows Terminal repo, I was able to make it work. It now works for shells hosted in both old conhost and Windows Terminal.

@timcassell timcassell force-pushed the taskbar-progress branch 2 times, most recently from 44adf4c to 4b1e35d Compare October 27, 2022 00:29
Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

It looks and works great, thank you @timcassell !

You should consider writing a blog post about hot to get it working for Windows 7 + 11 since we were both unable to find any solution on the internet.

@adamsitnik adamsitnik merged commit c698954 into dotnet:master Oct 27, 2022
@adamsitnik adamsitnik added this to the v0.13.3 milestone Oct 27, 2022
@timcassell timcassell deleted the taskbar-progress branch October 27, 2022 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants