[PR #14870] [MERGED] Correctness: Remove JsonUtils and IconConverter's 2-phase lookup bugs #30283

Open
opened 2026-01-31 09:39:48 +00:00 by claunia · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/microsoft/terminal/pull/14870
Author: @DHowett
Created: 2/17/2023
Status: Merged
Merged: 2/18/2023
Merged by: @DHowett

Base: mainHead: dev/duhowett/2023-compiler/2-twophase


📝 Commits (2)

  • 6c449d7 Correctness: Remove JsonUtils and IconConverter's 2-phase lookup bugs
  • 493e471 Yup, I figured this would bite me

📊 Changes

6 files changed (+83 additions, -87 deletions)

View changed files

📝 src/cascadia/LocalTests_SettingsModel/SettingsModel.LocalTests.vcxproj (+0 -2)
📝 src/cascadia/TerminalSettingsModel/IconPathConverter.cpp (+13 -13)
📝 src/cascadia/TerminalSettingsModel/JsonUtils.h (+68 -68)
📝 src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj (+0 -2)
📝 src/cascadia/WinRTUtils/inc/Utils.h (+2 -0)
📝 src/cascadia/ut_app/TerminalApp.UnitTests.vcxproj (+0 -2)

📄 Description

Our templates were declared in the wrong order in JsonUtils, so all we needed to do was reorder them. The tests bear this out.

This allows us to disable two-phase template name lookup. I also fixed a minor issue that resulted in the inclusion of too many copies of expandIconPath.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/microsoft/terminal/pull/14870 **Author:** [@DHowett](https://github.com/DHowett) **Created:** 2/17/2023 **Status:** ✅ Merged **Merged:** 2/18/2023 **Merged by:** [@DHowett](https://github.com/DHowett) **Base:** `main` ← **Head:** `dev/duhowett/2023-compiler/2-twophase` --- ### 📝 Commits (2) - [`6c449d7`](https://github.com/microsoft/terminal/commit/6c449d78156c3bb19404832cd379e9e7607d5162) Correctness: Remove JsonUtils and IconConverter's 2-phase lookup bugs - [`493e471`](https://github.com/microsoft/terminal/commit/493e471bdcf1a8bc1af493953f33a21836f449ef) Yup, I figured this would bite me ### 📊 Changes **6 files changed** (+83 additions, -87 deletions) <details> <summary>View changed files</summary> 📝 `src/cascadia/LocalTests_SettingsModel/SettingsModel.LocalTests.vcxproj` (+0 -2) 📝 `src/cascadia/TerminalSettingsModel/IconPathConverter.cpp` (+13 -13) 📝 `src/cascadia/TerminalSettingsModel/JsonUtils.h` (+68 -68) 📝 `src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj` (+0 -2) 📝 `src/cascadia/WinRTUtils/inc/Utils.h` (+2 -0) 📝 `src/cascadia/ut_app/TerminalApp.UnitTests.vcxproj` (+0 -2) </details> ### 📄 Description Our templates were declared in the wrong order in JsonUtils, so all we needed to do was reorder them. The tests bear this out. This allows us to disable two-phase template name lookup. I also fixed a minor issue that resulted in the inclusion of too many copies of expandIconPath. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
claunia added the pull-request label 2026-01-31 09:39:48 +00:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#30283