feat: add Web and Swing Terminal Implementations#1348
Conversation
89d5e30 to
91428cc
Compare
4343e1c to
6268b33
Compare
6268b33 to
d119191
Compare
|
augment review |
🤖 Augment PR SummarySummary: Adds new Web and Swing terminal front-ends built on Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
| } | ||
| } | ||
|
|
||
| public synchronized void waitDirty(int timeout) throws InterruptedException { |
There was a problem hiding this comment.
waitDirty(int timeout) loops until dirty becomes true, even if wait(timeout) times out, so it can block forever when no updates occur. That makes the timeout parameter surprising for callers who expect a bounded wait.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| // B: Background r-g-b | ||
|
|
||
| // Extract character and attributes from cell | ||
| char ch = (char) (cell & 0xffffffffL); |
There was a problem hiding this comment.
ScreenTerminal stores characters as full Unicode code points (lower 32 bits), but Swing rendering truncates them via (char) (cell & 0xffffffffL), so non-BMP characters (e.g., emoji) can’t render correctly. This also means wide-char handling added in ScreenTerminal won’t display as intended in Swing.
Severity: high
Other Locations
builtins/src/main/java/org/jline/builtins/SwingTerminal.java:660
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| @Override | ||
| public void write(int b) throws IOException { | ||
| if (component != null) { | ||
| component.write(String.valueOf((char) b)); |
There was a problem hiding this comment.
WebTerminalOutputStream.write(int) converts each byte directly to a char, which will corrupt UTF-8 output if anything writes one byte at a time (multi-byte sequences will be split). This differs from write(byte[],...) which decodes as UTF-8, so behavior depends on which overload the caller hits.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
augment review |
| if (!dirty.compareAndSet(true, false)) { | ||
| wait(timeout); | ||
| } | ||
| return dirty.compareAndSet(true, false); |
There was a problem hiding this comment.
As written, waitDirty(timeout) clears dirty in the initial compareAndSet and then immediately checks it again, so it returns false even when the screen was already dirty on entry. This can also drop a pending dirty notification by resetting the flag before waiting/returning.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| } | ||
| } | ||
|
|
||
| cursor_right(charWidth); |
There was a problem hiding this comment.
| int fg = (int) ((attr >>> 12) & 0x0fff); | ||
| boolean underline = (attr & 0x01000000L) != 0; | ||
| boolean inverse = (attr & 0x02000000L) != 0; | ||
| boolean conceal = (attr & 0x04000000L) != 0; |
| int cellY = y * charHeight; | ||
|
|
||
| // Determine character width (wide chars span 2 cells) | ||
| int cellSpan = Character.charCount(cp) > 1 || isWideCharacter(cp) ? 2 : 1; |
There was a problem hiding this comment.
|
|
||
| if (endOfInput) { | ||
| // Final flush | ||
| outputBuffer.clear(); |
There was a problem hiding this comment.
In decodeAndOutput(true), outputBuffer.clear() is called after decoder.decode(...), which discards any chars produced by that decode before the final flush(). This can lose the last decoded characters when the stream is closed.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| trailingIncomplete++; | ||
| } | ||
| if (trailingIncomplete > 0) { | ||
| int leadByte = b[end - 1 - trailingIncomplete] & 0xFF; |
There was a problem hiding this comment.
If the byte array ends with only UTF-8 continuation bytes (i.e., no leading byte before trailingIncomplete), b[end - 1 - trailingIncomplete] can index before off + i and throw ArrayIndexOutOfBoundsException.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| private void serveStaticResource(HttpExchange exchange, String path) throws IOException { | ||
| String resourcePath = path.substring(8); // Remove "/static/" | ||
| InputStream is = getClass().getResourceAsStream("/" + resourcePath); |
| * This is a simplified version - in the real implementation, | ||
| * we would need to properly capture the output from the SwingTerminalOutputStream. | ||
| */ | ||
| private String getCapturedTextFromTerminal() { |
There was a problem hiding this comment.
This test describes capturing output via TestTerminalComponent, but all assertions use swingTerminal.dump(...), so it doesn’t actually validate the SwingTerminalOutputStream decoder behavior it claims to test.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| } | ||
|
|
||
| public synchronized boolean waitDirty(long timeout) throws InterruptedException { | ||
| if (!dirty.compareAndSet(true, false)) { |
There was a problem hiding this comment.
waitDirty(long) clears dirty when it is already set (via compareAndSet(true, false)) but then returns false because the final compareAndSet(true, false) sees false. That means callers can miss an already-dirty state even though the method consumed it.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| if (endOfInput) { | ||
| // Final flush | ||
| outputBuffer.clear(); |
There was a problem hiding this comment.
In decodeAndOutput(true), outputBuffer.clear() discards any characters produced by decoder.decode(..., endOfInput=true) before flush(), so close() can silently drop trailing decoded output/replacement chars for incomplete sequences.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| int fg = (int) ((attr >>> 12) & 0x0fff); | ||
| boolean underline = (attr & 0x01000000L) != 0; | ||
| boolean inverse = (attr & 0x02000000L) != 0; | ||
| boolean conceal = (attr & 0x04000000L) != 0; |
| int cellY = y * charHeight; | ||
|
|
||
| // Determine character width (wide chars span 2 cells) | ||
| int cellSpan = Character.charCount(cp) > 1 || isWideCharacter(cp) ? 2 : 1; |
There was a problem hiding this comment.
cellSpan treats all non-BMP code points (Character.charCount(cp) > 1) as double-width, which can diverge from ScreenTerminal’s utf8_charwidth/continuation-marker behavior. This can make background/underline painting span into the next cell even when there is no continuation marker.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| private void serveStaticResource(HttpExchange exchange, String path) throws IOException { | ||
| String resourcePath = path.substring(8); // Remove "/static/" | ||
| InputStream is = getClass().getResourceAsStream("/" + resourcePath); |
There was a problem hiding this comment.
- Fix waitDirty() race condition: use dirty.get() instead of compareAndSet in guard to avoid consuming the flag prematurely - Apply conceal attribute (SGR 8) in SwingTerminal rendering - Fix decodeAndOutput() buffer handling: read decoded output before clearing buffer for flush, preventing data loss on stream close - Fix cellSpan: use isWideCharacter() only, not Character.charCount(), since not all non-BMP characters are wide - Fix resource leak: wrap getResourceAsStream() in try-with-resources - Fix UTF-8 byte array bounds check in WebTerminalOutputStream - Fix dump() parameter type from int to long for consistency - Use StandardCharsets.UTF_8 in URLDecoder and error responses - Remove dead code (getOrCreateSession, spurious serialVersionUID) - Remove DEBUG println statements from tests - Revert unnecessary StringBuilder for CSI parameter bytes (always ASCII) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
augment review |
| @Override | ||
| public void keyTyped(KeyEvent e) { | ||
| char ch = e.getKeyChar(); | ||
| if (ch != '\t' && ch != KeyEvent.CHAR_UNDEFINED && !e.isControlDown()) { |
There was a problem hiding this comment.
keyTyped enqueues any non-Tab character, including control chars like \n (Enter) and \b (Backspace), while keyPressed also enqueues sequences for those keys. This can result in duplicate input being sent into the line discipline.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| // Skip continuation cells for wide characters (they have cp == 0) | ||
| // The leading cell already rendered the glyph spanning multiple cells | ||
| if (cp == 0) { |
There was a problem hiding this comment.
Continuation cells (cp == 0) return without painting, so they won’t clear any previously painted glyph/background unless the leading cell paints across them. Since the span is derived from isWideCharacter(cp) (not from the ScreenTerminal continuation markers), wide chars like emoji can leave stale content or overwrite adjacent cells.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| /** | ||
| * Disposes of the terminal resources. | ||
| */ | ||
| public void dispose() { |
There was a problem hiding this comment.
Cleanup is only done in dispose(), but Terminal.close() (inherited from LineDisciplineTerminal) doesn’t set this class’s closed flag or stop inputThread / cursorTimer. Callers using try-with-resources or only calling close() can leak a running input thread/timer.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| /** | ||
| * Stops the HTTP server. | ||
| */ | ||
| public void stop() { |
There was a problem hiding this comment.
WebTerminal requires an explicit stop(): calling Terminal.close() won’t stop the HttpServer or shut down the executor, so a caller that only closes the terminal can leak a bound server and threads. Consider tying server shutdown to close()/onClose so resource cleanup is reliable.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
- Fix waitDirty() race condition: use dirty.get() instead of compareAndSet in guard to avoid consuming the flag prematurely - Apply conceal attribute (SGR 8) in SwingTerminal rendering - Fix decodeAndOutput() buffer handling: read decoded output before clearing buffer for flush, preventing data loss on stream close - Fix cellSpan: use isWideCharacter() only, not Character.charCount(), since not all non-BMP characters are wide - Fix resource leak: wrap getResourceAsStream() in try-with-resources - Fix UTF-8 byte array bounds check in WebTerminalOutputStream - Fix dump() parameter type from int to long for consistency - Use StandardCharsets.UTF_8 in URLDecoder and error responses - Remove dead code (getOrCreateSession, spurious serialVersionUID) - Remove DEBUG println statements from tests - Revert unnecessary StringBuilder for CSI parameter bytes (always ASCII) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
f39fe4b to
0ab3814
Compare
Add WebTerminal and SwingTerminal, two new Terminal implementations in the builtins module that extend LineDisciplineTerminal. WebTerminal serves a browser-based terminal over HTTP using JDK's built-in HttpServer. It renders ANSI escape sequences as styled HTML, supports GZIP compression, and handles keyboard input via AJAX polling. SwingTerminal renders a terminal in a Swing JComponent with full font metrics, ANSI color support, cursor blinking, and keyboard/mouse input handling. It can be embedded in any Swing application. Both terminals build on ScreenTerminal, which has been enhanced with: - Wide character (CJK) support via WCWidth - Continuation cell markers for multi-cell characters - dump() overloads for screen content extraction - waitDirty() for efficient polling Also includes: - Launcher utility to run any JLine demo in system/web/swing mode - Example programs for both terminal types - Documentation for the website - Comprehensive test suite including integration tests
0ab3814 to
c27a041
Compare
Overview
This PR introduces two new terminal implementations built on top of the existing
ScreenTerminalclass, enabling JLine to be used in web applications and desktop GUI applications.New Features
🌐 WebTerminal
An HTTP-based terminal implementation that serves a web interface:
HttpServer(no external dependencies)🖥️ SwingTerminal
A Swing JComponent-based terminal for desktop applications:
Implementation Details
Architecture
ScreenTerminaland inherit full ANSI escape sequence supportCode Quality
Files Added
Core Implementation
builtins/src/main/java/org/jline/builtins/WebTerminal.java- Web-based terminalbuiltins/src/main/java/org/jline/builtins/SwingTerminal.java- Swing-based terminalbuiltins/src/main/java/org/jline/builtins/TerminalDemo.java- Demo applicationsTesting
builtins/src/test/java/org/jline/builtins/TerminalTest.java- Comprehensive test suitebuiltins/src/test/java/org/jline/builtins/SimpleTerminalTest.java- Basic functionality testsDocumentation
website/docs/advanced/web-swing-terminals.md- Complete user documentation with examplesFiles Modified
builtins/src/main/java/org/jline/builtins/ScreenTerminal.java- AddedgetWidth()andgetHeight()public methodswebsite/docs/modules/builtins.md- Added section about terminal implementationswebsite/sidebars.ts- Added new documentation page to navigationUsage Examples
WebTerminal
SwingTerminal
Demo Applications
The
TerminalDemoclass provides several demo modes:web- Web terminal demoswing- Basic Swing terminal demoadvanced- Advanced Swing terminal with menu bar and font selectionboth- Both terminals running simultaneouslyTesting
Backward Compatibility
ScreenTerminal(new public getter methods)Performance
Future Enhancements
Potential improvements identified for future versions:
Documentation
Comprehensive documentation has been added to the website including:
This feature significantly expands JLine's capabilities, enabling it to be used in web applications and desktop GUI applications while maintaining the same powerful terminal functionality that JLine is known for.
Pull Request opened by Augment Code with guidance from the PR author