260517.6 · c67165c6c22b6758eb43209aaee45303f5b08b5b
[codex] Fix OSC 8 hyperlink state transitions (#1112)
* fix: hyperlink URL state transitions (fixes #1100)
* test hyperlink empty state coverage
reading declarations
@@ -249,6 +249,9 @@ func (s Style) Url(url string) Style { if s.url != nil { s2.url.id = s.url.id }+ if s2.url.url == "" && s2.url.id == "" {+ s2.url = nil+ } return s2 } @@ -258,12 +261,16 @@ func (s Style) Url(url string) Style { // were one Url, even if it spans multiple lines. func (s Style) UrlId(id string) Style { s2 := s- s2.url = &urlInfo{- id: "id=" + stripOSCControlsIfNeeded(id),+ s2.url = &urlInfo{}+ if id = stripOSCControlsIfNeeded(id); id != "" {+ s2.url.id = "id=" + id } if s.url != nil { s2.url.url = s.url.url }+ if s2.url.url == "" && s2.url.id == "" {+ s2.url = nil+ } return s2 }
@@ -89,6 +89,39 @@ func TestStyle(t *testing.T) { t.Errorf("wrong underline style: %v", us.GetUnderlineStyle()) } + us = us.Url("")+ if id, url := us.GetUrl(); id != "someId" || url != "" {+ t.Errorf("url clear should preserve id state: %q %q", id, url)+ }++ us = us.Url("http://example.net")+ if id, url := us.GetUrl(); id != "someId" || url != "http://example.net" {+ t.Errorf("url id not preserved across clear: %q %q", id, url)+ }++ us = us.UrlId("")+ if id, url := us.GetUrl(); id != "" || url != "http://example.net" {+ t.Errorf("url id clear did not preserve url: %q %q", id, url)+ }++ orphan := StyleDefault.UrlId("orphan")+ if id, url := orphan.GetUrl(); id != "orphan" || url != "" {+ t.Errorf("url id without url should be retained: %q %q", id, url)+ }++ orphan = orphan.Url("http://example.org")+ if id, url := orphan.GetUrl(); id != "orphan" || url != "http://example.org" {+ t.Errorf("url id set before url was not preserved: %q %q", id, url)+ }++ if id, url := StyleDefault.Url("").GetUrl(); id != "" || url != "" {+ t.Errorf("empty url should leave no hyperlink state: %q %q", id, url)+ }++ if id, url := StyleDefault.UrlId("").GetUrl(); id != "" || url != "" {+ t.Errorf("empty url id should leave no hyperlink state: %q %q", id, url)+ }+ us = us.Normal().Reverse(true).Italic(false) if us.GetAttributes() != AttrReverse { t.Errorf("wrong attributes: %v", us.GetAttributes())
@@ -793,6 +793,13 @@ func (t *tScreen) emitUrl(u urlInfo) { } } +// urlNeedsEmission reports whether a hyperlink transition has any wire effect.+// Url ids can be staged before the Url itself, and id-only transitions have no+// OSC 8 representation of their own.+func urlNeedsEmission(oldUrl, newUrl urlInfo) bool {+ return oldUrl != newUrl && (oldUrl.url != "" || newUrl.url != "")+}+ func (t *tScreen) drawCell(x, y int) int { str, style, width := t.cells.Get(x, y)@@ -826,8 +833,8 @@ func (t *tScreen) drawCell(x, y int) int { if style.url != nil { newUrl = *style.url }- // URL string can be long, so don't send it unless we really need to- if newUrl != oldUrl {+ // URL string can be long, so don't send it unless we really need to.+ if urlNeedsEmission(oldUrl, newUrl) { t.emitUrl(newUrl) } @@ -1013,7 +1020,7 @@ func (t *tScreen) draw() { } } - if t.curstyle.url != nil {+ if t.curstyle.url != nil && t.curstyle.url.url != "" { t.emitUrl(urlInfo{}) }
@@ -502,6 +502,26 @@ func TestOSC8ControlsAreStrippedFromOutput(t *testing.T) { } } +func TestOSC8IdWithoutUrlDoesNotEmitClose(t *testing.T) {+ tty := &spyTty{MockTerm: vt.NewMockTerm(vt.MockOptSize{X: 8, Y: 5})}+ s, err := NewTerminfoScreenFromTty(tty, OptAltScreen(false))+ if err != nil {+ t.Fatalf("failed to get screen: %v", err)+ }+ if err := s.Init(); err != nil {+ t.Fatalf("failed to initialize screen: %v", err)+ }+ defer s.Fini()++ tty.writes.Reset()+ s.PutStrStyled(0, 0, "X", StyleDefault.UrlId("orphan"))+ s.Show()++ if out := tty.Output(); strings.Contains(out, "\x1b]8;;\x1b\\") {+ t.Fatalf("unexpected OSC 8 close sequence for id-only style: %q", out)+ }+}+ func TestKeyboardProtocol(t *testing.T) { tests := []struct { name string