Repository: microsoft/terminal (language: cpp)

Issue:
Terminal profile icon not shown in Jump List when icon is from an EXE
I provided an EXE for the icon of two of my terminal profiles. It shows correctly in the in-app drop down and in the tabs. However the icon is not displayed in the TaskBar JumpList. See attached image.


![image](https://user-images.githubusercontent.com/18356694/232858917-980b265e-2739-46a5-bc93-311863dc7266.png)


_From MSFT:44188999_


Discussion / hints:
Is this one up for grabs? I think I found a possible solution.

<img width="231" alt="image" src="https://user-images.githubusercontent.com/2086722/233224376-a0234704-637a-499a-8580-adb2401580fb.png">

@jamespack oh heck yeah!
Is this the best place to post some code and ask questions? Or is there a better mechanism?

Fix patch:
diff --git a/src/cascadia/TerminalApp/Jumplist.cpp b/src/cascadia/TerminalApp/Jumplist.cpp
index 4665a35b80d..050fdbb4291 100644
--- a/src/cascadia/TerminalApp/Jumplist.cpp
+++ b/src/cascadia/TerminalApp/Jumplist.cpp
@@ -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;
diff --git a/src/cascadia/TerminalSettingsModel/IconPathConverter.cpp b/src/cascadia/TerminalSettingsModel/IconPathConverter.cpp
index c2ae1577cda..4b2239a9ad7 100644
--- a/src/cascadia/TerminalSettingsModel/IconPathConverter.cpp
+++ b/src/cascadia/TerminalSettingsModel/IconPathConverter.cpp
@@ -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;
             }
diff --git a/src/inc/til/string.h b/src/inc/til/string.h
index fb9bd3e566d..c32de1e0531 100644
--- a/src/inc/til/string.h
+++ b/src/inc/til/string.h
@@ -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.
@@ -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;
+    }
 }


Test patch: