From f045944b471dddb8ef5f2a159a21a22a30f9b892 Mon Sep 17 00:00:00 2001 From: arimu1 <19286898+arimu1@users.noreply.github.com> Date: Fri, 7 Aug 2026 23:02:34 +0700 Subject: [PATCH] fix: allow path tab-completion with trailing backslash on Windows Do not throw EOFError for escaped newlines (or unclosed quotes) when parsing in COMPLETE/SPLIT_LINE context, matching JLine DefaultParser. Also disable backslash escapes on Windows where '\' is the path separator, and implement CompletingParsedLine properly so completion candidates are escaped/quoted. Fixes #1169 Signed-off-by: arimu1 <19286898+arimu1@users.noreply.github.com> --- .../JLineShellAutoConfiguration.java | 7 + .../jline/DefaultJLineShellConfiguration.java | 7 + .../shell/jline/ExtendedDefaultParser.java | 148 +++++++----------- .../jline/ExtendedDefaultParserTests.java | 47 ++++++ 4 files changed, 120 insertions(+), 89 deletions(-) diff --git a/spring-shell-core-autoconfigure/src/main/java/org/springframework/shell/core/autoconfigure/JLineShellAutoConfiguration.java b/spring-shell-core-autoconfigure/src/main/java/org/springframework/shell/core/autoconfigure/JLineShellAutoConfiguration.java index a4039fb72..446c4072f 100644 --- a/spring-shell-core-autoconfigure/src/main/java/org/springframework/shell/core/autoconfigure/JLineShellAutoConfiguration.java +++ b/spring-shell-core-autoconfigure/src/main/java/org/springframework/shell/core/autoconfigure/JLineShellAutoConfiguration.java @@ -16,6 +16,7 @@ package org.springframework.shell.core.autoconfigure; +import java.io.File; import java.io.IOException; import java.nio.file.Path; import java.nio.file.Paths; @@ -211,6 +212,12 @@ public Parser parser() { ExtendedDefaultParser parser = new ExtendedDefaultParser(); parser.setEofOnUnclosedQuote(true); parser.setEofOnEscapedNewLine(true); + // Backslash is both the DefaultParser escape character and the Windows path + // separator. Disable escapes on Windows so path tab-completion works (see + // gh-1169 / gh-240). + if (File.separatorChar == '\\') { + parser.setEscapeChars(null); + } return parser; } diff --git a/spring-shell-jline/src/main/java/org/springframework/shell/jline/DefaultJLineShellConfiguration.java b/spring-shell-jline/src/main/java/org/springframework/shell/jline/DefaultJLineShellConfiguration.java index 8159a471d..5e3cb8bb1 100644 --- a/spring-shell-jline/src/main/java/org/springframework/shell/jline/DefaultJLineShellConfiguration.java +++ b/spring-shell-jline/src/main/java/org/springframework/shell/jline/DefaultJLineShellConfiguration.java @@ -1,5 +1,6 @@ package org.springframework.shell.jline; +import java.io.File; import java.io.IOException; import org.jline.reader.LineReader; @@ -53,6 +54,12 @@ public Parser parser() { ExtendedDefaultParser parser = new ExtendedDefaultParser(); parser.setEofOnUnclosedQuote(true); parser.setEofOnEscapedNewLine(true); + // Backslash is both the DefaultParser escape character and the Windows path + // separator. Disable escapes on Windows so path tab-completion works (see + // gh-1169 / gh-240). + if (File.separatorChar == '\\') { + parser.setEscapeChars(null); + } return parser; } diff --git a/spring-shell-jline/src/main/java/org/springframework/shell/jline/ExtendedDefaultParser.java b/spring-shell-jline/src/main/java/org/springframework/shell/jline/ExtendedDefaultParser.java index 046672bce..ed91f352d 100644 --- a/spring-shell-jline/src/main/java/org/springframework/shell/jline/ExtendedDefaultParser.java +++ b/spring-shell-jline/src/main/java/org/springframework/shell/jline/ExtendedDefaultParser.java @@ -22,6 +22,7 @@ import java.util.Objects; import java.util.function.Predicate; +import org.jline.reader.CompletingParsedLine; import org.jline.reader.EOFError; import org.jline.reader.ParsedLine; import org.jline.reader.Parser; @@ -135,16 +136,20 @@ else if (!isEscapeChar(line, i)) { wordCursor = words.get(words.size() - 1).length(); } - if (eofOnEscapedNewLine && (line != null && isEscapeChar(line, line.length() - 1))) { - throw new EOFError(-1, -1, "Escaped new line", "newline"); - } - if (eofOnUnclosedQuote && quoteStart >= 0 && context != ParseContext.COMPLETE) { - throw new EOFError(-1, -1, "Missing closing quote", - (line != null && line.charAt(quoteStart) == '\'') ? "quote" : "dquote"); + // Match JLine DefaultParser: never treat incomplete lines as EOF during + // completion or split-line (e.g. Windows paths ending with '\'). + if (context != ParseContext.COMPLETE && context != ParseContext.SPLIT_LINE) { + if (eofOnEscapedNewLine && line != null && !line.isEmpty() && isEscapeChar(line, line.length() - 1)) { + throw new EOFError(-1, -1, "Escaped new line", "newline"); + } + if (eofOnUnclosedQuote && quoteStart >= 0) { + throw new EOFError(-1, -1, "Missing closing quote", + (line != null && line.charAt(quoteStart) == '\'') ? "quote" : "dquote"); + } } String openingQuote = (quoteStart >= 0 && line != null) ? line.substring(quoteStart, quoteStart + 1) : null; - return wrap(new ExtendedArgumentList(line, words, wordIndex, wordCursor, cursor, openingQuote)); + return new ExtendedArgumentList(line, words, wordIndex, wordCursor, cursor, openingQuote); } /** @@ -226,7 +231,7 @@ public boolean isDelimiterChar(CharSequence buffer, int pos) { * * @author Marc Prud'hommeaux */ - public class ExtendedArgumentList implements ParsedLine, CompletingParsedLine { + public class ExtendedArgumentList implements CompletingParsedLine { private final String line; @@ -279,37 +284,65 @@ public String line() { } @Override - public CharSequence emit(CharSequence candidate) { + public CharSequence escape(CharSequence candidate, boolean complete) { StringBuilder sb = new StringBuilder(candidate); Predicate needToBeEscaped; + String quote = openingQuote; // Completion is protected by an opening quote: // Delimiters (spaces) don't need to be escaped, nor do other quotes, but // everything else does. // Also, close the quote at the end - if (openingQuote != null) { - needToBeEscaped = i -> isRawEscapeChar(sb.charAt(i)) - || String.valueOf(sb.charAt(i)).equals(openingQuote); - } // No quote protection, need to escape everything: delimiter chars (spaces), - // quote chars - // and escapes themselves - else { - needToBeEscaped = i -> isDelimiterChar(sb, i) || isRawEscapeChar(sb.charAt(i)) - || isRawQuoteChar(sb.charAt(i)); + if (escapeChars != null && escapeChars.length > 0) { + if (openingQuote != null) { + needToBeEscaped = i -> isRawEscapeChar(sb.charAt(i)) + || String.valueOf(sb.charAt(i)).equals(openingQuote); + } + // No quote protection, need to escape everything: delimiter chars + // (spaces), quote chars and escapes themselves + else { + needToBeEscaped = i -> isDelimiterChar(sb, i) || isRawEscapeChar(sb.charAt(i)) + || isRawQuoteChar(sb.charAt(i)); + } + for (int i = 0; i < sb.length(); i++) { + if (needToBeEscaped.test(i)) { + sb.insert(i++, escapeChars[0]); + } + } } - for (int i = 0; i < sb.length(); i++) { - if (needToBeEscaped.test(i)) { - sb.insert(i++, escapeChars[0]); + else if (openingQuote == null) { + // Without escape characters, quote candidates that contain delimiters + for (int i = 0; i < sb.length(); i++) { + if (isDelimiterChar(sb, i)) { + quote = "'"; + break; + } } } - if (openingQuote != null) { - sb.append(openingQuote); + if (quote != null) { + sb.insert(0, quote); + if (complete) { + sb.append(quote); + } } return sb; } + @Override + public int rawWordCursor() { + return wordCursor(); + } + + @Override + public int rawWordLength() { + return word().length(); + } + } private boolean isRawEscapeChar(char key) { + if (escapeChars == null) { + return false; + } for (char e : escapeChars) { if (e == key) { return true; @@ -319,6 +352,9 @@ private boolean isRawEscapeChar(char key) { } private boolean isRawQuoteChar(char key) { + if (quoteChars == null) { + return false; + } for (char e : quoteChars) { if (e == key) { return true; @@ -327,70 +363,4 @@ private boolean isRawQuoteChar(char key) { return false; } - /** - * Another copy from JLine's {@link org.jline.reader.impl.LineReaderImpl} - * - * Used to wrap {@link org.jline.reader.ParsedLine} into - * {@link org.jline.reader.CompletingParsedLine} - */ - private static org.jline.reader.CompletingParsedLine wrap(ParsedLine line) { - if (line instanceof org.jline.reader.CompletingParsedLine) { - return (org.jline.reader.CompletingParsedLine) line; - } - else { - return new org.jline.reader.CompletingParsedLine() { - public String word() { - return line.word(); - } - - public int wordCursor() { - return line.wordCursor(); - } - - public int wordIndex() { - return line.wordIndex(); - } - - public List words() { - return line.words(); - } - - public String line() { - return line.line(); - } - - public int cursor() { - return line.cursor(); - } - - public CharSequence escape(CharSequence candidate, boolean complete) { - return candidate; - } - - public int rawWordCursor() { - return wordCursor(); - } - - public int rawWordLength() { - return word().length(); - } - }; - } - } - - /** - * An extension of {@link ParsedLine} that, being aware of the quoting and escaping - * rules of the {@link Parser} that produced it, knows if and how a completion - * candidate should be escaped/quoted. - * - * @author Eric Bottard - * @author Piotr Olaszewski - */ - @FunctionalInterface - interface CompletingParsedLine { - - CharSequence emit(CharSequence candidate); - - } - } diff --git a/spring-shell-jline/src/test/java/org/springframework/shell/jline/ExtendedDefaultParserTests.java b/spring-shell-jline/src/test/java/org/springframework/shell/jline/ExtendedDefaultParserTests.java index 154b70f05..c776958ea 100644 --- a/spring-shell-jline/src/test/java/org/springframework/shell/jline/ExtendedDefaultParserTests.java +++ b/spring-shell-jline/src/test/java/org/springframework/shell/jline/ExtendedDefaultParserTests.java @@ -17,12 +17,18 @@ import java.util.stream.Stream; +import org.jline.reader.CompletingParsedLine; +import org.jline.reader.EOFError; import org.jline.reader.ParsedLine; +import org.jline.reader.Parser; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; class ExtendedDefaultParserTests { @@ -76,4 +82,45 @@ void testSpringExtendedDefaultParser(int cursor, int words, int wordIndex, int w assertThat(parse.wordCursor()).as("wordCursor").isEqualTo(wordCursor); } + @Test + void trailingBackslashDoesNotThrowOnCompleteWhenEofOnEscapedNewLine() { + // gh-1169 / gh-240: Windows path completion often ends with '\' + ExtendedDefaultParser parser = new ExtendedDefaultParser(); + parser.setEofOnEscapedNewLine(true); + String line = "pselect --path target\\"; + assertThatCode(() -> parser.parse(line, line.length(), Parser.ParseContext.COMPLETE)) + .doesNotThrowAnyException(); + } + + @Test + void trailingBackslashStillThrowsEofWhenAcceptingLine() { + ExtendedDefaultParser parser = new ExtendedDefaultParser(); + parser.setEofOnEscapedNewLine(true); + String line = "pselect --path target\\"; + assertThatThrownBy(() -> parser.parse(line, line.length(), Parser.ParseContext.ACCEPT_LINE)) + .isInstanceOf(EOFError.class) + .hasMessageContaining("Escaped new line"); + } + + @Test + void windowsStylePathsParseWithEscapesDisabled() { + // When escape chars are disabled (Windows configuration), backslashes are kept + // in words so multi-level path completion can continue. + ExtendedDefaultParser parser = new ExtendedDefaultParser(); + parser.setEofOnEscapedNewLine(true); + parser.setEscapeChars(null); + String line = "pselect --path target\\subdir\\"; + ParsedLine parsed = parser.parse(line, line.length(), Parser.ParseContext.COMPLETE); + assertThat(parsed.words()).containsExactly("pselect", "--path", "target\\subdir\\"); + assertThat(parsed.word()).isEqualTo("target\\subdir\\"); + } + + @Test + void parseResultImplementsCompletingParsedLine() { + ParsedLine parsed = springParser.parse("one two", 3, Parser.ParseContext.COMPLETE); + assertThat(parsed).isInstanceOf(CompletingParsedLine.class); + CompletingParsedLine completing = (CompletingParsedLine) parsed; + assertThat(completing.escape("a b", true).toString()).isEqualTo("a\\ b"); + } + }