Repository navigation
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #4531, #6189 — both of those are about the OSC 8 sequences themselves not being serialized, which this PR does not address. What it fixes is the underline corruption that shows up on the same cells: the serializer treats the hyperlink render hint as a real attribute. Adding OSC 8 serialization is a follow-up.
Problem
The ANSI and HTML serializers read the underline attributes straight off
IBufferCell, butExtendedAttrs.underlineStylereportsDASHEDfor every hyperlink cell so the renderers draw links with a dashed underline. That hint is not part of the cell's real attributes, and it leaks into serialization in two ways:SGR 4, so replaying a snapshot grows an underline the application never asked for.SGR 4:5whatever its real style, soSGR 4/4:2/4:3/4:4all come back as dashed.With
link(x)=OSC 8 ;; … ST+x+OSC 8 ;; ST:A+link(B)A\x1b[4mB\x1b[0mABA+link(SGR 4 + B)A\x1b[4:5mB\x1b[0mA\x1b[4mB\x1b[0mA+link(SGR 4:3 + B)A\x1b[4:5mB\x1b[0mA\x1b[4:3mB\x1b[0mserializeAsHTMLhas the same problem: it emitstext-decoration: underline dashedfor an underlined link, and a decoration that was never requested for a bare link.Two adjacent cells of the same hyperlink that differ only in their real underline style were also collapsed into one, because
_diffStylestarts with a fast path that delegates toCellData.attributesEquals, which compares the same hint-glazed getters:link(SGR 4:2 + AA + SGR 4:4 + BB)\x1b[4:5mAABB\x1b[0m\x1b[4:2mAA\x1b[4:4mBB\x1b[0mFix
Read the real underline state through a copy of the extended attributes with the link id cleared, and use it wherever the serializer previously used
IBufferCell.isUnderline()orgetUnderlineStyle(). Cells that are not part of a hyperlink take the untouched path and keep using theCellData.attributesEqualsfast path, so their output is unchanged.Note on
CellData.attributesEqualsIt has the same latent problem for hyperlink cells. Since it is only used by this addon I worked around it here rather than changing core; happy to move it into
CellDatainstead if that is preferred.Verification
masterand pass with the fix.npm run test-unit -- addons/addon-serialize/out-esbuild/SerializeAddon.test.js→ 51 passing.npm run lint→Found 0 warnings and 0 errors.