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

Add experimental feature for font list command #4886

Merged
merged 19 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
7 changes: 0 additions & 7 deletions src/AppInstallerCLICore/Commands/RootCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,13 +154,6 @@ namespace AppInstaller::CLI
keyDirectories.OutputLine({ Resource::LocString{ Resource::String::PortableRoot }, Runtime::GetPathTo(Runtime::PathName::PortablePackageMachineRoot, true).u8string() });
keyDirectories.OutputLine({ Resource::LocString{ Resource::String::PortableRoot86 }, Runtime::GetPathTo(Runtime::PathName::PortablePackageMachineRootX86, true).u8string() });
keyDirectories.OutputLine({ Resource::LocString{ Resource::String::InstallerDownloads }, Runtime::GetPathTo(Runtime::PathName::UserProfileDownloads, true).u8string() });

if (Settings::ExperimentalFeature::IsEnabled(Settings::ExperimentalFeature::Feature::Font))
{
keyDirectories.OutputLine({ Resource::LocString{ Resource::String::FontsInstallLocationUser }, Runtime::GetPathTo(Runtime::PathName::FontsUserInstallLocation, true).u8string() });
keyDirectories.OutputLine({ Resource::LocString{ Resource::String::FontsInstallLocationMachine }, Runtime::GetPathTo(Runtime::PathName::FontsMachineInstallLocation, true).u8string() });
}

keyDirectories.Complete();
context.Reporter.Info() << std::endl;
}
Expand Down
2 changes: 0 additions & 2 deletions src/AppInstallerCLICore/Resources.h
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,6 @@ namespace AppInstaller::CLI::Resource
WINGET_DEFINE_RESOURCE_STRINGID(FontFilePaths);
WINGET_DEFINE_RESOURCE_STRINGID(FontListCommandLongDescription);
WINGET_DEFINE_RESOURCE_STRINGID(FontListCommandShortDescription);
WINGET_DEFINE_RESOURCE_STRINGID(FontsInstallLocationUser);
WINGET_DEFINE_RESOURCE_STRINGID(FontsInstallLocationMachine);
WINGET_DEFINE_RESOURCE_STRINGID(FontVersion);
WINGET_DEFINE_RESOURCE_STRINGID(ForceArgumentDescription);
WINGET_DEFINE_RESOURCE_STRINGID(GatedVersionArgumentDescription);
Expand Down
16 changes: 4 additions & 12 deletions src/AppInstallerCLICore/Workflows/FontFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,11 @@ namespace AppInstaller::CLI::Workflow

void OutputInstalledFontFamiliesTable(Execution::Context& context, const std::vector<InstalledFontFamiliesTableLine>& lines)
{
Execution::TableOutput<4> table(context.Reporter, { Resource::String::FontFamily, Resource::String::FontFaces, Resource::String::FontFamily, Resource::String::FontFaces });
Execution::TableOutput<2> table(context.Reporter, { Resource::String::FontFamily, Resource::String::FontFaces });

for (size_t i = 0; i < lines.size(); i += 2)
for (auto line : lines)
{
// Displays 2 font families per line for better readability.
if ((i + 1) < lines.size())
{
table.OutputLine({ lines[i].FamilyName, std::to_string(lines[i].FaceCount), lines[i+1].FamilyName, std::to_string(lines[i+1].FaceCount) });
}
else
{
table.OutputLine({ lines[i].FamilyName, std::to_string(lines[i].FaceCount), {}, {} });
}
table.OutputLine({ line.FamilyName, std::to_string(line.FaceCount) });
}

table.Complete();
Expand Down Expand Up @@ -99,7 +91,7 @@ namespace AppInstaller::CLI::Workflow
InstalledFontFacesTableLine line(
familyName,
Utility::LocIndString(Utility::ToLower(Utility::ConvertToUTF8(fontFace.Name))),
Utility::LocIndString(Utility::ConvertToUTF8(fontFace.Version)),
Utility::LocIndString(fontFace.Version.ToString()),
filePath.u8string());

lines.push_back(std::move(line));
Expand Down
9 changes: 3 additions & 6 deletions src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw
Original file line number Diff line number Diff line change
Expand Up @@ -3133,6 +3133,9 @@ Please specify one of them using the --source option to proceed.</value>
<data name="WINGET_CONFIG_ERROR_PARAMETER_INTEGRITY_BOUNDARY" xml:space="preserve">
<value>Parameter cannot be passed across integrity boundary.</value>
</data>
<data name="APPINSTALLER_CLI_ERROR_INSTALLER_ZERO_BYTE_FILE" xml:space="preserve">
<value>Downloaded zero byte installer; ensure that your network connection is working properly.</value>
</data>
<data name="FontCommandShortDescription" xml:space="preserve">
<value>Manage fonts</value>
</data>
Expand Down Expand Up @@ -3165,12 +3168,6 @@ Please specify one of them using the --source option to proceed.</value>
<data name="FontListCommandLongDescription" xml:space="preserve">
<value>List all installed fonts, or full details of a specific font.</value>
</data>
<data name="FontsInstallLocationUser" xml:space="preserve">
<value>Fonts Install Location (User)</value>
</data>
<data name="FontsInstallLocationMachine" xml:space="preserve">
<value>Fonts Install Location (Machine)</value>
</data>
<data name="FontVersion" xml:space="preserve">
<value>Version</value>
</data>
Expand Down
27 changes: 27 additions & 0 deletions src/AppInstallerCLITests/Versions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -421,3 +421,30 @@ TEST_CASE("SemanticVersion", "[versions]")
REQUIRE(version.BuildMetadata() == Version("4.5.6"));
REQUIRE(version.PartAt(2).Other == "-beta+4.5.6");
}

