Skip to content

Support 'px' format in lineHeight option #5363

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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jun 21, 2025

This PR adds support for specifying lineHeight using pixel values with the 'px' suffix, while maintaining full backward compatibility with existing numeric multiplier format.

Problem

Previously, users could only specify lineHeight as a numeric multiplier (e.g., 1.2). When using DOM rendering, users wanted to set exact pixel values but had to manually calculate the multiplier:

// Before: Want 23px line height with 16px character height
// Had to calculate: 23 ÷ 16 = 1.4375
const term = new Terminal({ lineHeight: 1.4375 });

This was unintuitive and error-prone, especially when the character height changed.

Solution

Now lineHeight accepts both formats:

// Existing numeric format (unchanged)
const term1 = new Terminal({ lineHeight: 1.2 });

// New px format 
const term2 = new Terminal({ lineHeight: '24px' });

Implementation

  • Type System: Updated public and headless APIs to accept lineHeight?: number | string
  • Validation: Enhanced OptionsService to validate both formats with proper error handling
  • Renderers: Added shared calculateLineHeightInPixels() utility function for both DOM and WebGL renderers
  • Safety: Enforces minimum values (≥1 for numeric, ≥1px for string, but also ≥ character height)
  • WebGL Compatibility: Maintains char atlas compatibility by converting px values to multipliers when needed

Testing

  • All existing tests pass (2242 passing)
  • Added comprehensive test coverage for new functionality
  • Validates both valid and invalid inputs
  • Tests calculation logic for edge cases

Examples

// Numeric multipliers (existing behavior)
new Terminal({ lineHeight: 1.0 });   // 16px char → 16px line height
new Terminal({ lineHeight: 1.2 });   // 16px char → 19px line height  
new Terminal({ lineHeight: 1.5 });   // 16px char → 24px line height

// Pixel values (new feature)
new Terminal({ lineHeight: '20px' }); // Always 20px line height
new Terminal({ lineHeight: '23px' }); // Always 23px line height
new Terminal({ lineHeight: '25px' }); // Always 25px line height

// Minimum enforcement
new Terminal({ lineHeight: '10px' }); // 16px char → 16px line height (minimum)

Fixes #2612.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Copilot AI and others added 2 commits June 21, 2025 18:19
@Copilot Copilot AI changed the title [WIP] Support 'px' format in lineHeight option Support 'px' format in lineHeight option Jun 21, 2025
@Copilot Copilot AI requested a review from Tyriar June 21, 2025 18:24
Copilot finished work on behalf of Tyriar June 21, 2025 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support 'px' format in lineHeight option
2 participants