Skip to content

Commit

Permalink
Warn when font isn't found and another is chosen (#8207)
Browse files Browse the repository at this point in the history
Display a warning message when the DirectX renderer resolves a font that
isn't the one you selected to warn that it couldn't be found.

Also I wrote the dialog event chain out of `TermControl` to be reusable
in the future for other messages the control might want to tell a host
about and various levels. 

## Validation Steps Performed
- Manual validation, setting bad font name, fixing font name with
  `settings.json`.

Closes #1017
  • Loading branch information
miniksa authored Nov 11, 2020
1 parent 3a57378 commit 4998779
Show file tree
Hide file tree
Showing 10 changed files with 199 additions and 58 deletions.
Original file line number Diff line number Diff line change
@@ -1 +1 @@
renamer
renamer
2 changes: 1 addition & 1 deletion .github/actions/spell-check/expect/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2829,4 +2829,4 @@ zy
AAAAABBBBBBCCC
AAAAA
BBBBBCCC
abcd
abcd
71 changes: 43 additions & 28 deletions src/cascadia/TerminalApp/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
<?xml version="1.0" encoding="utf-8"?>
<root>
<!--
Microsoft ResX Schema
<!--
Microsoft ResX Schema
Version 2.0
The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
associated with the data types.
Example:
... ado.net/XML headers & schema ...
<resheader name="resmimetype">text/microsoft-resx</resheader>
<resheader name="version">2.0</resheader>
Expand All @@ -26,36 +26,36 @@
<value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
<comment>This is a comment</comment>
</data>
There are any number of "resheader" rows that contain simple
There are any number of "resheader" rows that contain simple
name/value pairs.
Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
mimetype set.
The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
extensible. For a given mimetype the value must be set accordingly:
Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
read any of the formats listed below.
mimetype: application/x-microsoft.net.object.binary.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
: and then encoded with base64 encoding.
mimetype: application/x-microsoft.net.object.soap.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Soap.SoapFormatter
: and then encoded with base64 encoding.
mimetype: application/x-microsoft.net.object.bytearray.base64
value : The object must be serialized into a byte array
value : The object must be serialized into a byte array
: using a System.ComponentModel.TypeConverter
: and then encoded with base64 encoding.
-->
Expand Down Expand Up @@ -488,4 +488,19 @@
<data name="ParentCommandBackButton.[using:Windows.UI.Xaml.Controls]ToolTipService.ToolTip" xml:space="preserve">
<value>Back</value>
</data>
</root>
<data name="ControlNoticeDialog.PrimaryButtonText" xml:space="preserve">
<value>OK</value>
</data>
<data name="NoticeDebug" xml:space="preserve">
<value>Debug</value>
</data>
<data name="NoticeError" xml:space="preserve">
<value>Error</value>
</data>
<data name="NoticeInfo" xml:space="preserve">
<value>Information</value>
</data>
<data name="NoticeWarning" xml:space="preserve">
<value>Warning</value>
</data>
</root>
44 changes: 44 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1121,6 +1121,8 @@ namespace winrt::TerminalApp::implementation
// - hostingTab: The Tab that's hosting this TermControl instance
void TerminalPage::_RegisterTerminalEvents(TermControl term, TerminalTab& hostingTab)
{
term.RaiseNotice({ this, &TerminalPage::_ControlNoticeRaisedHandler });

// Add an event handler when the terminal's selection wants to be copied.
// When the text buffer data is retrieved, we'll copy the data into the Clipboard
term.CopyToClipboard({ this, &TerminalPage::_CopyToClipboardHandler });
Expand Down Expand Up @@ -1928,6 +1930,48 @@ namespace winrt::TerminalApp::implementation
}
}