TEST_CASE("OpenTypeFontVersion", "[versions]")
{
// Valid font version.
OpenTypeFontVersion version = OpenTypeFontVersion("Version 1.234");
REQUIRE(version.ToString() == "1.234");
REQUIRE(version.GetParts().size() == 2);
REQUIRE(version.PartAt(0).Integer == 1);
REQUIRE(version.PartAt(1).Integer == 234);

// Font version with additional metadata.
version = OpenTypeFontVersion("Version 9.876.54 ;2024");
REQUIRE(version.ToString() == "9.876");
REQUIRE(version.GetParts().size() == 2);
REQUIRE(version.PartAt(0).Integer == 9);
REQUIRE(version.PartAt(1).Integer == 876);

// Invalid version. Font version must have at least 2 parts.
REQUIRE_NOTHROW(version = OpenTypeFontVersion("1234567"));
REQUIRE(version.IsUnknown());
REQUIRE(version.ToString() == "Unknown");

// Major and minor parts must have digits.
REQUIRE_NOTHROW(version = OpenTypeFontVersion(" abc.def "));
REQUIRE(version.IsUnknown());
REQUIRE(version.ToString() == "Unknown");
}
62 changes: 29 additions & 33 deletions src/AppInstallerCommonCore/Fonts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ namespace AppInstaller::Fonts
{
namespace
{
std::vector<std::wstring> preferredLocales = AppInstaller::Locale::GetUserPreferredLanguagesUTF16();
Copy link
Member

Choose a reason for hiding this comment

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

A global static is not how we should be reducing the number of calls to GetUserPreferredLanguagesUTF16. Global statics bring along several issues, made even worse when they are not statically initialized.

A better approach would be to have the font enumeration functions actually be methods of a type. The caller then controls the lifetime of anything that the font enumeration caches, like a member field of locales.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created a struct called FontCatalog that hold all of these font enumeration methods as well as a private member field for the preferred locales.


std::vector<std::filesystem::path> GetFontFilePaths(const wil::com_ptr<IDWriteFontFace>& fontFace)
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
{
UINT32 fileCount = 0;
Expand All @@ -35,9 +37,11 @@ namespace AppInstaller::Fonts
THROW_IF_FAILED(localLoader->GetFilePathLengthFromKey(fontFileReferenceKey, fontFileReferenceKeySize, &pathLength));
pathLength += 1; // Account for the trailing null terminator during allocation.

wchar_t* path = new wchar_t[pathLength];
std::wstring path;
path.resize(pathLength);
THROW_IF_FAILED(localLoader->GetFilePathFromKey(fontFileReferenceKey, fontFileReferenceKeySize, &path[0], pathLength));
filePaths.push_back(std::move(std::wstring(path)));
path.resize(pathLength - 1); // Remove the null char.
filePaths.emplace_back(std::move(path));
}
}

Expand All @@ -46,13 +50,19 @@ namespace AppInstaller::Fonts

std::wstring GetLocalizedStringFromFont(const wil::com_ptr<IDWriteLocalizedStrings>& localizedStringCollection)
{
std::vector<std::string> locales = AppInstaller::Locale::GetUserPreferredLanguages();
std::wstring preferredLocale = Utility::ConvertToUTF16(!locales.empty() ? locales[0] : "en-US");
UINT32 index = 0;
BOOL exists = false;

UINT32 index;
BOOL exists;
// TODO: Aggregate available locales and find best alternative locale if preferred locale does not exist.
if (FAILED(localizedStringCollection->FindLocaleName(preferredLocale.c_str(), &index, &exists)) || !exists)
for (const auto& locale : preferredLocales)
{
if (FAILED(localizedStringCollection->FindLocaleName(locale.c_str(), &index, &exists)) || exists)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (FAILED(localizedStringCollection->FindLocaleName(locale.c_str(), &index, &exists)) || exists)
if (SUCCEEDED_LOG(localizedStringCollection->FindLocaleName(locale.c_str(), &index, &exists)) && exists)

Should we exit the search early if one of the lookups fails? I think we would better off logging and continue trying to find a different one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to suggested.

{
break;
}
}

// If the locale does not exist, resort to the default value at the 0 index.
if (!exists)
{
index = 0;
}
Expand All @@ -61,9 +71,11 @@ namespace AppInstaller::Fonts
THROW_IF_FAILED(localizedStringCollection->GetStringLength(index, &length));
length += 1; // Account for the trailing null terminator during allocation.

wchar_t* localizedString = new wchar_t[length];
THROW_IF_FAILED(localizedStringCollection->GetString(index, localizedString, length));
return std::wstring(localizedString);
std::wstring localizedString;
localizedString.resize(length);
THROW_IF_FAILED(localizedStringCollection->GetString(index, &localizedString[0], length));
localizedString.resize(length - 1); // Remove the null char.
return localizedString;
}

std::wstring GetFontFaceName(const wil::com_ptr<IDWriteFont>& font)
Expand All @@ -80,7 +92,7 @@ namespace AppInstaller::Fonts
return GetLocalizedStringFromFont(familyNames);
}

