picocli icon indicating copy to clipboard operation
picocli copied to clipboard

"@Parameters" file name completion does not work

Open notwhoyouthinkiam opened this issue 5 years ago • 12 comments
trafficstars

Hello! I'm trying to create a command that accepts a file name, but no suggestion is shown after pressing tab, even if there is a file whose name starts with the partially typed string. This is the code I'm using:

    @Command(name = "mycmd1")
    static class MyCmd1 implements Callable<Void> {

        @Parameters(paramLabel = "FILE", arity = "1", description = "a file name")
        File file;

        @Override
        public Void call() {
            System.out.println(file.getAbsolutePath());
            return null;
        }

    }

If there's a file at /tmp/thisisafile.txt and I press tab after typing mycmd1 /tmp/thisis, shouldn't the completion occur? Am I missing something?

If I have an enum instead of a file, the completion works (mycmd2 f<TAB> gives mycmd2 foo) as expected:

    enum Something {
        foo, bar
    }

    @Command(name = "mycmd2")
    static class MyCmd2 implements Callable<Void> {

        @Parameters(paramLabel = "STH", arity = "1", description = "foo or bar")
        Something something;

        @Override
        public Void call() {
            System.out.println(something);
            return null;
        }

    }

Perhaps this is related to #762, but since new versions have been released since that issue, maybe things have changed.

Thank you!

notwhoyouthinkiam avatar Apr 15 '20 23:04 notwhoyouthinkiam

Hi @notwhoyouthinkiam, thank you for raising this issue!

Is this about JLine completion or bash completion? (You mentioned https://github.com/remkop/picocli/issues/762, so I am guessing JLine, but I would like to make sure...) Also, can you tell me which version of picocli and JLine you are using, and which OS you are testing on?

Finally, it may matter how things are wired up. Can you point me to a project or provide a small program that I can use to reproduce the issue?

remkop avatar Apr 16 '20 00:04 remkop

Hi, @remkop! Thank you for your reply.

It's about JLine completion (I'm sorry for not mentioning it before!). I'm using JLine 3.14.1 and Picocli 4.2.0 on Linux.

This is the full code of the minimal example I wrote before adding the issue:

import java.io.File;
import java.io.IOException;
import java.util.concurrent.Callable;

import org.jline.reader.LineReader;
import org.jline.reader.LineReaderBuilder;
import org.jline.reader.MaskingCallback;
import org.jline.reader.ParsedLine;
import org.jline.terminal.TerminalBuilder;

import picocli.CommandLine;
import picocli.CommandLine.Command;
import picocli.CommandLine.Parameters;
import picocli.shell.jline3.PicocliJLineCompleter;

@Command(subcommands = {
        App.MyCmd1.class, App.MyCmd2.class
})
public class App {

    @Command(name = "mycmd1")
    static class MyCmd1 implements Callable<Void> {

        @Parameters(paramLabel = "FILE", arity = "1", description = "a file name")
        File file;

        @Override
        public Void call() {
            System.out.println(file.getAbsolutePath());
            return null;
        }

    }

    enum Something {
        foo, bar
    }

    @Command(name = "mycmd2")
    static class MyCmd2 implements Callable<Void> {

        @Parameters(paramLabel = "STH", arity = "1", description = "foo or bar")
        Something something;

        @Override
        public Void call() {
            System.out.println(something);
            return null;
        }

    }

    public static void main(String[] args) throws IOException {
        App app = new App();
        CommandLine cl = new CommandLine(app);
        LineReader reader = LineReaderBuilder.builder().terminal(TerminalBuilder.builder().build())
                .completer((new PicocliJLineCompleter(cl.getCommandSpec()))).parser(null).build();

        while (true) {
            String line;
            line = reader.readLine(">>>>> ", null, (MaskingCallback) null, null).trim();
            ParsedLine pl = reader.getParser().parse(line, 0);
            String[] arguments = pl.words().toArray(new String[0]);
            new CommandLine(app).execute(arguments);
        }
    }
}