void TerminalPage::_ControlNoticeRaisedHandler(const IInspectable /*sender*/, const Microsoft::Terminal::TerminalControl::NoticeEventArgs eventArgs)
{
winrt::hstring message = eventArgs.Message();

winrt::hstring title;

switch (eventArgs.Level())
{
case TerminalControl::NoticeLevel::Debug:
title = RS_(L"NoticeDebug"); //\xebe8
break;
case TerminalControl::NoticeLevel::Info:
title = RS_(L"NoticeInfo"); // \xe946
break;
case TerminalControl::NoticeLevel::Warning:
title = RS_(L"NoticeWarning"); //\xe7ba
break;
case TerminalControl::NoticeLevel::Error:
title = RS_(L"NoticeError"); //\xe783
break;
}

_ShowControlNoticeDialog(title, message);
}

void TerminalPage::_ShowControlNoticeDialog(const winrt::hstring& title, const winrt::hstring& message)
{
if (auto presenter{ _dialogPresenter.get() })
{
// FindName needs to be called first to actually load the xaml object
auto controlNoticeDialog = FindName(L"ControlNoticeDialog").try_as<WUX::Controls::ContentDialog>();

ControlNoticeDialog().Title(winrt::box_value(title));

// Insert the message
NoticeMessage().Text(message);

// Show the dialog
presenter.ShowDialog(controlNoticeDialog);
}
}

// Method Description:
// - Copy text from the focused terminal to the Windows Clipboard
// Arguments:
Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,9 @@ namespace winrt::TerminalApp::implementation

void _PasteText();

void _ControlNoticeRaisedHandler(const IInspectable sender, const Microsoft::Terminal::TerminalControl::NoticeEventArgs eventArgs);
void _ShowControlNoticeDialog(const winrt::hstring& title, const winrt::hstring& message);

fire_and_forget _LaunchSettings(const Microsoft::Terminal::Settings::Model::SettingsTarget target);

void _OnTabClick(const IInspectable& sender, const Windows::UI::Xaml::Input::PointerRoutedEventArgs& eventArgs);
Expand Down
10 changes: 10 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,16 @@ the MIT License. See LICENSE in the project root for license information. -->
DefaultButton="Primary">
</ContentDialog>

<ContentDialog
x:Load="False"
x:Name="ControlNoticeDialog"
x:Uid="ControlNoticeDialog"
DefaultButton="Primary">
<TextBlock IsTextSelectionEnabled="True" TextWrapping="WrapWholeWords">
<Run x:Name="NoticeMessage"/>
</TextBlock>
</ContentDialog>

<ContentDialog
x:Load="False"
x:Name="CouldNotOpenUriDialog"
Expand Down
64 changes: 36 additions & 28 deletions src/cascadia/TerminalControl/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
<?xml version="1.0" encoding="utf-8"?>
<root>
<!--
Microsoft ResX Schema
<!--
Microsoft ResX Schema
Version 2.0
The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
associated with the data types.
Example:
... ado.net/XML headers & schema ...
<resheader name="resmimetype">text/microsoft-resx</resheader>
<resheader name="version">2.0</resheader>
Expand All @@ -26,36 +26,36 @@
<value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
<comment>This is a comment</comment>
</data>
There are any number of "resheader" rows that contain simple
There are any number of "resheader" rows that contain simple
name/value pairs.
Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
mimetype set.
The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
extensible. For a given mimetype the value must be set accordingly:
Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
read any of the formats listed below.
mimetype: application/x-microsoft.net.object.binary.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
: and then encoded with base64 encoding.
mimetype: application/x-microsoft.net.object.soap.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Soap.SoapFormatter
: and then encoded with base64 encoding.
mimetype: application/x-microsoft.net.object.bytearray.base64
value : The object must be serialized into a byte array
value : The object must be serialized into a byte array
: using a System.ComponentModel.TypeConverter
: and then encoded with base64 encoding.
-->
Expand Down Expand Up @@ -178,4 +178,12 @@
<data name="HowToOpenRun.Text" xml:space="preserve">
<value>Ctrl+Click to follow link</value>
</data>
</root>
<data name="NoticeFontNotFound" xml:space="preserve">
<value>Unable to find the selected font "{0}".

"{1}" has been selected instead.

Please either install the missing font or choose another one.</value>
<comment>0 and 1 are names of fonts provided by the user and system respectively.</comment>
</data>
</root>
28 changes: 28 additions & 0 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1987,6 +1987,33 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
// actually fail. We need a way to gracefully fallback.
_renderer->TriggerFontChange(newDpi, _desiredFont, _actualFont);

