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.

Sign up for GitHub

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

Jump to bottom

Detect likely Powerline fonts, add a special powerline preview #15365

Merged
merged 4 commits into from
Jun 9, 2023
Merged

Detect likely Powerline fonts, add a special powerline preview #15365

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fbf78ca
Detect Powerline fonts, and add a powerline preview?
DHowett May 16, 2023
1c89407
Update src/cascadia/TerminalSettingsEditor/PreviewConnection.cpp
DHowett May 16, 2023
e38c09e
Add powerline to spellbot?
DHowett May 30, 2023
3b7808c
Update src/cascadia/TerminalSettingsEditor/Profiles_Appearance.cpp
DHowett Jun 8, 2023
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
2 changes: 2 additions & 0 deletions .github/actions/spelling/allow/allow.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ ok'd
overlined
pipeline
postmodern
Powerline
powerline
ptys
qof
qps
Expand Down
40 changes: 23 additions & 17 deletions src/cascadia/TerminalSettingsEditor/Appearances.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,28 @@ using namespace winrt::Microsoft::Terminal::Settings::Model;

namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
bool Font::HasPowerlineCharacters()
{
if (!_hasPowerlineCharacters.has_value())
{
try
{
winrt::com_ptr<IDWriteFont> font;
THROW_IF_FAILED(_family->GetFont(0, font.put()));
BOOL exists{};
// We're actually checking for the "Extended" PowerLine glyph set.
// They're more fun.
THROW_IF_FAILED(font->HasCharacter(0xE0B6, &exists));
Comment on lines +32 to +34
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's worth noting that you aren't really checking for PowerLine glyphs here. This is a Private Use Area, and this particular range has been used for Cirth characters in the ConScript Unicode Registry for the past couple of decades. Admittedly there probably aren't a lot of terminal users with Cirth fonts, but it would be nice if there was a safer way to detect PowerLine fonts specifically. Could you not guesstimate that from the name or something?

Copy link
Member Author

@DHowett DHowett May 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Could you not guesstimate that from the name or something?

That was my first approach! Man, the code was a lot shorter then.

I profiled a bunch of fonts and found that there's:

  • fonts with "PL" in them
    • at the end
    • in the middle ("Xxx PL Mono")
      • ... which don't have PowerLine glyphs either ("IBM Plex Mono")
  • NerdFonts
    • with "NF" at the end or in the middle ("Hack NF")
    • with "Nerd Font" at the end or in the middle ("Xxx Nerd Font Propo")
    • with "NerdFont" in them somewhere
  • A hypothetical category of fonts that don't include a designator in the name, but do have somewhat-correct glyphs there

I know that it'll always be "best effort", and that powerline has coöpted part of the PUA that has already been used, but... it sucks all-up no matter how you slice it.

Copy link
Member

@zadjii-msft zadjii-msft May 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Admittedly there probably aren't a lot of terminal users with Cirth fonts

image

Copy link
Collaborator

@j4james j4james May 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

My thinking was that it wouldn't matter that much if you failed to detect some nerd fonts, because they'd just get the regular preview, which isn't the end of the world. Showing the nerd font preview to someone that's using another glyph set seems like a bigger deal.

That said, I've just been looking for a Cirth font to test with, and I couldn't find any monospace variants that could reasonably be used in a terminal (I'm sure I had one in the past, but I might be misremembering that). So feel free to leave it as is. I just wanted to make sure we at least considered other options here.

Edit: @zadjii-msft I just saw your Cirth screenshot now, but I'm assuming that's not a "standard" CSUR font - it's just remapping ASCII characters as Cirth glyphs. I suspect the monospace fonts I remember using in the past were probably doing the same thing.

Copy link
Member

@lhecker lhecker May 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I personally think the codepoint check is more robust, because that's how most terminals detect powerline glyphs anyways. VS Code for instance checks these codepoints to determine if it should increase the contrast of text or not (it won't change the colors for powerline glyphs). Basically, I believe that it's somewhat unlikely for a user to use a Cirth font and much more likely for them to use a powerline font.
Edit: And it will still work correctly for the real terminal after all. This only changes the behavior for the settings preview.

_hasPowerlineCharacters = (exists == TRUE);
}
catch (...)
{
_hasPowerlineCharacters = false;
}
}
return _hasPowerlineCharacters.value_or(false);
}

