Add an action for identifying windows

src/cascadia/Remoting/Monarch.cpp

@@ -574,35 +574,61 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
     }
 
     // Method Description:
-    // - This is an event handler for the IdentifyWindowsRequested event. A
-    //   Peasant may raise that event if they want _all_ windows to identify
-    //   themselves.
-    // - This will tell each and every peasant to identify themselves. This will
-    //   eventually propagate down to TerminalPage::IdentifyWindow.
+    // - Helper for doing something on each and every peasant, with no regard
+    //   for if the peasant is living or dead.
+    // - We'll try calling callback on every peasant.
+    // - If any single peasant is dead, then we'll call errorCallback, and move on.
+    // - We're taking an errorCallback here, because the thing we usually want
+    //   to do is TraceLog a message, but TraceLoggingWrite is actually a macro
+    //   that _requires_ the second arg to be a string literal. It can't just be
+    //   a variable.
     // Arguments:
-    // - <unused>
+    // - callback: The function to call on each peasant
+    // - errorCallback: The function to call if a peasant is dead.
     // Return Value:
     // - <none>
-    void Monarch::_identifyWindows(const winrt::Windows::Foundation::IInspectable& /*sender*/,
-                                   const winrt::Windows::Foundation::IInspectable& /*args*/)
+    void Monarch::_forAllPeasantsIgnoringTheDead(std::function<void(const Remoting::IPeasant&, const uint64_t)> callback,
+                                                 std::function<void(const uint64_t)> errorCallback)
     {
-        // Notify all the peasants to display their ID.
         for (const auto& [id, p] : _peasants)
         {
             try
             {
-                p.DisplayWindowId();
+                callback(p, id);
             }
             catch (...)
             {
                 LOG_CAUGHT_EXCEPTION();
                 // If this fails, we don't _really_ care. Just move on to the
                 // next one. Someone else will clean up the dead peasant.
-                TraceLoggingWrite(g_hRemotingProvider,
-                                  "Monarch_identifyWindows_Failed",
-                                  TraceLoggingInt64(id, "peasantID", "The ID of the peasant which we could not identify"),
-                                  TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
+                errorCallback(id);
             }
         }
     }
+
+    // Method Description:
+    // - This is an event handler for the IdentifyWindowsRequested event. A
+    //   Peasant may raise that event if they want _all_ windows to identify
+    //   themselves.
+    // - This will tell each and every peasant to identify themselves. This will
+    //   eventually propagate down to TerminalPage::IdentifyWindow.
+    // Arguments:
+    // - <unused>
+    // Return Value:
+    // - <none>
+    void Monarch::_identifyWindows(const winrt::Windows::Foundation::IInspectable& /*sender*/,
+                                   const winrt::Windows::Foundation::IInspectable& /*args*/)
+    {
+        // Notify all the peasants to display their ID.
+        auto callback = [](auto&& p, auto&& /*id*/) {
+            p.DisplayWindowId();
+        };
+        auto onError = [](auto&& id) {
+            TraceLoggingWrite(g_hRemotingProvider,
+                              "Monarch_identifyWindows_Failed",
+                              TraceLoggingInt64(id, "peasantID", "The ID of the peasant which we could not identify"),
+                              TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
+        };
+        _forAllPeasantsIgnoringTheDead(callback, onError);
+    }
 }

src/cascadia/Remoting/Monarch.h

@@ -77,6 +77,8 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
         void _identifyWindows(const winrt::Windows::Foundation::IInspectable& sender,
                               const winrt::Windows::Foundation::IInspectable& args);
 
+        void _forAllPeasantsIgnoringTheDead(std::function<void(const winrt::Microsoft::Terminal::Remoting::IPeasant&, const uint64_t)> callback,
+                                            std::function<void(const uint64_t)> errorCallback);
         friend class RemotingUnitTests::RemotingTests;
     };
 }

src/cascadia/TerminalApp/AppLogic.cpp

@@ -1399,8 +1399,4 @@ namespace winrt::TerminalApp::implementation
             _root->WindowId(id);
         }
     }
-    // -------------------------------- WinRT Events ---------------------------------
-    // Winrt events need a method for adding a callback to the event and removing the callback.
-    // These macros will define them both for you.
-    DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(AppLogic, RequestedThemeChanged, _requestedThemeChangedHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::UI::Xaml::ElementTheme);
 }

src/cascadia/TerminalApp/Resources/en-US/Resources.resw

@@ -588,9 +588,13 @@
   <data name="CommandPalette_MoreOptions.[using:Windows.UI.Xaml.Automation]AutomationProperties.HelpText" xml:space="preserve">
     <value>More options</value>
   </data>
