-
Notifications
You must be signed in to change notification settings - Fork 412
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for interactive shell using picocli #242
Comments
https://code.google.com/archive/p/cliche/ |
Thank you for the positive feedback! Great to hear! I think the only thing missing to accomplish a full interactive shell is a tokenizer that understands quotes. Basically, in your for loop you want to read in a single string from the console, and then parse it into separate arguments (like the shell does for you usually). So, input like
Is parsed into 6 tokens:
This can be a separate class from picocli.CommandLine. Do you think you'll be able to submit a pull request for this? |
I think that could be accomplished by splitting the input, looping and looking for quotes, and combining stuff. I might give this a try at some point. |
Looking forward to any contributions! FYI, I found the
|
On second thought, I'm not sure. I do all my work on a tablet using AIDE IDE, which unfortunately doesn't support Gradle for java projects. So I'll try, but not sure I can do proper testing. I'll probably have to do it in another project, test it, and copy it over hoping nothing breaks. |
I have some code tested for quotes and escaping spaces, I just need to work on putting it into picocli. Before I do so, any code style rules I need to be aware of? And I could also probably make some sort of runInteractive method, should I do that as a separate method, a flag for normal run, an annotation, or what? |
Generally, try to stick to the style of the existing code. I may reformat so don't worry too much. Also, can you show some examples of how this facility can/should be used? |
It takes a String, splits it on any unescaped/unquoted spaces, and returns a String array. |
I meant usage by users of the library. |
The main purpose is for an interactive shell, like this issue is about. You would just use it if a user of a project wanted to, say, put a path to a file with a space in it. This would split the rest up, but if the path was quoted, keep the spaces. Like in other shells, you can either quote it, or backslash escape. |
Like |
Ok, but isn't that how it currently works when a picocli-based command line utility is invoked from the command line or in a shell script? (Picocli already supports quoted command line arguments with embedded spaces.) I'm interested in seeing the code that application authors would need to write to support an interactive shell in their application. I imagine this would involve reading user input in a loop from standard input and processing the input as commands. I'm interested in seeing what your contribution adds in that scenario. |
Earlier on in the issue, you said that quoting wasn't supported. I made some code for that.
(If picocli already has quote support, most of that is unnecessary.)
|
Here's a simpler version, without all the quote code:
|
Basically, you just use a Scanner, split at new line, and print out a prompt (that's what the text to the left of the "$" in the terminal is called, right?). |
Thanks for the clarification. The quote support I mentioned is mostly coming from the operating system shell, where quoted arguments with embedded spaces are delivered to the application in a single argument. In a custom interactive shell, we need to do this work ourselves, which is what you are showing in the first example. Good stuff! Would it be possible to use |
I will be looking into |
The thing with my parser is that it supports escaping spaces and quotes, while |
I see. Escaping spaces may not be a common use case (users can quote to embed spaces instead), but I image that parameter values with embedded quotes are more common. Have you looked at doing this with a regular expression? I'd like to start with the simplest/shortest code possible. |
I haven't yet, because this felt simpler (if a lot more massive), but I can look into it. |
Good news: I got a regex. Bad news: I got a regex.
Now if you'll excuse me... screams into and murders innocent pillow |
Ouch! Your original proposal looks a lot easier to read and maintain. Let's go with that. I propose that we put this code in a separate class, that can be used something like this: class Example {
public static void main(String... args) {
String shPrompt = "cli> ";
System.out.print(shPrompt);
try (LineNumberReader reader = new LineNumberReader(new InputStreamReader(System.in))) {
String line = null;
while (line = reader.readLine() != null) {
String[] arguments = ArgumentSplitter.split(line);
CommandLine.run(new Main(), System.out, cmdSplit);
System.out.print(shPrompt);
}
}
}
} Tentatively named it |
Yeah, that makes since. Any particular reason you went with |
Like this?
Also, this version has support for escaping new lines, in case people script with their programs or something.
Example input:
Output:
(Note that currently, if you copy and paste the example input, it looks a bit messed up because it prints ">", but data and output are the same.)
|
Little issue I just realized: options inside quotes aren't escaped, so entering |
Good stuff. I would like to have a "core" method that just takes a String and splits it into substrings representing command line arguments. We could optionally support one or more convenience methods that check for multi-line input, but users may want to implement this themselves. The core method or methods would provide the building blocks for this. I imagine the signature of the core method to looks something like this:
Ideally, I would like to leave the choice of using a |
In that case, I'll probably move the other code to a function that takes a String, and then keep the newline escapes in the |
It sounds like the convenience method should not be a single method but a builder-like construct that users can configure before using it. Similar to |
Got it. So should I just basically have options for everything (quotes, maybe quotechar, escape newlines, escape spaces, etc)? |
Yes, that makes sense. |
I've made a new version of the class, using the It has two constructors, accepting either a I've added toggles and settings for a lot. Most of them come from They are initialized with some useful defaults, such as quotes as I also plan to add a method to check if something is in a category. Also note that while I have an option for comment characters (like
Example of an interactive shell using the
|
Cool. I’ll need some time to take a look. About the I noticed that the |
Can you share future versions in a pull request? GitHub has nice support for reviewing pull requests. |
It calls itself if new line escapes are enabled and the new line is escaped, in order to get whatever the user inputs next. It's inside an if block, so it only runs when needed. What do you mean, "read more input from the InputStream than we have consumed"? I can make it accept Unfortunately, as I stated above, I can't properly work and test via pull request, because |
Thanks for the clarification. About the "read more input ... consumed": my point was that once we pass the InputStream to the Scanner, the Scanner now "owns" this stream and we should no longer read from the stream directly. All access to this InputStream should go via the Scanner object. The Scanner could for example "read ahead" some characters from the InputStream into an internal buffer. We would miss the characters that are buffered in the first Scanner if we wrap the InputStream with another Scanner. Understood about the IDE. By the way, to do a pull request you don't need to compile, you can do everything on the GitHub web site: fork picocli, in your forked project navigate to |
I know that. However, I personally feel it makes the git history crazy ugly that way, but I can do that if you'd prefer. |
Yes please. Git lets you squash multiple commits into a single one to clean up the commit history. |
I know I'm a little bit late to the party, but there is jline which provides a GNU readline-like functionality in Java. |
Picocli itself should not have any external dependencies. If it is worth building things on top of jline, it may make sense to have a |
I personally am not a big fan of interactive shells. I'd second the idea to keep it as a separate project. Picocli core codebase is a clean and pristine work of art at the moment, lets keep it that way. An interactive shell might sound like a good idea initially but as your application grows in complexity you'd realize it's not. As an example from a project I've been working on, we'd developed a set of functions to aid in debugging of our microservices. So there's a function to read test data from a file and post it onto the JMS broker. Another function to send a message for dumping internal state etc. Since creating JMS connection pool and sessions is heavy it made sense to use something like Spring Shell, to do the initialization once on startup, and then run multiple commands:
The problem is that this goes against the Unix philosophy. Instead of having a set of small programs that can be connected to each other via pipes you now have a monolithic program that does everything and is no longer scriptable. So you can no longer do:
A few seconds I saved in startup initialization wasn't worth sacrificing the script-ability of my shell commands, given that often the slowest part of running a shell command is my typing speed. Furthermore, implementing an interactive shell into picocli won't be trivial:
I'd advise against trying to reinvent the unix shell inside a JVM. If you're on windows, you have my sympathy. Give a shot to Windows Subsystem for Linux or Cygwin. |
@Kshitiz-Sharma has a point about leveraging the unix shell. On the other hand, for some projects an interactive CLI may be the right choice. Picocli should work well in both use cases. I have looked at jline2 and jline3. This is a fairly large library with a lot of functionality. I haven't looked in depth yet but there's a lot there. Rather than re-implementing this functionality I will probably focus on making sure that picocli integrates really well with jline. For example, auto-generate jline completion functions for a picocli command. I'm also planning to play with jline some more and write some articles about combining jline with picocli to easily build powerful interactive CLIs. JLine2 does not have a tokenizer, as far as I can see, so the tokenizer in this ticket is certainly useful when integrating with jline2. JLine3 does have a tokenizer (DefaultParser), but I still need to look in detail. At first glance the tokenizer proposed here may have more features. |
There is also https://github.com/facebook/nailgun which seems like it wouldn't be very difficult to use with picocli. |
With #497 there is now a new module picocli-shell-jline2 where I plan to add functionality and documentation for building interactive shells with JLine 2 and picocli. As a starting point there will be a completer that dynamically completes the command in a JLine shell based on the picocli (Update: fixed link for renamed module) |
I'm going to close this ticket as I believe the original request has been met with the |
Hi @remkop ! Is the functionality actually still provided somewhere? I see that the https://github.com/remkop/picocli/tree/master/picocli-jline2-shell no longer exists :( |
Ignore me :) Found it: https://github.com/remkop/picocli/tree/master/picocli-shell-jline3 |
@Holmistr Glad you found it. At some early point I renamed the module from |
Just started using picocli for a simple command line app, and find it amazingly easy to use and cool. I have also been experimenting with having interactive shell in my program which once launched will also allow execution of several subcommands. I am using cliche for this. Though it seems like picocli should be already able to handle this. Am I missing some obvious way of putting CommandLine in a loop to continuously parse for subcommands.
Happy to contribute if there is an agreeable change which could make it work that way.
The text was updated successfully, but these errors were encountered: