starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-02-04 05:35:20 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

[PR #3107] [MERGED] Allow FontInfo{,Base,Desired} to store a font name > 32 wch #25240

New Issue
Open
opened 2026-01-31 09:08:13 +00:00 by claunia · 0 comments
claunia commented 2026-01-31 09:08:13 +00:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/microsoft/terminal/pull/3107
Author: @DHowett
Created: 10/7/2019
Status: ✅ Merged
Merged: 10/15/2019
Merged by: @DHowett-MSFT

Base: master ← Head: dev/duhowett/font32

📝 Commits (6)

  • 26d3fcb Allow FontInfo{,Base,Desired} to store a font name > 32 wch
  • 8ab7a7c Code review feedback
  • 5f7d3b5 FORMAT C: /Q
  • fdfd826 Do more good and fun things
  • f4daa08 And make it a span
  • 83fcb6c Fix the crashing Selection test

📊 Changes

24 files changed (+154 additions, -158 deletions)

View changed files

📝 src/buffer/out/textBuffer.cpp (+2 -6)
📝 src/buffer/out/textBuffer.hpp (+1 -1)
📝 src/host/getset.cpp (+2 -2)
📝 src/host/output.cpp (+1 -1)
📝 src/host/renderFontDefaults.cpp (+8 -4)
📝 src/host/renderFontDefaults.hpp (+2 -3)
📝 src/host/selection.cpp (+1 -6)
📝 src/host/selection.hpp (+0 -2)
📝 src/host/settings.cpp (+7 -7)
📝 src/host/settings.hpp (+4 -4)
📝 src/host/srvinit.cpp (+2 -2)
📝 src/host/telemetry.cpp (+1 -1)
📝 src/interactivity/onecore/SystemConfigurationProvider.cpp (+3 -1)
📝 src/interactivity/onecore/SystemConfigurationProvider.hpp (+0 -4)
📝 src/interactivity/win32/menu.cpp (+3 -3)
📝 src/renderer/base/FontInfoBase.cpp (+52 -39)
📝 src/renderer/base/FontInfoDesired.cpp (+6 -6)
📝 src/renderer/base/fontinfo.cpp (+12 -22)
📝 src/renderer/dx/DxRenderer.cpp (+2 -4)
📝 src/renderer/gdi/state.cpp (+13 -7)

...and 4 more files

📄 Description

Summary of the Pull Request

We now truncate the font name as it goes out to GDI APIs, in console API
servicing, and in the propsheet.

I attempted to defer truncating the font to as far up the stack as possible, so as to make FontInfo usable for the broadest set of cases.

PR Checklist

Detailed Description of the Pull Request / Additional comments

There are a couple things I have questions about, actually. I know that Settings gets memset (memsat?) by the registry deserializer, and perhaps that's another place for us to tackle. Right now, this pull request enables fonts whose names are >= 32 characters in Windows Terminal only, but the underpinnings are there for conhost as well. We'd need to explicitly break at the API, or perhaps return a failure or log something to telemetry.

  • Should we log truncation at the API boundary to telemetry?
  • Should the "launch face name" get the same treatment as the face name in Settings?
  • Should we fix Settings here, or later?
  • TrueTypeFontList is built out of things in winconp, the private console header. That fills me with dread that those structures are used for interop -- so I didn't bubble these fixes all the way up through the "default font" list, even though nominally the font list is also unbounded strings.
  • Is unsigned int right for codepage? For width?

I left some comments about truncating font faces around, but they could be tantamount to noise.

🔄 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/3107 **Author:** [@DHowett](https://github.com/DHowett) **Created:** 10/7/2019 **Status:** ✅ Merged **Merged:** 10/15/2019 **Merged by:** [@DHowett-MSFT](https://github.com/DHowett-MSFT) **Base:** `master` ← **Head:** `dev/duhowett/font32` --- ### 📝 Commits (6) - [`26d3fcb`](https://github.com/microsoft/terminal/commit/26d3fcb1d2d31a178dca9d8d2630020f2970ecda) Allow FontInfo{,Base,Desired} to store a font name > 32 wch - [`8ab7a7c`](https://github.com/microsoft/terminal/commit/8ab7a7c5844c787ebc91f63b3070d06ce4a6a44b) Code review feedback - [`5f7d3b5`](https://github.com/microsoft/terminal/commit/5f7d3b52d5c855106134c3d8830f54ceff730d7f) FORMAT C: /Q - [`fdfd826`](https://github.com/microsoft/terminal/commit/fdfd826b2c6a3a825cfbb43dd33732009088fc70) Do more good and fun things - [`f4daa08`](https://github.com/microsoft/terminal/commit/f4daa08c196ce0fb6174e0944d016d34ad25618a) And make it a span - [`83fcb6c`](https://github.com/microsoft/terminal/commit/83fcb6c2493710e29cb027ae8f7b4e36e797a5da) Fix the crashing Selection test ### 📊 Changes **24 files changed** (+154 additions, -158 deletions) <details> <summary>View changed files</summary> 📝 `src/buffer/out/textBuffer.cpp` (+2 -6) 📝 `src/buffer/out/textBuffer.hpp` (+1 -1) 📝 `src/host/getset.cpp` (+2 -2) 📝 `src/host/output.cpp` (+1 -1) 📝 `src/host/renderFontDefaults.cpp` (+8 -4) 📝 `src/host/renderFontDefaults.hpp` (+2 -3) 📝 `src/host/selection.cpp` (+1 -6) 📝 `src/host/selection.hpp` (+0 -2) 📝 `src/host/settings.cpp` (+7 -7) 📝 `src/host/settings.hpp` (+4 -4) 📝 `src/host/srvinit.cpp` (+2 -2) 📝 `src/host/telemetry.cpp` (+1 -1) 📝 `src/interactivity/onecore/SystemConfigurationProvider.cpp` (+3 -1) 📝 `src/interactivity/onecore/SystemConfigurationProvider.hpp` (+0 -4) 📝 `src/interactivity/win32/menu.cpp` (+3 -3) 📝 `src/renderer/base/FontInfoBase.cpp` (+52 -39) 📝 `src/renderer/base/FontInfoDesired.cpp` (+6 -6) 📝 `src/renderer/base/fontinfo.cpp` (+12 -22) 📝 `src/renderer/dx/DxRenderer.cpp` (+2 -4) 📝 `src/renderer/gdi/state.cpp` (+13 -7) _...and 4 more files_ </details> ### 📄 Description ## Summary of the Pull Request We now truncate the font name as it goes out to GDI APIs, in console API servicing, and in the propsheet. I attempted to defer truncating the font to as far up the stack as possible, so as to make FontInfo usable for the broadest set of cases. ## PR Checklist * [x] Closes #602 * [x] CLA signed * [x] Tests added/passed * [ ] Requires documentation to be updated * [x] I'm of the corefolk ## Detailed Description of the Pull Request / Additional comments There are a couple things I have questions about, actually. I know that `Settings` gets memset (memsat?) by the registry deserializer, and perhaps that's another place for us to tackle. Right now, this pull request enables fonts whose names are >= 32 characters _in Windows Terminal only_, but the underpinnings are there for conhost as well. We'd need to explicitly break at the API, or perhaps return a failure or log something to telemetry. * Should we log truncation at the API boundary to telemetry? * Should the "launch face name" get the same treatment as the face name in Settings? * Should we fix Settings here, or later? * `TrueTypeFontList` is built out of things in winconp, the private console header. That fills me with dread that those structures are used for interop -- so I didn't bubble these fixes all the way up through the "default font" list, even though nominally the font list is _also_ unbounded strings. * Is `unsigned int` right for codepage? For width? I left some comments about truncating font faces around, but they could be tantamount to noise. --- <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:08:13 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
⛺ Reserved
A11yCO
A11yMAS
A11ySev1
A11ySev2
A11ySev3
A11yTTValidated
A11yUsable
A11yVoiceAccess
A11yWCAG
Area-Accessibility
Area-AtlasEngine
Area-AzureShell
Area-Build
Area-Build
Area-Chat
Area-CmdPal
Area-CodeHealth
Area-Commandline
Area-CookedRead
Area-DefApp
Area-Extensibility
Area-Fonts
Area-GroupPolicy
Area-i18n
Area-Input
Area-Interaction
Area-Interop
Area-Localization
Area-Output
Area-Performance
Area-Portable
Area-Quality
Area-Remoting
Area-Rendering
Area-Schema
Area-Server
Area-Settings
Area-SettingsUI
Area-ShellExtension
Area-ShellExtension
Area-ShellExtension
Area-Suggestions
Area-Suggestions
Area-TerminalConnection
Area-TerminalControl
Area-Theming
Area-UserInterface
Area-VT
Area-Windowing
Area-WPFControl
AutoMerge
Blocking-Ingestion
Culprit-Centennial
Culprit-WinUI
Disability-All
Disability-Blind
Disability-LowVision
Disability-Mobility
External-Blocked-WinUI3
Fixed
Gathering-Data
good first issue
HCL-E+D
HCL-WindowsTerminal
Help Wanted
Impact-Compatibility
Impact-Compliance
Impact-Correctness
Impact-Visual
In-PR
InclusionBacklog
InclusionBacklog-Windows TerminalWin32
InclusionCommitted-202206
Issue-Bug
Issue-Docs
Issue-Feature
Issue-Feature
Issue-Question
Issue-Samples
Issue-Scenario
Issue-Task
Needs-Attention
Needs-Author-Feedback
Needs-Bisect
Needs-Discussion
Needs-Repro
Needs-Tag-Fix
Needs-Tag-Fix
Needs-Triage
No-Recent-Activity
Priority-0
Priority-1
Priority-2
Priority-3
Product-Cmd.exe
Product-Colortool
Product-Colortool
Product-Colortool
Product-Conhost
Product-Conpty
Product-Meta
Product-Powershell
Product-Terminal
Product-WSL
pull-request
Mirrored from GitHub Pull Request
 Resolution-Answered
Resolution-By-Design
Resolution-Duplicate
Resolution-External
Resolution-Fix-Available
Resolution-Fix-Committed
Resolution-No-Repro
Resolution-Won't-Fix
Severity-Blocking
Severity-Crash
Severity-DataLoss
spam
this-will-be-a-breaking-change
Tracking-External
WindowsTerminal_Win32
Work-Item
zAskModeBug
zInbox-Bug
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#25240
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?