AppearanceViewModel::AppearanceViewModel(const Model::AppearanceConfig& appearance) :
_appearance{ appearance }
{
Expand Down Expand Up @@ -288,25 +310,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation

IInspectable Appearances::CurrentFontFace() const
{
// look for the current font in our shown list of fonts
const auto& appearanceVM{ Appearance() };
const auto appearanceFontFace{ appearanceVM.FontFace() };
const auto& currentFontList{ ShowAllFonts() ? ProfileViewModel::CompleteFontList() : ProfileViewModel::MonospaceFontList() };
IInspectable fallbackFont;
for (const auto& font : currentFontList)
{
if (font.LocalizedName() == appearanceFontFace)
{
return box_value(font);
}
else if (font.LocalizedName() == L"Cascadia Mono")
{
fallbackFont = box_value(font);
}
}

// we couldn't find the desired font, set to "Cascadia Mono" since that ships by default
return fallbackFont;
return box_value(ProfileViewModel::FindFontWithLocalizedName(appearanceFontFace));
}

void Appearances::FontFace_SelectionChanged(const IInspectable& /*sender*/, const SelectionChangedEventArgs& e)
Expand Down
12 changes: 10 additions & 2 deletions src/cascadia/TerminalSettingsEditor/Appearances.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,22 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
struct Font : FontT<Font>
{
public:
Font(std::wstring name, std::wstring localizedName) :
Font(std::wstring name, std::wstring localizedName, IDWriteFontFamily* family) :
_Name{ name },
_LocalizedName{ localizedName } {};
_LocalizedName{ localizedName }
{
_family.copy_from(family);
}

hstring ToString() { return _LocalizedName; }
bool HasPowerlineCharacters();

WINRT_PROPERTY(hstring, Name);
WINRT_PROPERTY(hstring, LocalizedName);

private:
winrt::com_ptr<IDWriteFontFamily> _family;
std::optional<bool> _hasPowerlineCharacters;
};

struct AppearanceViewModel : AppearanceViewModelT<AppearanceViewModel>, ViewModelHelper<AppearanceViewModel>
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsEditor/Appearances.idl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ namespace Microsoft.Terminal.Settings.Editor
{
String Name { get; };
String LocalizedName { get; };
Boolean HasPowerlineCharacters { get; };
}

runtimeclass AppearanceViewModel : Windows.UI.Xaml.Data.INotifyPropertyChanged
Expand Down
20 changes: 17 additions & 3 deletions src/cascadia/TerminalSettingsEditor/PreviewConnection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,20 @@
using namespace ::winrt::Microsoft::Terminal::TerminalConnection;
using namespace ::winrt::Windows::Foundation;

static constexpr std::wstring_view PromptTextPlain{ L"C:\\> " };
static constexpr std::wstring_view PromptTextPowerline{ L"\x1b[49;34m\xe0b6\x1b[1;97;44m C:\\ \x1b[m\x1b[46;34m\xe0b8\x1b[49;36m\xe0b8\x1b[m " };

// clang-format off
static constexpr std::wstring_view PreviewText{
L"\x001b"
L"c" // Hard Reset (RIS); on separate lines to avoid becoming 0x01BC
L"Windows Terminal\r\n"
L"C:\\> \x1b[93m" L"git\x1b[m diff \x1b[90m-w\x1b[m\r\n"
L"{0}\x1b[93m" L"git\x1b[m diff \x1b[90m-w\x1b[m\r\n"
L"\x1b[1m" L"diff --git a/win b/win\x1b[m\r\n"
L"\x1b[36m@@ -1 +1 @@\x1b[m\r\n"
L"\x1b[31m- Windows Console\x1b[m\r\n"
L"\x1b[32m+ Windows Terminal!\x1b[m\r\n"
L"C:\\> \x1b[93mWrite-Host \x1b[36m\"\xd83c\xdf2f!\"\x1b[1D\x1b[m"
L"{0}\x1b[93mWrite-Host \x1b[36m\"\xd83c\xdf2f!\"\x1b[1D\x1b[m"
};
// clang-format on

Expand All @@ -27,7 +32,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
void PreviewConnection::Start() noexcept
{
// Send the preview text
_TerminalOutputHandlers(PreviewText);
_TerminalOutputHandlers(fmt::format(PreviewText, _displayPowerlineGlyphs ? PromptTextPowerline : PromptTextPlain));
}

void PreviewConnection::Initialize(const Windows::Foundation::Collections::ValueSet& /*settings*/) noexcept
Expand All @@ -45,4 +50,13 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
void PreviewConnection::Close() noexcept
{
}

void PreviewConnection::DisplayPowerlineGlyphs(bool d) noexcept
{
if (_displayPowerlineGlyphs != d)
{
_displayPowerlineGlyphs = d;
Start();
}
}
}
5 changes: 5 additions & 0 deletions src/cascadia/TerminalSettingsEditor/PreviewConnection.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,14 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
void Resize(uint32_t rows, uint32_t columns) noexcept;
void Close() noexcept;