std::wstring GetFontFaceVersion(const wil::com_ptr<IDWriteFont>& font)
Utility::OpenTypeFontVersion GetFontFaceVersion(const wil::com_ptr<IDWriteFont>& font)
{
wil::com_ptr<IDWriteLocalizedStrings> fontVersion;
BOOL exists;
Expand All @@ -90,23 +102,9 @@ namespace AppInstaller::Fonts
return {};
}

std::string value = Utility::ConvertToUTF8(GetLocalizedStringFromFont(fontVersion));

// Version is returned in the format of ex: 'Version 2.137 ;2017'
// Extract out the parts between 'Version' and ';'
if (Utility::CaseInsensitiveContainsSubstring(value, "Version"))
{
Utility::FindAndReplace(value, "Version", "");
}

if (Utility::CaseInsensitiveContainsSubstring(value, ";"))
{
Utility::FindAndReplace(value, ";", "");
}

Utility::Trim(value);
std::vector<std::string> items = Utility::Split(value, ' ', true);
return Utility::ConvertToUTF16(items[0]);
std::string value = AppInstaller::Utility::ConvertToUTF8(GetLocalizedStringFromFont(fontVersion));
Utility::OpenTypeFontVersion openTypeFontVersion { value };
return openTypeFontVersion;
}

FontFamily GetFontFamilyByIndex(const wil::com_ptr<IDWriteFontCollection>& collection, UINT32 index)
Expand Down Expand Up @@ -155,12 +153,10 @@ namespace AppInstaller::Fonts
BOOL exists;
THROW_IF_FAILED(collection->FindFamilyName(familyName.value().c_str(), &index, &exists));

if (!exists)
if (exists)
{
return {};
installedFontFamilies.emplace_back(GetFontFamilyByIndex(collection, index));
}

installedFontFamilies.emplace_back(GetFontFamilyByIndex(collection, index));
}
else
{
Expand Down
12 changes: 12 additions & 0 deletions src/AppInstallerCommonCore/Locale.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,18 @@ namespace AppInstaller::Locale
return result;
}

std::vector<std::wstring> GetUserPreferredLanguagesUTF16()
{
std::vector<std::wstring> result;

for (const auto& lang : winrt::Windows::System::UserProfile::GlobalizationPreferences::Languages())
{
result.emplace_back(std::wstring(lang));
}

return result;
}

