@ 261105.1

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

style.go

@@ -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 } 

style_test.go

@@ -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())

tscreen.go

@@ -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{}) 	} 

tscreen_test.go

@@ -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