void DisplayPowerlineGlyphs(bool d) noexcept;

winrt::Microsoft::Terminal::TerminalConnection::ConnectionState State() const noexcept { return winrt::Microsoft::Terminal::TerminalConnection::ConnectionState::Connected; }

WINRT_CALLBACK(TerminalOutput, winrt::Microsoft::Terminal::TerminalConnection::TerminalOutputHandler);
TYPED_EVENT(StateChanged, winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection, IInspectable);

private:
bool _displayPowerlineGlyphs{ false };
};
}
41 changes: 34 additions & 7 deletions src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ using namespace winrt::Microsoft::Terminal::Settings::Model;

namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
static Editor::Font _FontObjectForDWriteFont(IDWriteFontFamily* family);

Windows::Foundation::Collections::IObservableVector<Editor::Font> ProfileViewModel::_MonospaceFontList{ nullptr };
Windows::Foundation::Collections::IObservableVector<Editor::Font> ProfileViewModel::_FontList{ nullptr };

Expand Down Expand Up @@ -118,12 +120,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
com_ptr<IDWriteFontFamily> fontFamily;
THROW_IF_FAILED(fontCollection->GetFontFamily(i, fontFamily.put()));

// get the font's localized names
com_ptr<IDWriteLocalizedStrings> localizedFamilyNames;
THROW_IF_FAILED(fontFamily->GetFamilyNames(localizedFamilyNames.put()));

// construct a font entry for tracking
if (const auto fontEntry{ _GetFont(localizedFamilyNames) })
if (const auto fontEntry{ _FontObjectForDWriteFont(fontFamily.get()) })
{
// check if the font is monospaced
try
Expand Down Expand Up @@ -159,7 +157,32 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
}
CATCH_LOG();

Editor::Font ProfileViewModel::_GetFont(com_ptr<IDWriteLocalizedStrings> localizedFamilyNames)
Editor::Font ProfileViewModel::FindFontWithLocalizedName(const winrt::hstring& name) noexcept
{
// look for the current font in our shown list of fonts
Editor::Font fallbackFont{ nullptr };
try
{
const auto& currentFontList{ CompleteFontList() };
for (const auto& font : currentFontList)
{
if (font.LocalizedName() == name)
{
return font;
}
else if (font.LocalizedName() == L"Cascadia Mono")
{
fallbackFont = font;
}
}
}
CATCH_LOG();

// we couldn't find the desired font, set to "Cascadia Mono" if we found that since it ships by default
return fallbackFont;
}

static Editor::Font _FontObjectForDWriteFont(IDWriteFontFamily* family)
{
// used for the font's name as an identifier (i.e. text block's font family property)
std::wstring nameID;
Expand All @@ -169,6 +192,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
std::wstring localizedName;
UINT32 localizedNameIndex;

// get the font's localized names
winrt::com_ptr<IDWriteLocalizedStrings> localizedFamilyNames;
THROW_IF_FAILED(family->GetFamilyNames(localizedFamilyNames.put()));

// use our current locale to find the localized name
auto exists{ FALSE };
HRESULT hr;
Expand Down Expand Up @@ -211,7 +238,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation

if (!nameID.empty() && !localizedName.empty())
{
return make<Font>(nameID, localizedName);
return make<Font>(nameID, localizedName, family);
}
return nullptr;
}
Expand Down
3 changes: 1 addition & 2 deletions src/cascadia/TerminalSettingsEditor/ProfileViewModel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
static void UpdateFontList() noexcept;
static Windows::Foundation::Collections::IObservableVector<Editor::Font> CompleteFontList() noexcept { return _FontList; };
static Windows::Foundation::Collections::IObservableVector<Editor::Font> MonospaceFontList() noexcept { return _MonospaceFontList; };
static Editor::Font FindFontWithLocalizedName(winrt::hstring const& name) noexcept;