-  <data name="WindowIdPrefix" xml:space="preserve">
-    <value>Window: {}</value>
-    <comment>{} will be replaced with a number identifying this window</comment>
+  <data name="WindowIdLabel" xml:space="preserve">
+    <value>Window</value>
+    <comment>This is displayed as a label for a number, like "Window: 10"</comment>
+  </data>
+  <data name="UnnamedWindowName" xml:space="preserve">
+    <value>unnamed window</value>
+    <comment>text used to identify when a window hasn't been assigned a name by the user</comment>
   </data>
   <data name="WindowMaximizeButtonToolTip" xml:space="preserve">
     <value>Maximize</value>

src/cascadia/TerminalApp/TerminalAppLib.vcxproj

@@ -78,10 +78,6 @@
     <ClInclude Include="MinMaxCloseControl.h">
       <DependentUpon>MinMaxCloseControl.xaml</DependentUpon>
     </ClInclude>
-    <ClInclude Include="WindowNameConverter.h">
-      <DependentUpon>TerminalPage.idl</DependentUpon>
-      <SubType>Code</SubType>
-    </ClInclude>
     <ClInclude Include="PaletteItemTemplateSelector.h">
       <DependentUpon>PaletteItemTemplateSelector.idl</DependentUpon>
       <SubType>Code</SubType>
@@ -157,10 +153,6 @@
     <ClCompile Include="MinMaxCloseControl.cpp">
       <DependentUpon>MinMaxCloseControl.xaml</DependentUpon>
     </ClCompile>
-    <ClCompile Include="WindowNameConverter.cpp">
-      <DependentUpon>TerminalPage.idl</DependentUpon>
-      <SubType>Code</SubType>
-    </ClCompile>
     <ClCompile Include="PaletteItemTemplateSelector.cpp">
       <DependentUpon>PaletteItemTemplateSelector.idl</DependentUpon>
       <SubType>Code</SubType>

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -274,17 +274,9 @@ namespace winrt::TerminalApp::implementation
 
         _isAlwaysOnTop = _settings.GlobalSettings().AlwaysOnTop();
 
-        // This lambda is a local because there will be more toasts in the
-        // future that will need the same logic.
-        auto safeRefocus = [weakThis{ get_weak() }](auto&&, auto&&) {
-            if (auto page{ weakThis.get() })
-            {
-                page->_FocusActiveControl(nullptr, nullptr);
-            }
-        };
-
-        _windowIdToast = std::make_unique<Toast>(WindowIdToast());
-        WindowIdToast().Closed(safeRefocus);
+        // DON'T set up Toasts/TeachingTips here. They should be loaded and
+        // initialized the first time they're opened, in whatever method opens
+        // them.
 
         // Setup mouse vanish attributes
         SystemParametersInfoW(SPI_GETMOUSEVANISH, 0, &_shouldMouseVanish, false);
@@ -3025,8 +3017,8 @@ namespace winrt::TerminalApp::implementation
         return {};
     }
 
-    void TerminalPage::_FocusActiveControl(const IInspectable& /*sender*/,
-                                           const RoutedEventArgs& /*eventArgs*/)
+    void TerminalPage::_FocusActiveControl(IInspectable /*sender*/,
+                                           IInspectable /*eventArgs*/)
     {
         // We don't want to set focus on the tab if fly-out is open as it will be closed
         // TODO GH#5400: consider checking we are not in the opening state, by hooking both Opening and Open events
@@ -3375,19 +3367,81 @@ namespace winrt::TerminalApp::implementation
         co_await winrt::resume_foreground(Dispatcher());
         if (auto page{ weakThis.get() })
         {
-            page->_windowIdToast->Open();
+            // If we haven't ever loaded the TeachingTip, then do so now and
+            // create the toast for it.
+            if (page->_windowIdToast == nullptr)
+            {
+                if (MUX::Controls::TeachingTip tip{ page->FindName(L"WindowIdToast").try_as<MUX::Controls::TeachingTip>() })
+                {
+                    page->_windowIdToast = std::make_shared<Toast>(tip);
+                    // Make sure to use the weak ref when setting up this
+                    // callback.
+                    tip.Closed({ page->get_weak(), &TerminalPage::_FocusActiveControl });
+                }
+            }
+
+            if (page->_windowIdToast != nullptr)
+            {
+                page->_windowIdToast->Open();
+            }
+        }
+    }
+
+    // WindowName is a otherwise generic WINRT_OBSERVABLE_PROPERTY, but it needs
+    // to raise a PropertyChanged for WindowNameForDisplay, instead of
+    // WindowName.
+    winrt::hstring TerminalPage::WindowName() const noexcept
+    {
+        return _WindowName;
+    }
+    void TerminalPage::WindowName(const winrt::hstring& value)
+    {
+        if (_WindowName != value)
+        {
+            _WindowName = value;
+            _PropertyChangedHandlers(*this, Windows::UI::Xaml::Data::PropertyChangedEventArgs{ L"WindowNameForDisplay" });
         }
     }
 
