paulrutter commented on code in PR #412: URL: https://github.com/apache/felix-dev/pull/412#discussion_r2067982427
########## commit-message.txt: ########## Review Comment: This can just be part of the commit message, doesn't have to be a git artifact ########## gogo/jline/src/main/java/org/apache/felix/gogo/jline/Posix.java: ########## @@ -822,11 +840,26 @@ public void exit() throws Exception { terminal.close(); } }; + + // Register a signal handler for INT signal to properly propagate interruption + Terminal.SignalHandler prevIntHandler = terminal.handle(Terminal.Signal.INT, signal -> { Review Comment: is `Terminal.Signal.INT` working cross-platform? E.g. also on Windows? I know Windows can sometimes be different in handling signals. ########## gogo/jline/src/main/java/org/apache/felix/gogo/jline/Posix.java: ########## @@ -808,9 +809,26 @@ private void startShell(CommandSession session, Terminal terminal) { new Thread(() -> runShell(session, terminal), terminal.getName() + " shell").start(); } + /** Review Comment: Agreed, see my previous comment ########## gogo/runtime/src/main/java/org/apache/felix/gogo/runtime/Pipe.java: ########## @@ -107,7 +107,12 @@ public static Pipe getCurrentPipe() { return CURRENT.get(); } - private static Pipe setCurrentPipe(Pipe pipe) { + /** + * Set the current pipe for the current thread. + * @param pipe the pipe to set as current, or null to clear + * @return the previous pipe + */ + public static Pipe setCurrentPipe(Pipe pipe) { Review Comment: rebase the PR on master to remove this change. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@felix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org