These are the dependencies at pom.xml:

		<dependency>
			<groupId>org.jline</groupId>
			<artifactId>jline</artifactId>
			<version>3.14.1</version>
		</dependency>
		<dependency>
			<groupId>org.jline</groupId>
			<artifactId>jline-reader</artifactId>
			<version>3.14.1</version>
		</dependency>
		<dependency>
			<groupId>org.jline</groupId>
			<artifactId>jline-terminal</artifactId>
			<version>3.14.1</version>
		</dependency>
		<dependency>
			<groupId>info.picocli</groupId>
			<artifactId>picocli</artifactId>
			<version>4.2.0</version>
		</dependency>
		<dependency>
			<groupId>info.picocli</groupId>
			<artifactId>picocli-shell-jline3</artifactId>
			<version>4.2.0</version>
		</dependency>

The tab completion of mycmd2 with an enum works, but mycmd1 with File doesn't. Maybe I'm missing something...

Thank you!

notwhoyouthinkiam avatar Apr 16 '20 01:04 notwhoyouthinkiam

Sorry for my late reply.

The PicocliJLineCompleter does not include a built-in completer for @Parameters or @Option annotated fields of type java.io.File or java.nio.file.Path.

However, you can provide your own by specifying a completionCandidates = SomeClass.class attribute. For example:

class FileCompleter implements Iterable<String> {
    @Override
    public Iterator<String> iterator() {
        String[] files = new File(".").list();
        return files == null ? Collections.emptyIterator() : Arrays.asList(files).iterator();
    }
}

You would install it like this:

        @Parameters(paramLabel = "FILE", arity = "1", description = "a file name",
          completionCandidates = FileCompleter.class)
        File file;

This example only shows the files in the current directory and does not recurse into subdirectories, but you get the idea. If you implement recursion into subdirectories you may want to implement some threshold (to stop after 1000 candidates or something) to prevent the app from running out of memory.

remkop avatar Apr 16 '20 08:04 remkop

I'm sorry for my late reply as well.

Your example works fine, thanks!

Would it be possible for the FileCompleter class to access the partial string that the user has typed? Thus, it would be easy to make the completion work for arbitrary directory levels and for absolute paths (one level at a time, just like Bash does).

Thank you!

notwhoyouthinkiam avatar Apr 16 '20 19:04 notwhoyouthinkiam

That is a very good point.

I think that JLine itself also provides some file completer, perhaps it would be nice to leverage that.

Alternatively, this may be a bit of a hack, but we can make this information available in PicocliJLineCompleter in a ThreadLocal, so that your completer can pick it up. Your completer would look something like this:

class FileCompleter2 implements Iterable<String> {
    @Override
    public Iterator<String> iterator() {
        ParsedLine line = PicocliJLineCompleter.parsedLineThreadLocal.get();
        String userInput = line.words().get(line.wordIndex());

        String[] files = new File(userInput).list(); // this bit needs more work... :-)
        return files == null ? Collections.emptyIterator() : Arrays.asList(files).iterator();
    }
}

And the modified PicocliJLineCompleter would look something like this:

// modified PicocliJLineCompleter

    public static ThreadLocal<ParsedLine> parsedLineThreadLocal = new ThreadLocal<>();

    public void complete(LineReader reader, ParsedLine line, List<Candidate> candidates) {
        parsedLineThreadLocal.set(line);
        try {
            // below is existing code until the finally clause

            String[] words = new String[line.words().size()];
            words = line.words().toArray(words);
            List<CharSequence> cs = new ArrayList<CharSequence>();
            AutoComplete.complete(spec,
                    words,
                    line.wordIndex(),
                    0,
                    line.cursor(),
                    cs);
            for (CharSequence c : cs) {
                candidates.add(new Candidate((String) c));
            }
        } finally {
            parsedLineThreadLocal.remove();
        }
    }

Can you experiment with that?

remkop avatar Apr 16 '20 21:04 remkop

Using your suggestion, I managed to write a working version of that example program (which really looks like a hack):

import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.concurrent.Callable;

import org.jline.reader.Candidate;
import org.jline.reader.LineReader;
import org.jline.reader.LineReaderBuilder;
import org.jline.reader.MaskingCallback;
import org.jline.reader.ParsedLine;
import org.jline.terminal.TerminalBuilder;

import picocli.AutoComplete;
import picocli.CommandLine;
import picocli.CommandLine.Command;
import picocli.CommandLine.Model.CommandSpec;
import picocli.CommandLine.Parameters;
import picocli.shell.jline3.PicocliJLineCompleter;

@Command(subcommands = {
        App.MyCmd.class
})
public class App {

    @Command(name = "mycmd")
    static class MyCmd implements Callable<Void> {