-    // -------------------------------- WinRT Events ---------------------------------
-    // Winrt events need a method for adding a callback to the event and removing the callback.
-    // These macros will define them both for you.
-    DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TerminalPage, TitleChanged, _titleChangeHandlers, winrt::Windows::Foundation::IInspectable, winrt::hstring);
-    DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TerminalPage, LastTabClosed, _lastTabClosedHandlers, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::LastTabClosedEventArgs);
-    DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TerminalPage, SetTitleBarContent, _setTitleBarContentHandlers, winrt::Windows::Foundation::IInspectable, UIElement);
-    DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TerminalPage, FocusModeChanged, _focusModeChangedHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
-    DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TerminalPage, FullscreenChanged, _fullscreenChangedHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
-    DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TerminalPage, AlwaysOnTopChanged, _alwaysOnTopChangedHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
-    DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TerminalPage, RaiseVisualBell, _raiseVisualBellHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
-    DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TerminalPage, SetTaskbarProgress, _setTaskbarProgressHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
+    // WindowId is a otherwise generic WINRT_OBSERVABLE_PROPERTY, but it needs
+    // to raise a PropertyChanged for WindowIdForDisplay, instead of
+    // WindowId.
+    uint64_t TerminalPage::WindowId() const noexcept
+    {
+        return _WindowId;
+    }
+    void TerminalPage::WindowId(const uint64_t& value)
+    {
+        if (_WindowId != value)
+        {
+            _WindowId = value;
+            _PropertyChangedHandlers(*this, Windows::UI::Xaml::Data::PropertyChangedEventArgs{ L"WindowIdForDisplay" });
+        }
+    }
+
+    // Method Description:
+    // - Returns a label like "Window: 1234" for the ID of this window
+    // Arguments:
+    // - <none>
+    // Return Value:
+    // - a string for displaying the name of the window.
+    winrt::hstring TerminalPage::WindowIdForDisplay() const noexcept
+    {
+        return winrt::hstring{ fmt::format(L"{}: {}",
+                                           std::wstring_view(RS_(L"WindowIdLabel")),
+                                           _WindowId) };
+    }
+
+    // Method Description:
+    // - Returns a label like "<unnamed window>" when the window has no name, or the name of the window.
+    // Arguments:
+    // - <none>
+    // Return Value:
+    // - a string for displaying the name of the window.
+    winrt::hstring TerminalPage::WindowNameForDisplay() const noexcept
+    {
+        return _WindowName.empty() ?
+                   winrt::hstring{ fmt::format(L"<{}>", RS_(L"UnnamedWindowName")) } :
+                   _WindowName;
+    }
 }

src/cascadia/TerminalApp/TerminalPage.h

@@ -86,21 +86,29 @@ namespace winrt::TerminalApp::implementation
                                                      const bool initial,
                                                      const winrt::hstring cwd = L"");
 
+        // Normally, WindowName and WindowId would be
+        // WINRT_OBSERVABLE_PROPERTY's, but we want them to raise
+        // WindowNameForDisplay and WindowIdForDisplay instead
+        winrt::hstring WindowName() const noexcept;
+        void WindowName(const winrt::hstring& value);
+        uint64_t WindowId() const noexcept;
+        void WindowId(const uint64_t& value);
+        winrt::hstring WindowIdForDisplay() const noexcept;
+        winrt::hstring WindowNameForDisplay() const noexcept;
+
         WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
-        WINRT_OBSERVABLE_PROPERTY(winrt::hstring, WindowName, _PropertyChangedHandlers);
-        WINRT_OBSERVABLE_PROPERTY(uint64_t, WindowId, _PropertyChangedHandlers);
 
         // -------------------------------- WinRT Events ---------------------------------