// If the actual font isn't what was requested...
if (_actualFont.GetFaceName() != _desiredFont.GetFaceName())
{
// Then warn the user that we picked something because we couldn't find their font.

// Format message with user's choice of font and the font that was chosen instead.
const winrt::hstring message{ fmt::format(std::wstring_view{ RS_(L"NoticeFontNotFound") }, _desiredFont.GetFaceName(), _actualFont.GetFaceName()) };

// Capture what we need to resume later.
[strongThis = get_strong(), message]() -> winrt::fire_and_forget {
// Take these out of the lambda and store them locally
// because the coroutine will lose them into space
// by the time it resumes.
const auto msg = message;
const auto strong = strongThis;

// Pop the rest of this function to the tail of the UI thread
// Just in case someone was holding a lock when they called us and
// the handlers decide to do something that take another lock
// (like ShellExecute pumping our messaging thread...GH#7994)
co_await strong->Dispatcher();

auto noticeArgs = winrt::make<NoticeEventArgs>(NoticeLevel::Warning, std::move(msg));
strong->_raiseNoticeHandlers(*strong, std::move(noticeArgs));
}();
}

const auto actualNewSize = _actualFont.GetSize();
_fontSizeChangedHandlers(actualNewSize.X, actualNewSize.Y, initialUpdate);
}
Expand Down Expand Up @@ -3074,5 +3101,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TermControl, PasteFromClipboard, _clipboardPasteHandlers, TerminalControl::TermControl, TerminalControl::PasteFromClipboardEventArgs);
DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TermControl, CopyToClipboard, _clipboardCopyHandlers, TerminalControl::TermControl, TerminalControl::CopyToClipboardEventArgs);
DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TermControl, OpenHyperlink, _openHyperlinkHandlers, TerminalControl::TermControl, TerminalControl::OpenHyperlinkEventArgs);
DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TermControl, RaiseNotice, _raiseNoticeHandlers, TerminalControl::TermControl, TerminalControl::NoticeEventArgs);
// clang-format on
}
18 changes: 18 additions & 0 deletions src/cascadia/TerminalControl/TermControl.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "CopyToClipboardEventArgs.g.h"
#include "PasteFromClipboardEventArgs.g.h"
#include "OpenHyperlinkEventArgs.g.h"
#include "NoticeEventArgs.g.h"
#include <winrt/Microsoft.Terminal.TerminalConnection.h>
#include "../../renderer/base/Renderer.hpp"
#include "../../renderer/dx/DxRenderer.hpp"
Expand Down Expand Up @@ -81,6 +82,22 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
hstring _uri;
};

struct NoticeEventArgs :
public NoticeEventArgsT<NoticeEventArgs>
{
public:
NoticeEventArgs(const NoticeLevel level, const hstring& message) :
_level(level),
_message(message) {}

NoticeLevel Level() { return _level; };
hstring Message() { return _message; };

private:
const NoticeLevel _level;
const hstring _message;
};

struct TermControl : TermControlT<TermControl>
{
TermControl(IControlSettings settings, TerminalConnection::ITerminalConnection connection);
Expand Down Expand Up @@ -150,6 +167,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(PasteFromClipboard, _clipboardPasteHandlers, TerminalControl::TermControl, TerminalControl::PasteFromClipboardEventArgs);
DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(CopyToClipboard, _clipboardCopyHandlers, TerminalControl::TermControl, TerminalControl::CopyToClipboardEventArgs);
DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(OpenHyperlink, _openHyperlinkHandlers, TerminalControl::TermControl, TerminalControl::OpenHyperlinkEventArgs);
DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(RaiseNotice, _raiseNoticeHandlers, TerminalControl::TermControl, TerminalControl::NoticeEventArgs);

TYPED_EVENT(WarningBell, IInspectable, IInspectable);
TYPED_EVENT(ConnectionStateChanged, TerminalControl::TermControl, IInspectable);
Expand Down
Loading

0 comments on commit 4998779

Please sign in to comment.