diff --git a/pkg/gocui/escape.go b/pkg/gocui/escape.go index 174da7461..690ce527c 100644 --- a/pkg/gocui/escape.go +++ b/pkg/gocui/escape.go @@ -25,6 +25,18 @@ type escapeInterpreter struct { // the payload of an OSC 1717 per-line diff-metadata sequence (see // diff-line-metadata-notes.md), accumulated like hyperlink metadata strings.Builder + // OSC 1717 payloads that were superseded by the next record before any + // cell consumed them. A row can carry several records back-to-back with + // nothing rendered in between — difftastic's combined file+hunk banner, or + // the deletion+addition pair of a modification row collapsed to a single + // column — so a record's region can be zero-width. The write loop drains + // these into content-less carrier cells, keeping every record of the row + // discoverable instead of only the last one. + orphanedMetadata []string + // whether the payload currently in metadata has been stamped onto at + // least one cell; an unconsumed payload is orphaned rather than dropped + // when the next record arrives or the line ends + metadataConsumed bool // ConPTY emits cursor-positioning escapes (CUP) to skip over blank // rows rather than emitting LFs for them. To convert those into row @@ -446,7 +458,7 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { ei.hyperlink.Reset() ei.state = stateOSCParams case "1717": - ei.metadata.Reset() + ei.orphanUnconsumedMetadata() ei.state = stateOSCMetadata default: ei.state = stateOSCSkipUnknown @@ -509,6 +521,26 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { return false, nil } +// orphanUnconsumedMetadata prepares the metadata accumulator for a new OSC 1717 +// record. If the previous record's payload never made it onto a cell (its region +// is zero-width because the next record follows immediately — see +// orphanedMetadata), it is kept as an orphan rather than dropped. +func (ei *escapeInterpreter) orphanUnconsumedMetadata() { + if ei.metadata.Len() > 0 && !ei.metadataConsumed { + ei.orphanedMetadata = append(ei.orphanedMetadata, ei.metadata.String()) + } + ei.metadata.Reset() + ei.metadataConsumed = false +} + +// takeOrphanedMetadata hands the accumulated zero-width-record payloads (see +// orphanedMetadata) to the caller and clears the list. +func (ei *escapeInterpreter) takeOrphanedMetadata() []string { + result := ei.orphanedMetadata + ei.orphanedMetadata = nil + return result +} + // dropMetadataIfHandshake discards a just-completed OSC 1717 payload that carries no // fields (no ';'). A metadata-aware pager emits such a version-only record once, as // its first output, to announce it speaks the protocol (so we can probe it; see the diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index 05602bb86..c04b9c86e 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -956,13 +956,18 @@ func (b *viewBuffer) write(v *View, p []byte) { finishLine := func() { b.autoRenderHyperlinksInCurrentLine(v) - // A pager can render a blank changed line as just its OSC 1717 metadata - // followed by an empty line — delta does this for some empty deleted/added - // lines. Keep a content-less cell to carry that metadata, so the line is - // still recognized as a change; without it the line resolves to nothing and - // breaks a change block in two (e.g. when selecting a hunk in the focused - // main view). - if len(b.lines[b.wy].cells) == 0 && b.ei.metadata.Len() > 0 { + // A record whose region reached the line end without covering any cell + // still belongs to this line: a zero-width record region (see + // escapeInterpreter.orphanedMetadata), or a blank changed line that a + // pager renders as just its OSC 1717 metadata followed by the newline — + // delta does this for some empty deleted/added lines. Keep content-less + // cells to carry those payloads, so the line is still recognized as a + // change; without them the line resolves to nothing and breaks a change + // block in two (e.g. when selecting a hunk in the focused main view). + for _, payload := range b.ei.takeOrphanedMetadata() { + b.writeCells([]cell{{metadata: payload}}) + } + if b.ei.metadata.Len() > 0 && !b.ei.metadataConsumed { b.writeCells([]cell{{metadata: b.ei.metadata.String()}}) } } @@ -1121,6 +1126,15 @@ func (b *viewBuffer) parseInput(v *View, ch []byte, width int, x int, _ int) (bo truncateLine := false isEscape, err := b.ei.parseOne(ch) + + // An OSC 1717 record superseded before any cell consumed it (a zero-width + // record region — see escapeInterpreter.orphanedMetadata) still belongs to + // this line: materialize each such payload as a content-less carrier cell, + // in emission order, ahead of whatever this byte produces. + for _, payload := range b.ei.takeOrphanedMetadata() { + cells = append(cells, cell{metadata: payload}) + } + if err != nil { for _, chr := range b.ei.characters() { c := cell{ @@ -1147,7 +1161,7 @@ func (b *viewBuffer) parseInput(v *View, ch []byte, width int, x int, _ int) (bo fg: b.ei.curFgColor, bg: b.ei.curBgColor, } - return truncateLine, []cell{} + return truncateLine, cells } else if cf, ok := b.ei.instruction.(cursorForward); ok { // emit `n` space cells under the parser-tracked SGR — used // to materialize ConPTY's compressed runs of spaces (which @@ -1157,8 +1171,11 @@ func (b *viewBuffer) parseInput(v *View, ch []byte, width int, x int, _ int) (bo ch = []byte{' '} width = 1 } else if isEscape { - // do not output anything - return truncateLine, nil + // no visible output — but carrier cells still need writing + if len(cells) == 0 { + return truncateLine, nil + } + return truncateLine, cells } else if characterEquals(ch, '\t') { // fill tab-sized space tabWidth := v.TabWidth @@ -1177,6 +1194,9 @@ func (b *viewBuffer) parseInput(v *View, ch []byte, width int, x int, _ int) (bo chr: string(ch), width: width, } + if c.metadata != "" { + b.ei.metadataConsumed = true + } for range repeatCount { cells = append(cells, c) } diff --git a/pkg/gocui/view_test.go b/pkg/gocui/view_test.go index a94c04c0f..ed04035c0 100644 --- a/pkg/gocui/view_test.go +++ b/pkg/gocui/view_test.go @@ -259,6 +259,48 @@ func TestDiffLineMetadataPayloads(t *testing.T) { }, v.DiffLineMetadataPayloads()) } +func TestDiffLineMetadataZeroWidthRecords(t *testing.T) { + v := NewView("name", 0, 0, 80, 10, OutputNormal) + + // A row can carry several records back-to-back with nothing rendered in + // between, making all but the last record's region zero-width. Those + // payloads must survive as payloads of the row (via content-less carrier + // cells) rather than being clobbered by the record that follows. + osc := func(payload string) string { return "\x1b]1717;" + payload + "\x1b\\" } + v.writeString(strings.Join([]string{ + // difftastic's combined file+hunk banner: the f is immediately + // followed by the h. + osc("1;f;;;foo.txt") + osc("1;h;5;;foo.txt") + "foo.txt --- Go", + // A modification row collapsed to a single column carries its + // deletion and its addition back-to-back before the row's content. + osc("1;d;5;5;foo.txt") + osc("1;a;5;;foo.txt") + "595 new content", + // A record emitted right before the line end covers no cell either. + "trailing" + osc("1;d;6;6;foo.txt"), + }, "\n") + "\n") + + assert.Equal(t, [][]string{ + {"1;f;;;foo.txt", "1;h;5;;foo.txt"}, + {"1;d;5;5;foo.txt", "1;a;5;;foo.txt"}, + {"1;d;6;6;foo.txt"}, + }, v.DiffLineMetadataPayloads()) + + // The carrier cells are invisible: the rendered text is unchanged. + assert.Equal(t, []string{ + "foo.txt --- Go", + "595 new content", + "trailing", + }, v.BufferLines()) + + // A row's single-payload identity is its first record: the f of a banner, + // the d of a collapsed modification row. + payload, ok := v.DiffLineMetadataInLine(0) + assert.True(t, ok) + assert.Equal(t, "1;f;;;foo.txt", payload) + payload, ok = v.DiffLineMetadataInLine(1) + assert.True(t, ok) + assert.Equal(t, "1;d;5;5;foo.txt", payload) +} + func TestDiffLineMetadataHandshakeSwallowed(t *testing.T) { v := NewView("name", 0, 0, 80, 10, OutputNormal)