-        DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(TitleChanged, _titleChangeHandlers, winrt::Windows::Foundation::IInspectable, winrt::hstring);
-        DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(LastTabClosed, _lastTabClosedHandlers, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::LastTabClosedEventArgs);
-        DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(SetTitleBarContent, _setTitleBarContentHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::UI::Xaml::UIElement);
-        DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(FocusModeChanged, _focusModeChangedHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
-        DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(FullscreenChanged, _fullscreenChangedHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
-        DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(AlwaysOnTopChanged, _alwaysOnTopChangedHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
-        DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(RaiseVisualBell, _raiseVisualBellHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
-        DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(SetTaskbarProgress, _setTaskbarProgressHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
-        TYPED_EVENT(Initialized, winrt::Windows::Foundation::IInspectable, winrt::Windows::UI::Xaml::RoutedEventArgs);
-        TYPED_EVENT(IdentifyWindowsRequested, Windows::Foundation::IInspectable, Windows::Foundation::IInspectable);
+        TYPED_EVENT(TitleChanged, IInspectable, winrt::hstring);
+        TYPED_EVENT(LastTabClosed, IInspectable, winrt::TerminalApp::LastTabClosedEventArgs);
+        TYPED_EVENT(SetTitleBarContent, IInspectable, winrt::Windows::UI::Xaml::UIElement);
+        TYPED_EVENT(FocusModeChanged, IInspectable, IInspectable);
+        TYPED_EVENT(FullscreenChanged, IInspectable, IInspectable);
+        TYPED_EVENT(AlwaysOnTopChanged, IInspectable, IInspectable);
+        TYPED_EVENT(RaiseVisualBell, IInspectable, IInspectable);
+        TYPED_EVENT(SetTaskbarProgress, IInspectable, IInspectable);
+        TYPED_EVENT(Initialized, IInspectable, winrt::Windows::UI::Xaml::RoutedEventArgs);
+        TYPED_EVENT(IdentifyWindowsRequested, IInspectable, IInspectable);
 
     private:
         friend struct TerminalPageT<TerminalPage>; // for Xaml to bind events
@@ -130,6 +138,8 @@ namespace winrt::TerminalApp::implementation
         bool _isInFocusMode{ false };
         bool _isFullscreen{ false };
         bool _isAlwaysOnTop{ false };
+        winrt::hstring _WindowName{};
+        uint64_t _WindowId{ 0 };
 
         bool _rearranging;
         std::optional<int> _rearrangeFrom;
@@ -268,7 +278,7 @@ namespace winrt::TerminalApp::implementation
 
         void _CompleteInitialization();
 
-        void _FocusActiveControl(const IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& eventArgs);
+        void _FocusActiveControl(IInspectable sender, IInspectable eventArgs);
 
         void _UnZoomIfNeeded();
 

src/cascadia/TerminalApp/TerminalPage.idl

@@ -10,18 +10,6 @@ namespace TerminalApp
         Windows.Foundation.IAsyncOperation<Windows.UI.Xaml.Controls.ContentDialogResult> ShowDialog(Windows.UI.Xaml.Controls.ContentDialog dialog);
     };
 
-    runtimeclass WindowIdConverter : [default] Windows.UI.Xaml.Data.IValueConverter
-    {
-        WindowIdConverter();
-    };
-
-
-    runtimeclass WindowNameConverter : [default] Windows.UI.Xaml.Data.IValueConverter
-    {
-        WindowNameConverter();
-    };
-
-
     [default_interface] runtimeclass TerminalPage : Windows.UI.Xaml.Controls.Page, Windows.UI.Xaml.Data.INotifyPropertyChanged
     {
         TerminalPage();
@@ -37,6 +25,8 @@ namespace TerminalApp
         void IdentifyWindow();
         String WindowName;
         UInt64 WindowId;
+        String WindowNameForDisplay { get; };
+        String WindowIdForDisplay { get; };
 
         // We cannot use the default XAML APIs because we want to make sure
         // that there's only one application-global dialog visible at a time,

src/cascadia/TerminalApp/TerminalPage.xaml

@@ -10,13 +10,6 @@ the MIT License. See LICENSE in the project root for license information. -->
     xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
     mc:Ignorable="d">
 
-    <Page.Resources>
-        <ResourceDictionary>
-            <local:WindowIdConverter x:Key="WindowIdConverter"/>
-            <local:WindowNameConverter x:Key="WindowNameConverter"/>
-        </ResourceDictionary>
-    </Page.Resources>
-
     <Grid x:Name="Root" Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">
         <Grid.RowDefinitions>
             <RowDefinition Height="Auto" />
@@ -144,9 +137,10 @@ the MIT License. See LICENSE in the project root for license information. -->
         dismiss itself if the window is unfocused (In Xaml Islands). This is
         tracked by MUX#4382-->
         <mux:TeachingTip x:Name="WindowIdToast"
+                         x:Load="False"
                          IsLightDismissEnabled="True"
-                         Title="{x:Bind WindowId, Mode=OneWay, Converter={StaticResource WindowIdConverter}}"
-                         Subtitle="{x:Bind WindowName, Mode=OneWay, Converter={StaticResource WindowNameConverter}}">
+                         Title="{x:Bind WindowIdForDisplay}"
+                         Subtitle="{x:Bind WindowNameForDisplay}">
         </mux:TeachingTip>
     </Grid>
 </Page>