std::string LocaleIdToBcp47Tag(LCID localeId)
{
WCHAR localeName[MAX_LOCALE_SNAME_LEN] = {0};
Expand Down
4 changes: 2 additions & 2 deletions src/AppInstallerCommonCore/Public/winget/Fonts.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
#pragma once
#include <AppInstallerVersions.h>
#include <string>
#include <vector>

Expand All @@ -10,14 +11,13 @@ namespace AppInstaller::Fonts
{
std::wstring Name;
std::vector<std::filesystem::path> FilePaths;
std::wstring Version;
Utility::OpenTypeFontVersion Version;
};

struct FontFamily
{
std::wstring Name;
std::vector<FontFace> Faces;
std::wstring Version;
};

/// <summary>
Expand Down
5 changes: 4 additions & 1 deletion src/AppInstallerCommonCore/Public/winget/Locale.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ namespace AppInstaller::Locale
// Get the list of user Preferred Languages from settings. Returns an empty vector in rare cases of failure.
std::vector<std::string> GetUserPreferredLanguages();

// Get the list of user Preferred Languages from settings. Returns an empty vector in rare cases of failure.
std::vector<std::wstring> GetUserPreferredLanguagesUTF16();

// Get the bcp47 tag from a locale id. Returns empty string if conversion cannot be performed.
std::string LocaleIdToBcp47Tag(LCID localeId);
}
}
16 changes: 16 additions & 0 deletions src/AppInstallerSharedLib/Public/AppInstallerVersions.h
Original file line number Diff line number Diff line change
Expand Up @@ -281,4 +281,20 @@ namespace AppInstaller::Utility

// Checks if there are overlaps within the list of version ranges
bool HasOverlapInVersionRanges(const std::vector<VersionRange>& ranges);

// The OpenType font version.
// The format of this version type is 'Version 1.234 ;567'
// The only part that is of importance is the 'Major.Minor' parts.
// The 'Version' string is typically found at the beginning of the version string.
// Any value after a digit that is not a '.' represents some other meaning.
struct OpenTypeFontVersion : Version
{
OpenTypeFontVersion() = default;

OpenTypeFontVersion(std::string&& version);
OpenTypeFontVersion(const std::string& version) :
OpenTypeFontVersion(std::string(version)) {}

void Assign(std::string version, std::string_view splitChars = DefaultSplitChars) override;
};
}
60 changes: 60 additions & 0 deletions src/AppInstallerSharedLib/Versions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -661,4 +661,64 @@ namespace AppInstaller::Utility

return false;
}

OpenTypeFontVersion::OpenTypeFontVersion(std::string&& version)
{
Assign(std::move(version), DefaultSplitChars);
}

void OpenTypeFontVersion::Assign(std::string version, std::string_view splitChars)
{
// Open type version requires using the default split character
THROW_HR_IF(E_INVALIDARG, splitChars != DefaultSplitChars);

// Split on default split character.
std::vector<std::string> parts = Split(version, '.', true);

std::string majorString;
std::string minorString;

// Font version must have a "major.minor" part.
if (parts.size() >= 2)
{
// Find first digit and trim all preceding characters.
std::string firstPart = parts[0];
size_t majorStartIndex = firstPart.find_first_of(s_Digit_Characters);

if (majorStartIndex != std::string::npos)
{
firstPart.erase(0, majorStartIndex);
}

size_t majorEndIndex = firstPart.find_last_of(s_Digit_Characters);
majorString = firstPart.substr(0, majorEndIndex + 1);

// Parse and verify minor part.
std::string secondPart = parts[1];
size_t endPos = secondPart.find_first_not_of(s_Digit_Characters);

// If a non-digit character exists, trim off the remainder.
if (endPos != std::string::npos)
{
secondPart.erase(endPos, secondPart.length());
}

minorString = secondPart;
}

// Verify results.
if (!majorString.empty() && !minorString.empty())
{
m_parts.emplace_back(majorString);
m_parts.emplace_back(minorString);
m_version = Utility::Join(DefaultSplitChars, { majorString, minorString });

Trim();
}
else
{
m_version = s_Version_Part_Unknown;
m_parts.emplace_back(0, std::string{ s_Version_Part_Unknown });
}
}
}
Loading