ProfileViewModel(const Model::Profile& profile, const Model::CascadiaSettings& settings);
Model::TerminalSettings TermSettings() const;
Expand Down Expand Up @@ -123,8 +124,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
static Windows::Foundation::Collections::IObservableVector<Editor::Font> _MonospaceFontList;
static Windows::Foundation::Collections::IObservableVector<Editor::Font> _FontList;

static Editor::Font _GetFont(com_ptr<IDWriteLocalizedStrings> localizedFamilyNames);

Model::CascadiaSettings _appSettings;
Editor::AppearanceViewModel _unfocusedAppearanceViewModel;
};
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsEditor/ProfileViewModel.idl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ namespace Microsoft.Terminal.Settings.Editor
{
static Windows.Foundation.Collections.IObservableVector<Font> CompleteFontList { get; };
static Windows.Foundation.Collections.IObservableVector<Font> MonospaceFontList { get; };
static Font FindFontWithLocalizedName(String name);

Microsoft.Terminal.Settings.Model.TerminalSettings TermSettings { get; };

Expand Down
20 changes: 19 additions & 1 deletion src/cascadia/TerminalSettingsEditor/Profiles_Appearance.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
Profiles_Appearance::Profiles_Appearance()
{
InitializeComponent();
_previewConnection = winrt::make_self<PreviewConnection>();
}

void Profiles_Appearance::OnNavigatedTo(const NavigationEventArgs& e)
Expand All @@ -36,7 +37,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
if (!_previewControl)
{
const auto settings = _Profile.TermSettings();
_previewControl = Control::TermControl(settings, settings, make<PreviewConnection>());
_previewConnection->DisplayPowerlineGlyphs(_looksLikePowerlineFont());
_previewControl = Control::TermControl(settings, settings, *_previewConnection);
_previewControl.IsEnabled(false);
_previewControl.AllowFocusWhenDisabled(false);
_previewControl.DisplayCursorWhileBlurred(true);
Expand Down Expand Up @@ -68,9 +70,25 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
_Profile.DeleteUnfocusedAppearance();
}

bool Profiles_Appearance::_looksLikePowerlineFont() const
{
if (_Profile && _Profile.DefaultAppearance())
{
if (auto fontName = _Profile.DefaultAppearance().FontFace(); !fontName.empty())
{
if (auto font = ProfileViewModel::FindFontWithLocalizedName(fontName))
DHowett marked this conversation as resolved.
Show resolved Hide resolved
{
return font.HasPowerlineCharacters();
}
}
}
return false;
}

void Profiles_Appearance::_onProfilePropertyChanged(const IInspectable&, const PropertyChangedEventArgs&) const
{
const auto settings = _Profile.TermSettings();
_previewConnection->DisplayPowerlineGlyphs(_looksLikePowerlineFont());
_previewControl.UpdateControlSettings(settings, settings);
}
}
3 changes: 3 additions & 0 deletions src/cascadia/TerminalSettingsEditor/Profiles_Appearance.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "Profiles_Appearance.g.h"
#include "Utils.h"
#include "PreviewConnection.h"

namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
Expand All @@ -26,7 +27,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation

private:
void _onProfilePropertyChanged(const IInspectable&, const PropertyChangedEventArgs&) const;
bool _looksLikePowerlineFont() const;

winrt::com_ptr<PreviewConnection> _previewConnection{ nullptr };
Microsoft::Terminal::Control::TermControl _previewControl{ nullptr };
Windows::UI::Xaml::Data::INotifyPropertyChanged::PropertyChanged_revoker _ViewModelChangedRevoker;
Windows::UI::Xaml::Data::INotifyPropertyChanged::PropertyChanged_revoker _AppearanceViewModelChangedRevoker;
Expand Down