GROOVY-7567: groovysh: Fix file completion with blanks, use escaping …

…instead of hyphens (closes apache#92)

Trying to close hyphens leads to complex/buggy code. As an example, completing:
```:load "foo|```
with candidate file 'foo bar' leads to
```:load "'foo bar'```
because jline does not pass the initial hyphen to the FileNameCompleter.

LICENSE

@@ -218,7 +218,7 @@ The following class within this product:
  org.codehaus.groovy.tools.shell.completion.FileNameCompleter
 
 was derived from JLine 2.12, and the following patch:
-https://github.com/jline/jline2/issues/90
+https://github.com/jline/jline2/pull/204
 JLine2 is made available under a BSD License.
 For details, see licenses/jline2-license.
 

subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/commands/LoadCommand.groovy

@@ -41,7 +41,7 @@ class LoadCommand
 
  @Override
  protected List<Completer> createCompleters() {
- return [ new FileNameCompleter(true, true) ]
+ return [ new FileNameCompleter(true) ]
  }
 
  @Override

subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/completion/FileNameCompleter.groovy

@@ -36,10 +36,14 @@ import static jline.internal.Preconditions.checkNotNull
 /**
  * PATCHED copy from jline 2.12, with
  * https://github.com/jline/jline2/issues/90 (no trailing blank)
+ * https://github.com/jline/jline2/pull/204
+ *
  * NOTE: we hope to work with the jline project to have this functionality
  * absorbed into a future jline release and then remove this file, so keep
  * that in mind if you are thinking of changing this file.
- *
+ */
+
+ /**
  * A file name completer takes the buffer and issues a list of
  * potential completions.
  * <p/>
@@ -62,14 +66,10 @@ import static jline.internal.Preconditions.checkNotNull
 public class FileNameCompleter
 implements Completer
 {
- // TODO: Handle files with spaces in them
-
  private static final boolean OS_IS_WINDOWS;
 
  private boolean printSpaceAfterFullCompletion = true;
 
- private boolean handleLeadingHyphen = false;
-
  public boolean getPrintSpaceAfterFullCompletion() {
  return printSpaceAfterFullCompletion;
  }
@@ -91,15 +91,9 @@ implements Completer
  }
 
 
- public FileNameCompleter(boolean blankSuffix, boolean handleLeadingHyphen) {
- this(blankSuffix)
- this.handleLeadingHyphen = handleLeadingHyphen
- }
-
  public int complete(String buffer, final int cursor, final List<CharSequence> candidates) {
  // buffer can be null
  checkNotNull(candidates);
- String hyphenChar = null;
 
  if (buffer == null) {
  buffer = "";
@@ -110,10 +104,6 @@ implements Completer
  }
 
  String translated = buffer;
- if (handleLeadingHyphen && (translated.startsWith('\'') || translated.startsWith('"'))) {
- hyphenChar = translated[0];
- translated = translated.substring(1);
- }
 
  // Special character: ~ maps to the user's home directory in most OSs
  if (!OS_IS_WINDOWS && translated.startsWith("~")) {
@@ -142,7 +132,7 @@ implements Completer
 
  File[] entries = (dir == null) ? new File[0] : dir.listFiles();
 
- return matchFiles(buffer, translated, entries, candidates, hyphenChar);
+ return matchFiles(buffer, translated, entries, candidates);
  }
 
  protected static String separator() {
@@ -153,36 +143,31 @@ implements Completer
  return Configuration.getUserHome();
  }
 
- protected static File getUserDir() {
+ /*
+ * non static for testing
+ */
+ protected File getUserDir() {
  return new File(".");
  }
 
- protected int matchFiles(final String buffer, final String translated, final File[] files, final List<CharSequence> candidates, final String hyphenChar) {
+ protected int matchFiles(final String buffer, final String translated, final File[] files, final List<CharSequence> candidates) {
  if (files == null) {
  return -1;
  }
 
- int matches = 0;
-
- // first pass: just count the matches
- for (File file : files) {
- if (file.getAbsolutePath().startsWith(translated)) {
- matches++;
- }
- }
  for (File file : files) {
  if (file.getAbsolutePath().startsWith(translated)) {
- CharSequence name = file.getName()
- if (matches == 1) {
- if (file.isDirectory()) {
- name += separator();
- } else {
- if (printSpaceAfterFullCompletion && !hyphenChar) {
- name += ' ';
- }
+ CharSequence name = file.getName();
+ String renderedName = render(name).toString();
+ if (file.isDirectory()) {
+ renderedName += separator();
+ } else {
+ if (printSpaceAfterFullCompletion) {
+ renderedName += ' '
  }
  }
- candidates.add(render(name, hyphenChar).toString());
+
+ candidates.add(renderedName);
  }
  }
 
@@ -191,18 +176,21 @@ implements Completer
  return index + separator().length();
  }
 
- protected CharSequence render(final CharSequence name, final String hyphenChar) {
- if (hyphenChar != null) {
- return escapedNameInHyphens(name, hyphenChar);
- }
- if (name.contains(' ')) {
- return escapedNameInHyphens(name, '\'');
- }
- return name;
+ /**
+ * @param name
+ * @param hyphenChar force hyphenation with this if not null
+ * @return name in hyphens if it contains a blank
+ */
+ protected static CharSequence render(final CharSequence name) {
+ return escapedName(name);
  }
 
- private String escapedNameInHyphens(final CharSequence name, String hyphen) {
- // need to escape every instance of chartoEscape, and every instance of the escape char backslash
- return hyphen + name.toString().replace('\\', '\\\\').replace(hyphen, '\\' + hyphen) + hyphen
+ /**
+ *
+ * @return name in hyphens Strings with hyphens and backslashes escaped
+ */
+ private static String escapedName(final CharSequence name) {
+ // Escape blanks, hyphens and escape characters
+ return name.toString().replace('\\', '\\\\').replace('"', '\\"').replace('\'', '\\\'').replace(' ', '\\ ')
  }
 }

subprojects/groovy-groovysh/src/test/groovy/org/codehaus/groovy/tools/shell/completion/FileNameCompleterTest.groovy

@@ -18,24 +18,48 @@
  */
 package org.codehaus.groovy.tools.shell.completion
 
+import org.junit.rules.TemporaryFolder
+
 class FileNameCompleterTest extends GroovyTestCase {
 
  void testRender() {
- FileNameCompleter completer = new FileNameCompleter()
- assert completer.render('foo', null) == 'foo'
- assert completer.render('foo bar', null) == '\'foo bar\''
- assert completer.render('foo \'bar', null) == '\'foo \\\'bar\''
- assert completer.render('foo \'bar', '\'') == '\'foo \\\'bar\''
- assert completer.render('foo " \'bar', '"') == '"foo \\" \'bar"'
+ assert FileNameCompleter.render('foo') == 'foo'
+ assert FileNameCompleter.render('foo bar') == 'foo\\ bar'
+ // intentionally adding empty String, to get better power assert output
+ assert FileNameCompleter.render('foo \'\"bar') == 'foo\\ \\\'\\\"bar' + ''
+ }
+
+ void testCompletionNoFiles() {
+ // abusing junit testrule
+ TemporaryFolder testFolder = null;
+ try {
+ testFolder = new TemporaryFolder();
+ testFolder.create()
+
+ FileNameCompleter completor = new FileNameCompleter() {
+ @Override
+ protected File getUserDir() {
+ return testFolder.getRoot()
+ }
+ }
+ def candidates = []
+ String buffer = ''
+ assert 0 == completor.complete(buffer, 0, candidates)
+ assert [] == candidates
+ } finally {
+ if (testFolder != null) {
+ testFolder.delete()
+ }
+ }
  }
 
  void testMatchFiles_Unix() {
  if(! System.getProperty('os.name').startsWith('Windows')) {
  FileNameCompleter completer = new FileNameCompleter()
  List<String> candidates = []
- int resultIndex = completer.matchFiles('foo/bar', '/foo/bar', [new File('/foo/baroo'), new File('/foo/barbee')] as File[], candidates, null)
+ int resultIndex = completer.matchFiles('foo/bar', '/foo/bar', [new File('/foo/baroo'), new File('/foo/barbee')] as File[], candidates)
  assert resultIndex == 'foo/'.length()
- assert candidates == ['baroo', 'barbee']
+ assert candidates == ['baroo ', 'barbee ']
  }
  }
 }