From 3d39ef3518baad98e047e42494905015e67d2a6c Mon Sep 17 00:00:00 2001 From: Janne Valkealahti Date: Mon, 12 Jun 2023 10:12:52 +0100 Subject: [PATCH] Discard empty args - Take 2 of #763 - Revert changes in 06e89dcca38bc7e99cec90d04528e6421c222618 what comes for `ExtendedDefaultParser`. - Discard empty args in a `Shell` coming from `ExtendedDefaultParser` - `ExtendedDefaultParserTests` has more tests, some commented out, to see some differences for jline default parser impl. Something to get handled in #517 --- .../java/org/springframework/shell/Shell.java | 4 +- .../shell/jline/ExtendedDefaultParser.java | 6 +- .../jline/ExtendedDefaultParserTests.java | 92 +++++++++++++++---- 3 files changed, 77 insertions(+), 25 deletions(-) diff --git a/spring-shell-core/src/main/java/org/springframework/shell/Shell.java b/spring-shell-core/src/main/java/org/springframework/shell/Shell.java index 34b6ddbd..400f539b 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/Shell.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/Shell.java @@ -195,7 +195,9 @@ public class Shell { return NO_INPUT; } - List words = input.words(); + // List words = input.words(); + // gh-763 + List words = input.words().stream().filter(w -> w.length() > 0).collect(Collectors.toList()); String line = words.stream().collect(Collectors.joining(" ")).trim(); String command = findLongestCommand(line, false); diff --git a/spring-shell-core/src/main/java/org/springframework/shell/jline/ExtendedDefaultParser.java b/spring-shell-core/src/main/java/org/springframework/shell/jline/ExtendedDefaultParser.java index aa783c1f..cf4f1b72 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/jline/ExtendedDefaultParser.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/jline/ExtendedDefaultParser.java @@ -126,12 +126,10 @@ public class ExtendedDefaultParser implements Parser { } if (current.length() > 0 || (line != null && cursor == line.length())) { - if (current.length() > 0) { - words.add(current.toString()); - } + words.add(current.toString()); } - if (line != null && cursor == line.length() && words.size() > 0) { + if (line != null && cursor == line.length()) { wordIndex = words.size() - 1; wordCursor = words.get(words.size() - 1).length(); } diff --git a/spring-shell-core/src/test/java/org/springframework/shell/jline/ExtendedDefaultParserTests.java b/spring-shell-core/src/test/java/org/springframework/shell/jline/ExtendedDefaultParserTests.java index aa60282c..38ad5f62 100644 --- a/spring-shell-core/src/test/java/org/springframework/shell/jline/ExtendedDefaultParserTests.java +++ b/spring-shell-core/src/test/java/org/springframework/shell/jline/ExtendedDefaultParserTests.java @@ -15,38 +15,90 @@ */ package org.springframework.shell.jline; -import org.junit.jupiter.api.Test; +import java.util.stream.Stream; + +import org.jline.reader.ParsedLine; +import org.jline.reader.impl.DefaultParser; +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; public class ExtendedDefaultParserTests { - @Test - void wordsParsing() { - ExtendedDefaultParser parser = new ExtendedDefaultParser(); + private final ExtendedDefaultParser springParser = new ExtendedDefaultParser(); + private final DefaultParser jlineParser = new DefaultParser(); - assertThat(parser.parse("one", 0).words()).hasSize(1); - assertThat(parser.parse("one", 3).words()).hasSize(1); + static Stream args() { + // cursor words wordIndex wordCursor line + // We've left these tests here to show issues + // differences between DefaultParser and + // ExtendedDefaultParser. relates gh517 + return Stream.of( + Arguments.of(0, 1, 0, 0, "one"), + Arguments.of(3, 1, 0, 3, "one "), + Arguments.of(4, 2, 1, 0, "one "), - assertThat(parser.parse("one two", 0).words()).hasSize(2); - assertThat(parser.parse("one two", 7).words()).hasSize(2); + Arguments.of(0, 2, 0, 0, "one two"), + Arguments.of(1, 2, 0, 1, "one two"), + Arguments.of(2, 2, 0, 2, "one two"), + Arguments.of(3, 2, 0, 3, "one two"), + Arguments.of(7, 2, 1, 3, "one two"), + Arguments.of(7, 2, 1, 3, "one two "), + Arguments.of(8, 3, 2, 0, "one two "), - assertThat(parser.parse("'one'", 0).words()).hasSize(1); - assertThat(parser.parse("'one'", 5).words()).hasSize(1); + Arguments.of(0, 1, 0, 0, "'one'"), + // Arguments.of(5, 1, 0, 3, "'one' "), + Arguments.of(6, 2, 1, 0, "'one' "), - assertThat(parser.parse("'one' two", 0).words()).hasSize(2); - assertThat(parser.parse("'one' two", 9).words()).hasSize(2); + Arguments.of(0, 1, 0, 0, "'one'"), + Arguments.of(1, 1, 0, 0, "'one'"), + Arguments.of(2, 1, 0, 1, "'one'"), + Arguments.of(3, 1, 0, 2, "'one'"), + Arguments.of(4, 1, 0, 3, "'one'"), + // Arguments.of(5, 1, 0, 3, "'one'"), - assertThat(parser.parse("one 'two'", 0).words()).hasSize(2); - assertThat(parser.parse("one 'two'", 9).words()).hasSize(2); + Arguments.of(0, 1, 0, 0, "'one' "), + Arguments.of(1, 1, 0, 0, "'one' "), + Arguments.of(2, 1, 0, 1, "'one' "), + Arguments.of(3, 1, 0, 2, "'one' "), + Arguments.of(4, 1, 0, 3, "'one' "), + // Arguments.of(5, 1, 0, 3, "'one' "), + Arguments.of(6, 2, 1, 0, "'one' "), - assertThat(parser.parse("\"one\"", 0).words()).hasSize(1); - assertThat(parser.parse("\"one\"", 5).words()).hasSize(1); + Arguments.of(0, 1, 0, 0, "\"one\""), + Arguments.of(1, 1, 0, 0, "\"one\""), + Arguments.of(2, 1, 0, 1, "\"one\""), + Arguments.of(3, 1, 0, 2, "\"one\""), + Arguments.of(4, 1, 0, 3, "\"one\""), + // Arguments.of(5, 1, 0, 3, "\"one\""), - assertThat(parser.parse("\"one\" two", 0).words()).hasSize(2); - assertThat(parser.parse("\"one\" two", 9).words()).hasSize(2); + Arguments.of(0, 1, 0, 0, "\"one\" "), + Arguments.of(1, 1, 0, 0, "\"one\" "), + Arguments.of(2, 1, 0, 1, "\"one\" "), + Arguments.of(3, 1, 0, 2, "\"one\" "), + Arguments.of(4, 1, 0, 3, "\"one\" "), + // Arguments.of(5, 1, 0, 3, "\"one\" "), + Arguments.of(6, 2, 1, 0, "\"one\" ") + ); + } - assertThat(parser.parse("one \"two\"", 0).words()).hasSize(2); - assertThat(parser.parse("one \"two\"", 9).words()).hasSize(2); + @ParameterizedTest + @MethodSource("args") + void testSpringExtendedDefaultParser(int cursor, int words, int wordIndex, int wordCursor, String line) { + ParsedLine parse = springParser.parse(line, cursor); + assertThat(parse.words()).as("words").hasSize(words); + assertThat(parse.wordIndex()).as("wordIndex").isEqualTo(wordIndex); + assertThat(parse.wordCursor()).as("wordCursor").isEqualTo(wordCursor); + } + + @ParameterizedTest + @MethodSource("args") + void testJlineDefaultParser(int cursor, int words, int wordIndex, int wordCursor, String line) { + ParsedLine parse = jlineParser.parse(line, cursor); + assertThat(parse.words()).as("words").hasSize(words); + assertThat(parse.wordIndex()).as("wordIndex").isEqualTo(wordIndex); + assertThat(parse.wordCursor()).as("wordCursor").isEqualTo(wordCursor); } }