SGR 2 (Faint) support

[fy0]

https://xtermjs.org/docs/api/vtfeatures/> SGR 2: Faint, decreased intensity.

Made it for gray text from codex:

Summary by CodeRabbit

• New Features

  • Proper support for wide (2-cell) characters with placeholders for multi-width runes.
  • Faint/dim text rendering with automatic color dimming.

• Improvements

  • Modernized platform build-tag syntax.
  • Added rune-width handling via an external library to improve layout.

• Tests

  • New test validating faint attribute and dimmed foreground color.

• API Changes

  • Exported text attribute constants and added RawLines() for direct line access.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds modern go:build directives to platform-gated files, promotes several terminal attribute flags to exported Attr* constants (plus AttrFaint, AttrWide, AttrWideDummy), exposes RawLines() and dimColor(), implements runewidth-aware wide-rune insertion via github.com/rivo/uniseg, and adds a faint-handling test.

Changes

Cohort / File(s)	Summary
Build constraint modernization ioctl_other.go, ioctl_posix.go, vt_other.go, vt_posix.go	Add modern //go:build directives alongside existing // +build lines for Plan9/NACL/Windows and POSIX platforms; no runtime logic changes.
State & attributes state.go	Promote private attr constants to exported Attr* names; add AttrFaint, AttrWide, AttrWideDummy; add RawLines() []line and dimColor(Color) Color; update attribute application, resets, and rendering logic to account for faint and wide attributes and dimmed colors.
Parser and wide-rune handling parse.go	Replace private attr usages with exported names (attrGfx→AttrGfx, attrWrap→AttrWrap); import github.com/rivo/uniseg; implement rune-width-aware insertion and wrapping for width-2 runes, placeholders for wide-char tails, and cleanup of overlapped wide cells.
Tests vt_test.go	Add TestSGRFaint to verify SGR faint handling: ensures AttrFaint is set and FG color is dimmed (RGB halved) as expected.
Module dependencies go.mod	Add github.com/rivo/uniseg v0.4.7 requirement.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 I nibble bytes and nibble lines,
I mark the wide where rune-width shines.
A faint new hue, a softer light,
Build tags fresh — the code hops right.
Hooray! I twirl my whiskers bright. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'SGR 2 (Faint) support' accurately describes the main objective of the PR, which is to add support for the SGR 2 (Faint) attribute for rendering decreased intensity text.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4bc538e and 051b8e0.

📒 Files selected for processing (2)

• go.mod
• parse.go

🧰 Additional context used

🧬 Code graph analysis (1)

parse.go (1)

state.go (6)

• State (86-107)
• AttrGfx (17-17)
• ModeWrap (37-37)
• AttrWrap (20-20)
• AttrWideDummy (23-23)
• AttrWide (22-22)

🔇 Additional comments (8)

parse.go (7)

3-4: LGTM!

The uniseg import is appropriate for Unicode-aware character width calculation.

---

12-21: LGTM!

Consistent use of the exported AttrGfx and AttrWrap constants. The wrap handling logic is preserved correctly.

---

28-31: LGTM!

The zero-width fallback to 1 is a reasonable safety measure for terminal emulation. Note that this may cause combining characters (like accents) to be rendered with their own cell rather than combining with the previous character, which is typical terminal behavior.

---

33-43: LGTM!

Correct handling of wide characters at line boundaries. When wrap mode is disabled and a 2-width character doesn't fit, dropping it silently is standard terminal behavior.

---

45-79: LGTM!

The wide character overlap handling is comprehensive and correct:

• Properly clears the main part when overwriting a dummy cell
• Properly clears the dummy when overwriting a main wide char
• Handles the cascading case where placing a new wide char's dummy would overwrite another wide char's main

The use of space ' ' for the dummy cell is standard practice.

---

81-86: LGTM!

Cursor movement correctly advances by the character's display width and properly sets the wrap-next state when reaching the line boundary.

---

190-192: LGTM!

Consistent use of the exported AttrGfx constant.

go.mod (1)

5-5: Dependency version is current and secure.

The uniseg v0.4.7 dependency is the latest released version and has no known security vulnerabilities. The addition appropriately supports Unicode-aware character width calculation.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

state.go (1)

292-300: Dimming by 50% may be too aggressive for faint text.

The dimColor function halves each RGB component to create a faint appearance. While this is simple and effective, dimming by 50% may make text too hard to read in some color schemes, especially for already-dark colors.

Consider:

