Provide the focused tab title in the Tray Icon's context menu

[leonMSFT]

This PR adds a bit more information to each item in the Tray Icon's window selection submenu.
Currently it only shows the window ID and window name if given one.
Now each item will instead show{Window ID} : {Active Tab Title} [{Window Name}]

[leonMSFT]

I don't actually know what to call this - WindowTitle? FocusedTabTitle?

[zadjii-msft]

WindowTitle might be more appropriate, since users can also suppress the "use tab title as window title" behavior (and then the window's title is just Windows Terminal

[DHowett]

This one I actually prefer not to be the window title. Imagine, you'd have a list inside of a Terminal tray icon that just says "Windows Terminal, Windows Terminal, Windows Terminal, Windows Terminal, Windows Terminal". Then again, that's my bias. 😄

[zadjii-msft]

Yea that's a good point. Plus, I hadn't read to AppHost.cpp:311 yet when I made this comment

[zadjii-msft]

Alright so I don't love the idea of returning the Peasants out of the Monarch. I just worry that's a potential footgun if people aren't careful.

That being said, we need to return something with multiple values here. We may need to do something like

    struct PeasantInfo{
        UInt64 Id;
        String Name;
        String TabTitle;
    }
    ...
    [default_interface] runtimeclass Monarch {
        ...
        Windows.Foundation.Collections.IMapView<UInt64, PeasantInfo> GetPeasantInfo { get; };
        ...
    }

If we do it like that, then I think WinRT will marshal the entire PeasantInfo struct into the calling process, rather than pass a com_ptr to the object residing in (some other process).

HMU on teams if that sounds insane. Also @DHowett call me out if that's not how winrt works

[zadjii-msft]

WindowTitle might be more appropriate, since users can also suppress the "use tab title as window title" behavior (and then the window's title is just Windows Terminal

[zadjii-msft]

Ah okay. I see. You changed the title handling so that we always bubble the focused tab title up. So maybe it should be ActiveTabTitle, not WindowTitle

[zadjii-msft]

I dig it, thanks for bearing with me ☺️

[lhecker]

I believe the time is nigh to abstract this away, as it's been repeated 3x now in this file.
You can put something like this into the header:

template<typename F>
void _forEachPeasant(F&& func)
{
    using Map = decltype(_peasants);
    using R = std::invoke_result_t<F, Map::key_type, Map::mapped_type>;
    static constexpr auto IsVoid = std::is_void_v<R>;

    std::vector<uint64_t> peasantsToErase;

    for (const auto& [id, p] : _peasants)
    {
        try
        {
            if constexpr (IsVoid)
            {
                func(id, p);
            }
            else
            {
                if (!func(id, p))
                {
                    break;
                }
            }
        }
        catch (const winrt::hresult_error& exception)
        {
            if (exception.code() == 0x800706ba) // The RPC server is unavailable.
            {
                peasantsToErase.emplace_back(id);
            }
            else
            {
                LOG_CAUGHT_EXCEPTION();
                throw;
            }
        }
    }

    for (const auto& id : peasantsToErase)
    {
        _peasants.erase(id);
        _clearOldMruEntries(id);
    }
}

Then use it like that:

Windows::Foundation::Collections::IVectorView<Remoting::PeasantInfo> Monarch::GetPeasantInfos()
{
    std::vector<Remoting::PeasantInfo> names;
    names.reserve(_peasants.size());

    _forEachPeasant([&](const auto& id, const auto& p) {
        names.emplace_back(id, p.WindowName(), p.ActiveTabTitle());
    });

    return winrt::single_threaded_vector(std::move(names)).GetView();
}

bool Monarch::DoesQuakeWindowExist()
{
	  bool result = false;
	  _forEachPeasant([&](const auto& id, const auto& p) {
	      if (p.WindowName() == QuakeWindowName)
	      {
	          result = true;
	      }
	      // In other words:
	      // "continue if we didn't get a positive result"
	      return !result;
	  });
	  return result;
}

[lhecker]

I believe you can even replace all usages of _forAllPeasantsIgnoringTheDead with that template.
@zadjii-msft Idk if that's correct. May _forAllPeasantsIgnoringTheDead remove dead peasants?

[zadjii-msft]

May _forAllPeasantsIgnoringTheDead remove dead peasants?

Meh, I suppose it could. off the top of my head, I don't remember if there was a specific reason it doesn't. IIRC, for things that were using _forAllPeasantsIgnoringTheDead, it didn't really matter if a peasant had died. I just left the removal for something later on that actually did care if the peasant had died.

[lhecker]

I've updated my above template to contain:

catch (const winrt::hresult_error& exception)
{
    if (exception.code() == 0x800706ba) // The RPC server is unavailable.
    {
        peasantsToErase.emplace_back(id);
    }
    else
    {
        LOG_CAUGHT_EXCEPTION();
        throw;
    }
}

What do you think about that?

[leonMSFT]

If it will replace _forAllPeasantsIgnoringTheDead, it should probably also accept an onError function then?

[lhecker]

I think we can just replace _forAllPeasantsIgnoringTheDead a different time. I don't know how important the two TraceLoggingWrite calls are...
Although it would be preferable if _forAllPeasantsIgnoringTheDead was a template as well so that we don't have to use std::function (which prevents inlining).

[ghost]

Hello @leonMSFT!

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.

[lhecker]

BTW I realized after this PR was merged that you can also write this:

fmt::memory_buffer out;
fmt::format_to(out, L"#{}", p.Id);

if (!p.TabTitle.empty())
{
    fmt::format_to(out, L": {}", p.TabTitle);
}

if (!p.Name.empty())
{
    fmt::format_to(out, L" [{}]", p.Name);
}

fmt::format_to(out, L"\0");
AppendMenu(submenu, MF_STRING, gsl::narrow<UINT_PTR>(p.Id), out.data());

It doesn't even allocate anything on the heap!

[ghost]

🎉Windows Terminal Preview v1.12.2922.0 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Store Download