feat: change default of softwareSignals to false and deprecate#2020
feat: change default of softwareSignals to false and deprecate#2020gnodet wants to merge 4 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR deprecates ChangesSoftware Signals Default Change
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR changes the default behavior of JLine’s system terminals regarding software interception of signal control characters by making org.jline.terminal.softwareSignals default to false, and it deprecates the property to guide applications toward native/raw-byte handling (or LineReader’s INTERRUPT widget) going forward.
Changes:
- Changed the default value for
PROP_SOFTWARE_SIGNALSfrom"true"to"false"inAbstractUnixSysTerminalandPosixSysTerminal. - Deprecated
TerminalBuilder.PROP_SOFTWARE_SIGNALSwith updated Javadoc explaining the migration path. - Added a regression test verifying that when the property is unset,
0x03is delivered as a byte and does not raiseSignal.INT.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| terminal/src/test/java/org/jline/terminal/impl/PosixSysTerminalTest.java | Adds a new test to confirm the default disables software signal interception when the property is unset. |
| terminal/src/main/java/org/jline/terminal/TerminalBuilder.java | Deprecates PROP_SOFTWARE_SIGNALS and documents the new default and migration guidance. |
| terminal/src/main/java/org/jline/terminal/impl/PosixSysTerminal.java | Switches the softwareSignals default from "true" to "false" when reading the system property. |
| terminal/src/main/java/org/jline/terminal/impl/AbstractUnixSysTerminal.java | Switches the softwareSignals default from "true" to "false" when reading the system property. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
terminal/src/main/java/org/jline/terminal/TerminalBuilder.java (1)
180-202: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider
forRemoval = trueon@Deprecated.The Javadoc states removal is planned, but the annotation itself doesn't set
forRemoval. Using@Deprecated(since = "4.4.0", forRemoval = true)makes this machine-checkable (IDEs/build tools flag call sites more prominently) rather than relying solely on prose.♻️ Suggested tweak
- `@Deprecated` + `@Deprecated`(since = "4.4.0", forRemoval = true) `@SuppressWarnings`("java:S1133") // Intentional deprecation; removal planned for a future major version public static final String PROP_SOFTWARE_SIGNALS = "org.jline.terminal.softwareSignals";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@terminal/src/main/java/org/jline/terminal/TerminalBuilder.java` around lines 180 - 202, The deprecation on PROP_SOFTWARE_SIGNALS should be made machine-checkable to match the Javadoc’s removal notice. Update the `@Deprecated` annotation on TerminalBuilder.PROP_SOFTWARE_SIGNALS to include since and forRemoval metadata so tools and IDEs flag usage more strongly, while keeping the existing warning text and suppression unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@terminal/src/main/java/org/jline/terminal/TerminalBuilder.java`:
- Around line 180-202: The deprecation on PROP_SOFTWARE_SIGNALS should be made
machine-checkable to match the Javadoc’s removal notice. Update the `@Deprecated`
annotation on TerminalBuilder.PROP_SOFTWARE_SIGNALS to include since and
forRemoval metadata so tools and IDEs flag usage more strongly, while keeping
the existing warning text and suppression unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f10364da-1771-46a4-ab36-c0b9d3e8fae0
📒 Files selected for processing (4)
terminal/src/main/java/org/jline/terminal/TerminalBuilder.javaterminal/src/main/java/org/jline/terminal/impl/AbstractUnixSysTerminal.javaterminal/src/main/java/org/jline/terminal/impl/PosixSysTerminal.javaterminal/src/test/java/org/jline/terminal/impl/PosixSysTerminalTest.java
There was a problem hiding this comment.
🧹 Nitpick comments (1)
demo/src/main/java/org/jline/demo/examples/RawModeExample.java (1)
49-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: consolidate loop exit points (SonarCloud S135).
The loop now has two
breakstatements (interrupt byte andq/Q). SonarCloud flags this as a maintainability smell. Not a compiler warning, so it won't trip-Werror, but consolidating avoids a new code-smell finding.♻️ Optional consolidation
- while (true) { + boolean quit = false; + while (!quit) { int c = reader.read(100); if (c != -1) { if (c == intrChar) { terminal.writer().println("Interrupt received (Ctrl+C) — exiting."); terminal.writer().flush(); - break; + quit = true; + continue; } terminal.writer().println("Read: " + (char) c + " (ASCII: " + c + ")"); terminal.writer().flush(); if (c == 'q' || c == 'Q') { - break; + quit = true; } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@demo/src/main/java/org/jline/demo/examples/RawModeExample.java` around lines 49 - 65, The while loop in RawModeExample’s main read loop has multiple exit points that trigger SonarCloud S135. Consolidate the loop termination logic in the read loop around the reader.read(100) handling so there is only one break path, using the existing intrChar and q/Q checks to set a single exit condition before breaking.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@demo/src/main/java/org/jline/demo/examples/RawModeExample.java`:
- Around line 49-65: The while loop in RawModeExample’s main read loop has
multiple exit points that trigger SonarCloud S135. Consolidate the loop
termination logic in the read loop around the reader.read(100) handling so there
is only one break path, using the existing intrChar and q/Q checks to set a
single exit condition before breaking.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d9849059-3dce-4816-9052-540580f7b802
📒 Files selected for processing (2)
demo/src/main/java/org/jline/demo/examples/RawModeExample.javaterminal/src/main/java/org/jline/terminal/TerminalBuilder.java
🚧 Files skipped from review as they are similar to previous changes (1)
- terminal/src/main/java/org/jline/terminal/TerminalBuilder.java
|
Fix #1960
Changes the default of
org.jline.terminal.softwareSignalsfromtruetofalseand deprecates the property for eventual removal.Changes
Default changed:
PROP_SOFTWARE_SIGNALSnow defaults tofalsein bothAbstractUnixSysTerminalandPosixSysTerminal.Property deprecated:
PROP_SOFTWARE_SIGNALSis marked@Deprecated(since = "4.4.0", forRemoval = true)with Javadoc explaining the migration paths.Test added:
testDefaultSoftwareSignalsDisabledverifies that when the property is unset, no software signal interception occurs (the VINTR byte is delivered without raisingSignal.INT).RawModeExample updated: demonstrates how to handle Ctrl+C in a raw-mode input loop by reading the VINTR character from terminal attributes via
getControlChar(ControlChar.VINTR).Migration
Applications that relied on
terminal.handle(Signal.INT, ...)in raw mode should migrate to one of:-Dorg.jline.terminal.softwareSignals=truerestores the old behavior temporarily.UserInterruptExceptionfromreadLine()(seeSimpleLineReadingExample).RawModeExample).Summary by CodeRabbit