Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Support SGR 53 "overline" in terminal #181242

Closed
Tyriar opened this issue May 1, 2023 · 9 comments · Fixed by #183217 or #183951
Closed

Support SGR 53 "overline" in terminal #181242

Tyriar opened this issue May 1, 2023 · 9 comments · Fixed by #183217 or #183951
Assignees
@Tyriar
Labels
feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders terminal Integrated terminal issues terminal-rendering upstream Issue identified as 'upstream' component related (exists outside of VS Code) upstream-issue-fixed The underlying upstream issue has been fixed upstream-issue-linked This is an upstream issue that has been reported upstream verification-found Issue verification failed verification-needed Verification of issue is requested verified Verification succeeded
Milestone
May 2023

Comments

@Tyriar
Copy link
Member

Tyriar commented May 1, 2023

WT completed feature request: microsoft/terminal#6000
xterm.js issue: xtermjs/xterm.js#4499
@Tyriar Tyriar added feature-request Request for new features or functionality terminal Integrated terminal issues terminal-rendering labels May 1, 2023
@Tyriar Tyriar added this to the Backlog milestone May 1, 2023
@Tyriar Tyriar self-assigned this May 1, 2023
@Tyriar Tyriar mentioned this issue May 1, 2023
Closed
@Tyriar Tyriar changed the title Support SGR 52 "overline" in terminal Support SGR 53 "overline" in terminal May 2, 2023
@Tyriar Tyriar modified the milestones: Backlog, May 2023 May 20, 2023
@Tyriar Tyriar added upstream Issue identified as 'upstream' component related (exists outside of VS Code) upstream-issue-linked This is an upstream issue that has been reported upstream upstream-issue-fixed The underlying upstream issue has been fixed labels May 20, 2023
@Tyriar
Copy link
Member Author

Tyriar commented May 20, 2023

PR xtermjs/xterm.js#4526

Tyriar added a commit that referenced this issue May 23, 2023
@Tyriar
xterm@5.2.0-beta.49
81d508e 
Fixes #161316
Fixes #181242
@Tyriar Tyriar mentioned this issue May 23, 2023
Merged
@VSCodeTriageBot VSCodeTriageBot added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels May 23, 2023
@Tyriar
Copy link
Member Author

Tyriar commented May 26, 2023

To verify, for each value of terminal.integrated.gpuAcceleration, run the following:

echo -e '\x1b[53moverlined\x1b[55m normal'

Verify the "overlined" text has an overline and " normal" does not.

@Tyriar Tyriar added the verification-needed Verification of issue is requested label May 26, 2023
@roblourens roblourens added the verified Verification succeeded label May 30, 2023
@roblourens
Copy link
Member

With gpuAcceleration "off", I don't see an overline

image

@roblourens roblourens reopened this May 30, 2023
@roblourens roblourens added verification-found Issue verification failed and removed verified Verification succeeded labels May 30, 2023
@VSCodeTriageBot VSCodeTriageBot removed the insiders-released Patch has been released in VS Code Insiders label May 30, 2023
@ChaseKnowlden ChaseKnowlden mentioned this issue May 31, 2023
Closed
@Tyriar
Copy link
Member Author

Tyriar commented May 31, 2023

Good find, I didn't update xterm.css.

@Tyriar
Copy link
Member Author

Tyriar commented May 31, 2023

After adding the styles, depending on the font it can still be cut off unfortunately due to the cell size in the dom renderer being strict. Captured that issue in xtermjs/xterm.js#4541

@Tyriar
Copy link
Member Author

Tyriar commented May 31, 2023

When verifying again, if you don't see an overline, inspect the DOM and verify text-decoration: overline shows on the children of .xterm-rows

Tyriar added a commit that referenced this issue May 31, 2023
@Tyriar
Update xterm.css
26378c2 
Fixes #181242
@Tyriar Tyriar mentioned this issue May 31, 2023
Merged
@VSCodeTriageBot VSCodeTriageBot added the unreleased Patch has not yet been released in VS Code Insiders label May 31, 2023
@ChaseKnowlden
Copy link
Contributor

Not working still.

@ChaseKnowlden ChaseKnowlden mentioned this issue May 31, 2023
Closed
@Tyriar
Copy link
Member Author

Tyriar commented May 31, 2023

@ChaseKnowlden the remaining problem is tracked in xtermjs/xterm.js#4541

@VSCodeTriageBot VSCodeTriageBot added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels May 31, 2023
@joyceerhl joyceerhl added the verified Verification succeeded label May 31, 2023
@ChaseKnowlden
Copy link
Contributor

Not working on Latest Insiders.

@microsoft microsoft locked as resolved and limited conversation to collaborators Jun 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@Tyriar Tyriar

Labels
feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders terminal Integrated terminal issues terminal-rendering upstream Issue identified as 'upstream' component related (exists outside of VS Code) upstream-issue-fixed The underlying upstream issue has been fixed upstream-issue-linked This is an upstream issue that has been reported upstream verification-found Issue verification failed verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Milestone
May 2023
5 participants
@roblourens @Tyriar @joyceerhl @ChaseKnowlden @VSCodeTriageBot