Skip to content

Commit

Permalink
Add jump list support for indirect icon references (#15221)
Browse files Browse the repository at this point in the history
Adds support to jump list generation for icon paths that include an
indirect reference e.g. `c:\windows\system32\shell32.dll,214`

If given a path that has an indirect icon reference parse the path into
component parts `filePath` and `iconIndex` and use
`IShellLinkW::SetIconLocation` to set the Icon for the entry. Otherwise
do what we always do.

This PR also introduces `til::to_int`, which is based on `til::to_ulong`
and supports signed integers.

## Validation Steps Performed
Icons were visible in the jump list and in terminal next to the
profiles.

Closes #15205
  • Loading branch information
jamespack authored Apr 26, 2023
1 parent ca5834e commit 4d5962e
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 9 deletions.
33 changes: 27 additions & 6 deletions src/cascadia/TerminalApp/Jumplist.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,19 +149,40 @@ winrt::com_ptr<IShellLinkW> Jumplist::_createShellLink(const std::wstring_view n
const auto module{ GetWtExePath() };
THROW_IF_FAILED(sh->SetPath(module.data()));
THROW_IF_FAILED(sh->SetArguments(args.data()));
auto propStore{ sh.as<IPropertyStore>() };

PROPVARIANT titleProp;
titleProp.vt = VT_LPWSTR;
titleProp.pwszVal = const_cast<wchar_t*>(name.data());

PROPVARIANT iconProp;
iconProp.vt = VT_LPWSTR;
iconProp.pwszVal = const_cast<wchar_t*>(path.data());
// Check for a comma in the path. If we find one we have an indirect icon. Parse the path into a file path and index/id.
auto commaPosition = path.find(L",");
if (commaPosition != std::wstring_view::npos)
{
const std::wstring iconPath{ path.substr(0, commaPosition) };

auto propStore{ sh.as<IPropertyStore>() };
THROW_IF_FAILED(propStore->SetValue(PKEY_Title, titleProp));
THROW_IF_FAILED(propStore->SetValue(PKEY_AppUserModel_DestListLogoUri, iconProp));
// We dont want the comma included so add 1 to its position
int iconIndex = til::to_int(path.substr(commaPosition + 1));
if (iconIndex != til::to_int_error)
{
THROW_IF_FAILED(sh->SetIconLocation(iconPath.data(), iconIndex));
}
}
else if (til::ends_with(path, L"exe") || til::ends_with(path, L"dll"))
{
// We have a binary path but no index/id. Default to 0
THROW_IF_FAILED(sh->SetIconLocation(path.data(), 0));
}
else
{
PROPVARIANT iconProp;
iconProp.vt = VT_LPWSTR;
iconProp.pwszVal = const_cast<wchar_t*>(path.data());

THROW_IF_FAILED(propStore->SetValue(PKEY_AppUserModel_DestListLogoUri, iconProp));
}

THROW_IF_FAILED(propStore->SetValue(PKEY_Title, titleProp));
THROW_IF_FAILED(propStore->Commit());

return sh;
Expand Down
6 changes: 3 additions & 3 deletions src/cascadia/TerminalSettingsModel/IconPathConverter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,9 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation

if (commaIndex != std::wstring::npos)
{
// Convert the string iconIndex to an int
const auto index = til::to_ulong(pathView.substr(commaIndex + 1));
if (index == til::to_ulong_error)
// Convert the string iconIndex to a signed int to support negative numbers which represent an Icon's ID.
const auto index{ til::to_int(pathView.substr(commaIndex + 1)) };
if (index == til::to_int_error)
{
return std::nullopt;
}
Expand Down
21 changes: 21 additions & 0 deletions src/inc/til/string.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
}

inline constexpr unsigned long to_ulong_error = ULONG_MAX;
inline constexpr int to_int_error = INT_MAX;

// Just like std::wcstoul, but without annoying locales and null-terminating strings.
// It has been fuzz-tested against clang's strtoul implementation.
Expand Down Expand Up @@ -373,4 +374,24 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
return compare_string_ordinal(lhs, rhs) == CSTR_LESS_THAN;
}
};

// Implement to_int in terms of to_ulong by negating its result. to_ulong does not expect
// to be passed signed numbers and will return an error accordingly. That error when
// compared against -1 evaluates to true. We account for that by returning to_int_error if to_ulong
// returns an error.
constexpr int to_int(const std::wstring_view& str, unsigned long base = 0) noexcept
{
auto result = to_ulong_error;
const auto signPosition = str.find(L"-");
const bool hasSign = signPosition != std::wstring_view::npos;
result = hasSign ? to_ulong(str.substr(signPosition + 1), base) : to_ulong(str, base);

// Check that result is valid and will fit in an int.
if (result == to_ulong_error || (result > INT_MAX))
{
return to_int_error;
}

return hasSign ? result * -1 : result;
}
}

0 comments on commit 4d5962e

Please sign in to comment.