Skip to content

Support OSC-1717 metadata#538

Open
stefanhaller wants to merge 3 commits into
so-fancy:nextfrom
stefanhaller:osc-1717-metadata
Open

Support OSC-1717 metadata#538
stefanhaller wants to merge 3 commits into
so-fancy:nextfrom
stefanhaller:osc-1717-metadata

Conversation

@stefanhaller

Copy link
Copy Markdown

This adds support for the OSC-1717 spec described in jesseduffield/lazygit#5731; see also jesseduffield/lazygit#5732 for more context.

The spec is still in draft mode, so we may want to hold off merging this until we got more feedback on it. On the other hand, we want pagers to be ready by the time lazygit releases the feature, that's why I'm opening this for review already.

stk and others added 2 commits July 2, 2026 21:44
A host that renders diff-so-fancy's output (e.g. lazygit) needs to map a
rendered diff row back to its patch-space identity -- file, line type, and
new/old line numbers -- to act on the line the user points at. diff-so-fancy
strips the +/- markers and conveys the side by color, so that identity cannot
be recovered from the painted text; the pager, which still has it at render
time, has to state it.

Gate the emission on the OSC1717_METADATA environment variable so output is
byte-for-byte unchanged outside such a host -- harmless in a raw terminal, less,
or tmux. The host advertises the protocol versions it understands and
diff-so-fancy emits the highest mutually-understood one (just V1 today). See the
protocol in diff-line-metadata-osc-spec.md; this is the third reference
implementation after delta and difftastic.

The emitter tracks its own old/new line counters, seeded from each hunk header,
and classifies every content line by its leading +/- indicator -- read before
the existing code strips that indicator and rewrites the line. sanitize_display
strips OSC sequences, so the record is prepended to the already-sanitized line
rather than embedded in it. The path prefers the new-file side, falling back to
the old side for a deletion (whose new side is /dev/null), which also recovers
the path for a noprefix deletion that never reaches $last_file_seen.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A host can already learn diff-so-fancy's per-line diff metadata, but only from
the records emitted before content lines — so a diff with no content (a binary
file, or the empty diff a host would use to probe) emits nothing, and "speaks
the protocol" is indistinguishable from "unsupported pager".

Emit a version-only OSC 1717 record (no further fields) once, before processing,
whenever a version is negotiated. It's content-independent, so a host can probe
diff-so-fancy on an empty diff and get a conclusive answer.

The osc_records test helper now skips this handshake record so the per-line
assertions are unchanged; two new tests cover the handshake (first, and present
on an empty diff). See diff-line-metadata-osc-spec.md §4.4.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@scottchiefbaker

Copy link
Copy Markdown
Contributor

I'd definitely like to start working on and testing this new feature. I'd like to target a feature branch instead of next. I'm not sure how that works on GitHub? Are you able to target a new feature branch? Do I have to create it before you can commit to it?

@scottchiefbaker

scottchiefbaker commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

I applied this .diff manually to my tree and created a new branch.

Please look at: https://github.com/so-fancy/diff-so-fancy/tree/osc1717

I did massage some of your code (which was quite well written) to match my coding conventions. All the unit tests still pass.

@stefanhaller

Copy link
Copy Markdown
Author

I'm not sure what you mean by "target a feature branch". Your new osc1717 branch seems to contain unrelated changes, is that what you mean? I would strongly recommend against that, let's keep independent things separate please. If your goal is to be able to test this OSC stuff together with other in-flight changes, I recommend to create a throwaway integration branch for that purpose that squash-merges all the branches that you want to test together; see here for an example of such a branch.

I pushed a fixup commit to this branch to carry over your changes into this PR; please check if I got them all. I'd prefer to keep working here; you have permission to push changes to this branch too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants