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

Warn when font isn't found and another is chosen #8207

Merged
merged 7 commits into from
Nov 11, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 47 additions & 32 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 @@ -223,12 +223,12 @@
<value>Warnings were found while parsing your keybindings:</value>
</data>
<data name="TooManyKeysForChord" xml:space="preserve">
<value>&#x2022; Found a keybinding with too many strings for the "keys" array. There should only be one string value in the "keys" array.</value>
<comment>{Locked="\"keys\"","&#x2022;"} This glyph is a bullet, used in a bulleted list.</comment>
<value> Found a keybinding with too many strings for the "keys" array. There should only be one string value in the "keys" array.</value>
<comment>{Locked="\"keys\"",""} This glyph is a bullet, used in a bulleted list.</comment>
</data>
<data name="MissingRequiredParameter" xml:space="preserve">
<value>&#x2022; Found a keybinding that was missing a required parameter value. This keybinding will be ignored.</value>
<comment>{Locked="&#x2022;"} This glyph is a bullet, used in a bulleted list.</comment>
<value> Found a keybinding that was missing a required parameter value. This keybinding will be ignored.</value>
<comment>{Locked=""} This glyph is a bullet, used in a bulleted list.</comment>
miniksa marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="LegacyGlobalsProperty" xml:space="preserve">
<value>The "globals" property is deprecated - your settings might need updating. </value>
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;
Comment on lines +1941 to +1952
Copy link
Member

Choose a reason for hiding this comment

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

What are all of these //\xebe8 for?

Copy link
Member Author

Choose a reason for hiding this comment

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

I had this bright idea that I was going to use Segoe icons for a bug shaped icon for debug and an I in a circle info icon for informational. But it didn't work right because the text in the dialog won't font fallback. And in lieu of adding another text field with the font set just right to take only the Segoe icon.... I just left the codepoints here for if I cared enough in the future because it was mildly painful to look them up in the first place.

Copy link
Member

Choose a reason for hiding this comment

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

Yea les keep em

}

_ShowControlNoticeDialog(title, message);
}

void TerminalPage::_ShowControlNoticeDialog(winrt::hstring title, winrt::hstring message)
miniksa marked this conversation as resolved.
Show resolved Hide resolved
{
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>();
Comment on lines +1962 to +1963
Copy link
Member

Choose a reason for hiding this comment

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

Wait...Really? So you never actually use controlNoticeDialog? I guess might as well not set it to the variable (or call Title off of controlNoticeDialog below).

Copy link
Member Author

Choose a reason for hiding this comment

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

Well.... the trickery is, as far as I know, that these things are lazy-instantiated. So I believe this is necessary to make XAML load the object before I can do the things below. So yeah, I don't use it. But I had to ask for it to make sure XAML did the needful first on the relevant other variables.


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

// Insert the reason and the URI
NoticeReason().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(winrt::hstring title, 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="NoticeReason"/>
miniksa marked this conversation as resolved.
Show resolved Hide resolved
</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>
24 changes: 23 additions & 1 deletion src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1979,14 +1979,35 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
// Arguments:
// - initialUpdate: whether this font update should be considered as being
// concerned with initialization process. Value forwarded to event handler.
void TermControl::_UpdateFont(const bool initialUpdate)
winrt::fire_and_forget TermControl::_UpdateFont(const bool initialUpdate)
{
const int newDpi = static_cast<int>(static_cast<double>(USER_DEFAULT_SCREEN_DPI) * SwapChainPanel().CompositionScaleX());

// TODO: MSFT:20895307 If the font doesn't exist, this doesn't
// 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.

// Save things we need to resume later.
auto strongThis{ get_strong() };

// 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()) };
miniksa marked this conversation as resolved.
Show resolved Hide resolved

// 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 Dispatcher();

auto noticeArgs = winrt::make_self<NoticeEventArgs>(NoticeLevel::Warning, message);
_raiseNoticeHandlers(*strongThis, *noticeArgs);
}

miniksa marked this conversation as resolved.
Show resolved Hide resolved
const auto actualNewSize = _actualFont.GetSize();
_fontSizeChangedHandlers(actualNewSize.X, actualNewSize.Y, initialUpdate);
}
Expand Down Expand Up @@ -3074,5 +3095,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
}
20 changes: 19 additions & 1 deletion 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(NoticeLevel level, hstring message) :
_level(level),
_message(message) {}
miniksa marked this conversation as resolved.
Show resolved Hide resolved

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 Expand Up @@ -241,7 +259,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
void _InitializeBackgroundBrush();
winrt::fire_and_forget _BackgroundColorChanged(const COLORREF color);
bool _InitializeTerminal();
void _UpdateFont(const bool initialUpdate = false);
winrt::fire_and_forget _UpdateFont(const bool initialUpdate = false);
void _SetFontSize(int fontSize);
void _TappedHandler(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::Input::TappedRoutedEventArgs const& e);
void _KeyDownHandler(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::Input::KeyRoutedEventArgs const& e);
Expand Down
Loading