1. Using a less aggressive dimming factor (e.g., 33% or 25% reduction) for better readability.
2. Making the dimming factor configurable.
3. Testing with various color combinations to ensure faint text remains legible.

That said, 50% dimming matches the reference image mentioned in the PR description, so this may be intentional.

Please verify that the 50% dimming matches the intended visual appearance from the reference image and provides acceptable readability across different color schemes.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c2f2317 and 4bc538e.

📒 Files selected for processing (3)

• go.mod
• parse.go
• state.go

🧰 Additional context used

🧬 Code graph analysis (2)

state.go (1)

color.go (3)

• Color (35-35)
• DefaultFG (27-27)
• DefaultBG (28-28)

parse.go (1)

state.go (6)

• State (86-107)
• AttrGfx (17-17)
• ModeWrap (37-37)
• AttrWrap (20-20)
• AttrWideDummy (23-23)
• AttrWide (22-22)

🔇 Additional comments (9)

parse.go (4)

28-31: Treating zero-width as 1 may not handle combining characters correctly.

Zero-width characters (e.g., combining marks) are treated as width 1, which causes them to advance the cursor. In proper Unicode rendering, these should combine with the previous character without advancing the cursor.

Is combining character support required for this terminal emulator? If so, this logic should be enhanced to handle width==0 properly by combining with the previous cell rather than treating it as width 1.

---

34-43: Wide character silently dropped when wrapping is disabled.

When a wide character doesn't fit and ModeWrap is not enabled, the function returns early (line 41), silently dropping the character without writing anything.

Please confirm this is the intended behavior for the terminal emulator. Some terminals might instead truncate the wide character or write a replacement character.

---

45-86: Wide character handling logic is comprehensive and well-bounded.

The implementation correctly:

• Clears overlapped wide characters in all scenarios (writing over dummy, main, or placing dummy over another wide char's main).
• Performs proper bounds checking before all array accesses.
• Sets appropriate flags (AttrWide, AttrWideDummy) for the main and dummy cells.
• Moves the cursor by the actual character width.

The complexity is necessary to maintain correct rendering state when wide characters are overwritten or wrap across lines.

---

3-3: No action needed — the library version is valid and secure.

github.com/mattn/go-runewidth v0.0.19 is the latest available version with no known vulnerabilities or security issues.

go.mod (1)

5-5: Dependency addition aligns with wide-character support.

The addition of github.com/mattn/go-runewidth v0.0.19 is necessary for the wide-character handling implemented in parse.go. Version verification was requested in the parse.go review.

state.go (4)

14-24: Exported attribute constants enable external API usage.

The promotion of attribute constants to exported names (e.g., AttrReverse, AttrBold) and the addition of new constants (AttrFaint, AttrWide, AttrWideDummy) align well with the PR objectives:

• AttrFaint implements SGR 2 (faint/decreased intensity) support.
• AttrWide and AttrWideDummy enable correct wide-character (CJK) rendering.

The comments on lines 22-23 helpfully document the purpose of the wide-character flags.

---

162-166: Unsynchronized access via RawLines() could lead to data races.

The RawLines() method exposes the internal lines slice without acquiring the mutex, breaking the thread-safety guarantees typically provided by the State type. While the comment documents this, it places a significant burden on callers to ensure proper synchronization.

Please confirm:

1. Is this API necessary, or could callers use the existing Cell(x, y) method with proper locking?
2. Where will this be used, and have the call sites been audited to ensure they properly synchronize access?
3. Consider making this a private method (e.g., rawLines()) if it's only needed internally.

Based on learnings, unsynchronized APIs in concurrent code require careful justification and documentation of the exact use cases.

---

280-285: Faint and bold attributes are mutually exclusive.

The implementation ensures that bold (line 280) and faint (line 283) are mutually exclusive:

• Bold brightening (FG + 8) is only applied if NOT faint.
• Faint dimming is only applied if NOT bold.

This is a reasonable design choice, as applying both simultaneously would be ambiguous. However, this behavior should be documented, as some terminal emulators may handle the combination differently.

Please confirm this mutual exclusion matches the intended terminal behavior and consider documenting it in code comments or external documentation.

---

636-660: SGR attribute handling correctly implements faint support.

The implementation properly handles SGR (Select Graphic Rendition) sequences:

• SGR 0 (line 636): Resets all attributes including the new AttrFaint.
• SGR 2 (lines 641-642): Sets AttrFaint as per the PR objective.
• SGR 21/22 (line 652): Clears both AttrBold and AttrFaint, correctly implementing "normal intensity."

All changes use the exported attribute constants consistently.