        @Parameters(paramLabel = "FILE", arity = "1", description = "a file name", completionCandidates = FileCompleter.class)
        File file;

        @Override
        public Void call() {
            System.out.println(file.getAbsolutePath());
            return null;
        }

    }

    static class FileCompleter implements Iterable<String> {

        public static File getDir(String d) {
            File f = new File(d);
            if (f.isDirectory())
                return f;
            String sep = File.separator;
            String pfx = (d.startsWith(sep) || d.startsWith("." + sep)) ? d.substring(0, d.indexOf(sep) + 1) : "";
            d = d.substring(pfx.length());
            int lastSlash = d.lastIndexOf(sep);
            if (lastSlash >= 0) {
                f = new File(pfx + d.substring(0, lastSlash));
                if (f.isDirectory())
                    return f;
            }
            f = new File(pfx);
            if (f.isDirectory())
                return f;
            return new File(".");
        }

        @Override
        public Iterator<String> iterator() {
            ParsedLine line = ModifiedPicocliJLineCompleter.parsedLineThreadLocal.get();
            String userInput = line == null ? "" : line.words().get(line.wordIndex());
            File dir = getDir(userInput);
            List<String> candidates = new LinkedList<>();
            String left, middle;
            if (userInput.equals(".") || dir.getPath().equals(".") && !userInput.startsWith("." + File.separator)) {
                left = "";
                middle = "";
            } else {
                left = dir.getPath();
                middle = dir.getPath().endsWith(File.separator) ? "" : File.separator;
            }
            for (File file : dir.listFiles()) {
                candidates.add(left + middle + file.getName() + (file.isDirectory() ? File.separator : ""));
            }
            return candidates.iterator();
        }
    }

    static class ModifiedPicocliJLineCompleter extends PicocliJLineCompleter {

        public static ThreadLocal<ParsedLine> parsedLineThreadLocal = new ThreadLocal<>();

        private final CommandSpec shadowedSpec;

        public ModifiedPicocliJLineCompleter(CommandSpec spec) {
            super(spec);
            this.shadowedSpec = spec;
        }

        @Override
        public void complete(LineReader reader, ParsedLine line, List<Candidate> candidates) {
            parsedLineThreadLocal.set(line);
            try {
                String[] words = new String[line.words().size()];
                words = line.words().toArray(words);
                List<CharSequence> cs = new ArrayList<>();
                AutoComplete.complete(shadowedSpec, words, line.wordIndex(), 0, line.cursor(), cs);
                for (CharSequence c : cs) {
                    String s = (String) c;
                    if (s.endsWith(File.separator) && new File(s).isDirectory()) {
                        // is there a way to test if this is using a FileCompleter?
                        candidates.add(new Candidate(s, s, null, null, null, null, false));
                    } else
                        candidates.add(new Candidate(s));
                }
            } finally {
                parsedLineThreadLocal.remove();
            }
        }

    }

    public static void main(String[] args) throws IOException {
        App app = new App();
        CommandLine cl = new CommandLine(app);
        LineReader reader = LineReaderBuilder.builder().terminal(TerminalBuilder.builder().build())
                .completer((new ModifiedPicocliJLineCompleter(cl.getCommandSpec()))).parser(null).build();

        while (true) {
            String line;
            line = reader.readLine(">>>>> ", null, (MaskingCallback) null, null).trim();
            ParsedLine pl = reader.getParser().parse(line, 0);
            String[] arguments = pl.words().toArray(new String[0]);
            new CommandLine(app).execute(arguments);
        }
    }
}

This is already usable, but there are two issues:

  • File names containing spaces are (correctly) completed with escaped spaces (\ ), but the parser ignores that as if a\ b represented two arguments. Since this is unrelated to the file name completion (as it happens regardless of the type), maybe there is already some option to deal with this case.
  • If the Candidate instances are created the way the original PicocliJLineCompleter does, then a space is always added after completing an entire value. This is undesired when we're typing paths, because the user may want to complete one level at a time, just like Bash does. When the candidate is a directory, the Candidate object must be created using the longer constructor with complete set to false. I used a workaround: do this if the candidate string is a valid path and ends with / (which FileCompleter adds for directories, like Bash); however, this also changes the behaviour in the (unlikely, but possible) event that the completed string of other type happens to be a valid (full or relative) path and ends with a slash. Maybe there is a way to avoid this, if it's possible for the complete method in PicocliJLineCompleter to tell if FileCompleter is being used.

