Fix OSC color queries and add OSC 4 palette support - #327
Fix OSC color queries and add OSC 4 palette support#327Levi Zitting (glitchedmob) wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes terminal OSC color query handling by ensuring device status/attribute responses are written synchronously (so clients reading immediately after issuing a query can reliably receive a reply), and adds support for OSC 4 palette color queries used by “system theme” detection workflows.
Changes:
- Add OSC 4 (palette) query handling in the emulator, backed by new
getPaletteColor(int)APIs onTerminal/TerminalDisplay. - Make device status report (DSR) and device attributes responses write synchronously via new
TerminalOutputStream.send*Immediately(...)methods. - Add emulator tests covering OSC 4 single and multi-color query responses.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/com/jediterm/terminal/ui/TerminalPanel.java | Implements palette color lookup for OSC 4 queries in the UI display layer. |
| core/src/com/jediterm/terminal/TerminalDisplay.java | Adds getPaletteColor(int) hook for displays to supply palette colors. |
| core/src/com/jediterm/terminal/Terminal.java | Adds getPaletteColor(int) hook at the terminal API level. |
| core/src/com/jediterm/terminal/model/JediTerminal.java | Delegates palette color queries to the display; routes DSR/DA writes through immediate APIs. |
| core/src/com/jediterm/terminal/emulator/JediEmulator.java | Adds OSC 4 parsing/response logic for palette color queries. |
| core/src/com/jediterm/terminal/TerminalOutputStream.java | Introduces default “immediate” send methods intended for synchronous responses. |
| core/src/com/jediterm/terminal/TerminalStarter.java | Implements synchronous write paths and serializes writes to avoid interleaving. |
| core/tests/src/com/jediterm/util/BackBufferDisplay.java | Implements palette color lookup in the test display. |
| core/tests/src/com/jediterm/EmulatorTest.java | Adds tests for OSC 4 query responses (single and multiple indices). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public void testOsc4Query() throws IOException { | ||
| TestSession session = new TestSession(10, 10); | ||
| session.process("\u001B]4;0;?\7"); | ||
| Assert.assertEquals("\033]4;0;rgb:0000/0000/0000\7", session.getTerminal().getOutputAndClear()); | ||
|
|
||
| session.process("\u001B]4;1;?\7"); | ||
| Assert.assertEquals("\033]4;1;rgb:cdcd/0000/0000\7", session.getTerminal().getOutputAndClear()); | ||
|
|
||
| session.process("\u001B]4;16;?\7"); | ||
| Assert.assertEquals("\033]4;16;rgb:0000/0000/0000\7", session.getTerminal().getOutputAndClear()); | ||
|
|
||
| session.process("\u001B]4;232;?\7"); | ||
| Assert.assertEquals("\033]4;232;rgb:0808/0808/0808\7", session.getTerminal().getOutputAndClear()); | ||
|
|
||
| session.process("\u001B]4;0;?\u001B\\"); | ||
| Assert.assertEquals("\033]4;0;rgb:0000/0000/0000\u001B\\", session.getTerminal().getOutputAndClear()); | ||
| } | ||
|
|
||
| public void testOsc4MultipleQuery() throws IOException { | ||
| TestSession session = new TestSession(10, 10); | ||
| session.process("\u001B]4;0;?;1;?\7"); | ||
| String output = session.getTerminal().getOutputAndClear(); | ||
| Assert.assertTrue(output.contains("\033]4;0;rgb:0000/0000/0000\7")); | ||
| Assert.assertTrue(output.contains("\033]4;1;rgb:cdcd/0000/0000\7")); | ||
| } |
There was a problem hiding this comment.
The regression this PR fixes is about OSC query responses being written asynchronously (so the client can finish reading before the response is sent). The new OSC 4 tests validate formatting, but they use BackBufferTerminal/TestOutputStream which writes synchronously, so they won't catch a reintroduction of the async-response bug. Consider adding a test that uses a TerminalOutputStream where sendString(...) is intentionally async/queued and asserts that OSC 10/11/4 responses are emitted via sendStringImmediately(...) (i.e., available before process(...) returns).
There was a problem hiding this comment.
I'm not totally sure this is necessary. It might be somewhat complicated to move the pattern being suggested.
Happy to add these more complex tests if someone else thinks it's needed
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
I was trying to get the "system" theme detection working in Opencode, which uses OSC 11 to query the terminal's background color. But the query wasn't working. The terminal would receive it but never respond.
The problem was that responses were being sent asynchronously. By the time the response was sent, the application had already finished reading. This PR makes those responses synchronous, which fixes the issue. I also added support for querying palette colors since that was something else I found that the Opencode 'system' theme relied on.
Disclaimer: I did use AI to help write this code, but I'm certainly doing my best here not to spam. I did verify the tests pass and my manual testing seemed to confirm the fix. I am not an expert in Terminal stuff or event Java for that matter. I'm happy to take feedback and improve this, or hand it off to someone with more experience.