Skip to content

fix(emulator): fix extended attributes response#945

Merged
gdamore merged 1 commit intomainfrom
more-key-tests
Dec 30, 2025
Merged

fix(emulator): fix extended attributes response#945
gdamore merged 1 commit intomainfrom
more-key-tests

Conversation

@gdamore
Copy link
Copy Markdown
Owner

@gdamore gdamore commented Dec 30, 2025 •
edited by coderabbitai Bot
Loading

Summary by CodeRabbit

Bug Fixes

  • Fixed terminal device control string response format to align with standard escape sequence specifications. Terminal responses now use the correct protocol format, improving compatibility with terminal emulators and ensuring proper interaction across different implementations.

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 30, 2025 •
edited
Loading

📝 Walkthrough

Walkthrough

The pull request corrects the escape sequence format for extended attributes responses in the terminal emulator. The ">q" CSI handler now emits a proper DCS-style sequence (ESC P) instead of CSI-style (ESC [), with corresponding test expectations updated to match this protocol change.

Changes

Cohort / File(s) Summary
Extended Attributes Response Format
vt/emulate.go		 Changed ">q" CSI handler to emit DCS-style escape sequence (ESC P | name version | ESC \) instead of CSI-style (ESC [ P | name version | ESC \)
Extended Attributes Test Expectations
mock/term_test.go		 Updated TestExtendedAttr expected prefix from "ESC [ P>|" to "ESC P>|" and error message from "Missing prefix 'CSI P>|'" to "Missing prefix 'ESC P>|'"

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Poem

Escape sequences dance in the night,
From brackets bold to proper light,
DCS now flows where CSI once stood,
A protocol fix, as it should! 🐰✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(emulator): fix extended attributes response' directly relates to the main change in the changeset, which corrects the escape sequence format for extended attributes (DCS) responses in the emulator.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ 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 44224cf and 1bf0d7d.

📒 Files selected for processing (2)
  • mock/term_test.go
  • vt/emulate.go
🔇 Additional comments (2)
vt/emulate.go (1)

623-626: Protocol correction looks good!

The DCS response now properly uses ESC P instead of ESC [ P, aligning with the VT terminal standard for Device Control String sequences. The format \x1bP>|name version\x1b\\ correctly frames the terminal identity response.

mock/term_test.go (1)

239-241: Test expectations correctly updated!

The test now validates the proper DCS prefix format and the error message accurately reflects "ESC P>|" terminology instead of the previous "CSI P>|". This aligns perfectly with the protocol correction in vt/emulate.go.

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

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

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 30, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.37%. Comparing base (44224cf) to head (1bf0d7d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #945      +/-   ##
==========================================
+ Coverage   61.64%   62.37%   +0.72%     
==========================================
  Files          37       37              
  Lines        3293     3293              
==========================================
+ Hits         2030     2054      +24     
+ Misses       1107     1081      -26     
- Partials      156      158       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gdamore gdamore merged commit 1bf0d7d into main Dec 30, 2025
15 checks passed
@gdamore gdamore deleted the more-key-tests branch December 30, 2025 02:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@gdamore