Thank you so much for your answers! Despite being a hack and having the aforementioned issues, this is already good enough to be used.

notwhoyouthinkiam avatar Apr 17 '20 01:04 notwhoyouthinkiam

File names containing spaces are (correctly) completed with escaped spaces (\ ), but the parser ignores that as if a\ b represented two arguments

Is this the JLine parser or the picocli parser? If this is about the picocli parser, one idea is to quote completion candidate paths containing a space. So, in the list of generated candidates, if you have a path like:

C:\Program Files\Java\jre1.8.0_202\

then replace it with:

"C:\Program Files\Java\jre1.8.0_202\"

Bash does something similar. This is just an idea, I am not sure if this is feasible or whether it would work... :-)


Maybe there is a way to avoid this, if it's possible for the complete method in PicocliJLineCompleter to tell if FileCompleter is being used.

Here is another solution, it uses a public static field, which feels a bit like a hack 😜 again, but it may be more robust than your existing workaround:

// In the completer, create a public static field with the last processed ParsedLine instance

static class FileCompleter implements Iterable<String> {
    public static ParsedLine lastProcessedParsedLine; // <-- new

    @Override
    public Iterator<String> iterator() {
        ParsedLine line = ModifiedPicocliJLineCompleter.parsedLineThreadLocal.get();
        lastProcessedParsedLine = line;
        // remaining processing as in your code above...

Then, in ModifiedPicocliJLineCompleter, we decide which Candidate constructor to call based on whether the candidate was produced by our FileCompleter:

// ModifiedPicocliJLineCompleter: check if our ParsedLine was processed by our FileCompleter

        @Override
        public void complete(LineReader reader, ParsedLine line, List<Candidate> candidates) {
            parsedLineThreadLocal.set(line);
            try {
                String[] words = new String[line.words().size()];
                words = line.words().toArray(words);
                List<CharSequence> cs = new ArrayList<>();
                AutoComplete.complete(shadowedSpec, words, line.wordIndex(), 0, line.cursor(), cs);
                for (CharSequence c : cs) {
                    String s = c.toString();
                    if (line == FileCompleter.lastProcessedParsedLine) { // <--- new
                        // candidates are produced by our FileCompleter
                        candidates.add(new Candidate(s, s, null, null, null, null, false));
                    } else
                        candidates.add(new Candidate(s));
                }
            } finally {
                parsedLineThreadLocal.remove();
            }
        }

Thank you so much for your answers! Despite being a hack and having the aforementioned issues, this is already good enough to be used.

You are very welcome! Please star picocli on GitHub! 😉

We have focused on very specific solutions in this ticket, but I believe it is a general problem. Many picocli/JLine applications will want file completion. It may be worth exploring how picocli (or JLine!) can be improved to allow applications to get file completion without too many workarounds. Ideas welcome!

remkop avatar Apr 17 '20 02:04 remkop

I apologise for the delay! Once again, thank you for your answer. I've starred Picocli on Github! :smiley:

I'm using the parser returned by getParser() of LineReader, as in the code of my previous answer. Quoting the arguments works well, as long as the user adds the left quote in the first place. Bash does the same thing, but if the user doesn't add the quote, it auto-completes with escaped spaces and parses them accordingly (i.e. Program\ Files is treated as a single argument). Windows Command Prompt adds the quote automatically after auto-completion: if the user types cd C:\Progra and presses tab, it becomes cd "C:\Program Files\". Both approaches are valid, so it would be nice if Picocli/JLine could either parse the strings with escaped spaces (since it already auto-completes this way) or automatically add the left quote when the auto-completed string contains spaces.

Your solution for checking whether FileCompleter is being used works well, preventing any side effects when other parameter types are used! The correct condition is if (line == FileCompleter.lastProcessedParsedLine && s.endsWith(File.separator)), because we still want the space when a regular file name is completed. This is Bash's behaviour as well; for directories, it just adds a trailing slash, which FileCompleter's iterator() also does.

I don't know much about the internals of Picocli and JLine (I've just started experiencing with them), but I agree that file name completion is a general problem, as many applications have commands to open or save files. Ideally, file name completion should work for @Parameters/@Option fields whenever their type is File or Path; however, I don't know if this would be possible without some hacks that wouldn't probably be appropriate for Picocli classes. :stuck_out_tongue:

Thanks for your answers!

notwhoyouthinkiam avatar Apr 18 '20 12:04 notwhoyouthinkiam

I've searched a little more about it, and I've just found out that the other problem can be easily solved by creating a JLine parser like this:

DefaultParser parser = new DefaultParser();
parser.setEscapeChars(null);

Thus, passing this parser (.parser(parser)) instead of null, the second behaviour I mentioned before occurs: the quote is automatically added rather than the escaped strings.

notwhoyouthinkiam avatar Apr 24 '20 19:04 notwhoyouthinkiam

Very nice! Thanks for letting me know!

The more general solution I have in mind involves bringing (copying) some of the code that currently lives in the AutoComplete class (the AutoComplete::complete method and helper methods) into the picocli-shell-jline3 module. That was my conclusion in https://github.com/remkop/picocli/issues/762, and I still think that would be the best way forward.

This would allow the logic that currently lives in the AutoComplete::complete method to use the jline Candidate API directly. This has the benefit you already mentioned with the constructor to display a = at the end, but also has other benefits like plugging the option description into the right Candidate field so JLine can display this description together with the completion candidates for a much richer end user experience.

In addition to File completers, all kinds of option parameter and positional parameter completers would become possible. One idea would be to build an extension mechanism where applications can register completors on a type basis, or perhaps even on an individual OptionSpec or PositionalParamSpec basis. That would allow for one option to only show directories as completion candidates, and another option to show only files, and a third option to show both, for example.

I am currently working on other things, but if anyone would be interested in submitting a PR in this area, that would help in improving completion for everyone.

remkop avatar Apr 24 '20 23:04 remkop

would it make sense to have a ContextConsumer interface which completion candidates could optionally implement to "feed them" the context or would that be outside picocli 'contract'

i.e.

static class FileCompleter implements Iterable<String>, ContextConsumer {
    
