jdaugherty commented on code in PR #15367:
URL: https://github.com/apache/grails-core/pull/15367#discussion_r3061985352
##########
grails-bootstrap/src/main/groovy/org/grails/build/interactive/CandidateListCompletionHandler.java:
##########
@@ -18,76 +18,77 @@
*/
package org.grails.build.interactive;
-import java.io.IOException;
import java.util.List;
-import jline.console.ConsoleReader;
-import jline.console.CursorBuffer;
-import jline.console.completer.CompletionHandler;
+import org.jline.reader.Candidate;
+import org.jline.reader.Completer;
+import org.jline.reader.LineReader;
+import org.jline.reader.ParsedLine;
/**
- * Fixes issues with the default CandidateListCompletionHandler such as
clearing out the whole buffer when
- * a completion matches a list of candidates
+ * A Completer implementation that wraps candidate list completion behavior.
+ * In JLine 3, completion handling is integrated into the LineReader itself,
+ * so this class now acts as a utility completer that can be composed with
others.
*
* @author Graeme Rocher
* @since 2.0
*/
-public class CandidateListCompletionHandler implements CompletionHandler {
+public class CandidateListCompletionHandler implements Completer {
- private boolean eagerNewlines = true;
+ private final Completer delegate;
- public void setAlwaysIncludeNewline(boolean eagerNewlines) {
- this.eagerNewlines = eagerNewlines;
+ public CandidateListCompletionHandler() {
+ this.delegate = null;
}
- public boolean complete(ConsoleReader reader,
@SuppressWarnings("rawtypes") List<CharSequence> candidates, int pos) throws
IOException {
- CursorBuffer buf = reader.getCursorBuffer();
-
- // if there is only one completion, then fill in the buffer
- if (candidates.size() == 1) {
- String value = candidates.get(0).toString();
-
- // fail if the only candidate is the same as the current buffer
- if (value.equals(buf.toString())) {
- return false;
- }
-
-
jline.console.completer.CandidateListCompletionHandler.setBuffer(reader, value,
pos);
+ public CandidateListCompletionHandler(Completer delegate) {
+ this.delegate = delegate;
+ }
- return true;
+ @Override
+ public void complete(LineReader reader, ParsedLine line, List<Candidate>
candidates) {
+ if (delegate != null) {
+ delegate.complete(reader, line, candidates);
}
- if (candidates.size() > 1) {
- String value = getUnambiguousCompletions(candidates);
-
-
jline.console.completer.CandidateListCompletionHandler.setBuffer(reader, value,
pos);
+ if (reader == null) {
+ return;
}
- if (eagerNewlines) {
- reader.println();
+ String commonPrefix = getUnambiguousCompletions(candidates);
+ if (commonPrefix == null) {
+ return;
}
-
jline.console.completer.CandidateListCompletionHandler.printCandidates(reader,
candidates);
- // redraw the current console buffer
- reader.drawLine();
+ String current = line != null ? line.word() : "";
+ if (current == null) {
+ current = "";
Review Comment:
In JLine 3, `Completer.complete()` is expected to contribute `Candidate`s
and let `LineReader` handle buffer updates. Writing `suffix` directly into
`reader.getBuffer()` and forcing a redraw here bypasses that flow and can lead
to double insertion or redraw glitches when the reader applies completion
itself.
Can this class be reduced to candidate production only, with the
common-prefix behavior handled by JLine's normal completion machinery?
##########
grails-shell-cli/src/main/groovy/org/grails/cli/GrailsCli.groovy:
##########
@@ -625,17 +622,29 @@ class GrailsCli {
protected Boolean bang(ExecutionContext context) {
def console = context.console
- def history = console.reader.history
+ def history = console.history
- //move one step back to !
- history.previous()
+ if (history == null || history.size() == 0) {
+ console.error('! not valid. Can not repeat without history')
+ return false
+ }
+
+ // Get previous command from history
+ def historyIterator = history.reverseIterator()
+ if (!historyIterator.hasNext()) {
+ console.error('! not valid. Can not repeat without history')
+ return false
+ }
- if (!history.previous()) {
+ // Skip the current '!' command
+ historyIterator.next()
+
+ if (!historyIterator.hasNext()) {
console.error('! not valid. Can not repeat without history')
+ return false
}
Review Comment:
Potential `!` regression: this unconditionally skips the newest history
entry, assuming the current `!` command has already been persisted. If JLine 3
has not added `!` to history at this point, `foo`, `bar`, `!` will replay `foo`
instead of `bar`.
Worth adding a regression test for that exact sequence.
##########
grails-bootstrap/src/main/groovy/grails/build/logging/GrailsConsole.java:
##########
@@ -190,25 +190,31 @@ protected void initialize(InputStream systemIn,
PrintStream systemOut, PrintStre
redirectSystemOutAndErr(true);
- System.setProperty(ShutdownHooks.JLINE_SHUTDOWNHOOK, "false");
-
if (isInteractiveEnabled()) {
Review Comment:
Behavior change from `8.0.x`: interactive mode now always calls
`createTerminal()`, so `grails.console.enable.terminal=false` is no longer
honored on the interactive path. If that property is still meant to disable
terminal activation in headless or embedded environments, this branch probably
needs to stay conditional.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/SbomPlugin.groovy:
##########
@@ -90,7 +90,8 @@ class SbomPlugin implements Plugin<Project> {
private static Map<String, String> LICENSE_MAPPING = [
'pkg:maven/org.antlr/[email protected]?type=jar'
: 'BSD-3-Clause', // maps incorrectly because of
https://github.com/CycloneDX/cyclonedx-core-java/issues/205
'pkg:maven/jline/[email protected]?type=jar'
: 'BSD-2-Clause', // maps incorrectly because of
https://github.com/CycloneDX/cyclonedx-core-java/issues/205
- 'pkg:maven/org.jline/[email protected]?type=jar'
: 'BSD-2-Clause', // maps incorrectly because of
https://github.com/CycloneDX/cyclonedx-core-java/issues/205
+ 'pkg:maven/org.jline/[email protected]?type=jar'
: 'BSD-3-Clause', // maps incorrectly because of
https://github.com/CycloneDX/cyclonedx-core-java/issues/205
Review Comment:
Why do we have 2 different 3.x versions? Shouldn't there only be one.
##########
grails-shell-cli/src/main/groovy/org/grails/cli/GrailsCli.groovy:
##########
@@ -434,49 +430,50 @@ class GrailsCli {
// CTRL-D was pressed, exit interactive mode
exitInteractiveMode()
} else if (commandLine.trim()) {
- if (nonBlockingInput.isNonBlockingEnabled()) {
- handleCommandWithCancellationSupport(console,
commandLine, commandExecutor, nonBlockingInput)
- } else {
- handleCommand(cliParser.parseString(commandLine))
- }
+ handleCommandWithCancellationSupport(console, commandLine,
commandExecutor)
}
} catch (BuildCancelledException cancelledException) {
console.updateStatus('Build stopped.')
} catch (UserInterruptException e) {
exitInteractiveMode()
+ } catch (EndOfFileException e) {
+ exitInteractiveMode()
} catch (Throwable e) {
console.error("Caught exception ${e.message}", e)
}
}
}
- private Boolean handleCommandWithCancellationSupport(GrailsConsole
console, String commandLine, ExecutorService commandExecutor,
NonBlockingInputStream nonBlockingInput) {
+ private Boolean handleCommandWithCancellationSupport(GrailsConsole
console, String commandLine, ExecutorService commandExecutor) {
ExecutionContext executionContext =
createExecutionContext(cliParser.parseString(commandLine))
Future<?> commandFuture = commandExecutor.submit({
handleCommand(executionContext) } as Callable<Boolean>)
- def terminal = console.reader.terminal
- if (terminal instanceof UnixTerminal) {
- ((UnixTerminal) terminal).disableInterruptCharacter()
- }
+
+ Terminal.SignalHandler previousHandler = null
try {
Review Comment:
Regression from `8.0.x`: the old loop canceled on both Ctrl+C and Esc
(`KEYPRESS_CTRL_C` and `KEYPRESS_ESC`), but this version only hooks
`Terminal.Signal.INT`. That means Esc no longer stops a running interactive
command.
If Esc cancellation is still intended, this path needs equivalent handling
or a regression test documenting the 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