     @Override useContext(Context ctx) {
           // use ctx to setup what iteration will do..
     }

    @Override
    public Iterator<String> iterator() {
        //. ...

then ctx could have things like partial string for use with jline and additional info...especially access to what other parameters been set thus far.

maxandersen avatar Jun 09 '20 07:06 maxandersen

Yes, that is certainly an idea. That would help get "richer" input to the completers. (Note that we also want to solve the other problem of how to let completers create richer output, like generating org.jline.reader.Candidate objects instead of just Strings. But that is separate.)

One tricky aspect is how to model the context.

JLine would provide an instance of org.jline.reader.LineReader and a org.jline.reader.ParsedLine object as a source of context. The ParsedLine offers the full list of tokens on the command line, and which token the cursor is on, and the location of the cursor within that token.

Other completion systems may offer other features, like whether the completion candidates should generate = after option names, but they may not offer tokenization for example (they may just offer a single String that is the full user input so far).

I have not looked enough at what is available so I am hesitant to commit to a Context interface at this stage... How about keeping that completely open: context has type Object, or a generic type?

Thinking out loud: how about also looking at the output problem with this interface:

// TBD where would this interface live?
// (in `picocli.CommandLine`, `picocli.AutoComplete`, or somewhere else?)
//
public interface ICompleter<C, T> {
    List<? extends T> completionCandidates(C context, Class<T> candidateType);
}

Also, this possible context implementation:

class SimpleCompletionContext {
    public String[] args() {...}
    public int argIndex() {...}
    public int positionInArg() {...}
    public int cursor() {...}
}

Then, in AutoComplete, we modify/add a version of the complete method as follows:

public static <T> int complete(CommandSpec spec, 
                           String[] args, 
                           int argIndex, 
                           int positionInArg, 
                           int cursor, 
                           List<CharSequence> candidates,
                           Object context, 
                           Class<T> candidateType,
                           List<T> typedCandidates) { ... }

We reuse the existing logic, with this refinement: When an option or positional parameter's completionCandidates are to be invoked, we see if this completion candidates provider implements the new ICompleter interface, and if so, we pass it the context and use the result to populate the typedCandidates list.

If context is null, we create a SimpleCompletionContext from the method parameters and pass CharSequence.class as the candidateType.

If the completer throws an exception (because it does not support a candidateType, or something just went wrong), we fall back to calling the existing Iterator<String> iterator() method.

Thoughts?

remkop avatar Jun 09 '20 08:06 remkop