#1346 path/host completion for custom types #1347
Conversation
loicrouchon
force-pushed
the
feature/1346-path-completion-for-custom-types
branch
from
b8c7f06
to
9cb448b
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1347 +/- ##
============================================
+ Coverage 93.76% 93.81% +0.04%
- Complexity 474 476 +2
============================================
Files 2 2
Lines 6961 6998 +37
Branches 1869 1872 +3
============================================
+ Hits 6527 6565 +38
Misses 147 147
+ Partials 287 286 -1 ☔ View full report in Codecov by Sentry. |
remkop
added
theme: auto-completion
|
First, thanks for pointing out the lack of a About the PR, my intention was to not reflect this in the CommandLine model, and keep the change limited to the AutoComplete class. If anything, this is a concern of the completion, not the command. About the new option, I am not sure if there should be a short option. Providing a short option signals to end users that this is a common thing to do, common enough to provide a shortcut. Providing only a long option signals that this is possible but optional; this seems most appropriate. Can I ask you to change the above?
That is interesting. Looks like an existing bug. Would you mind raising a separate issue for this? (Ideally with how to reproduce it) |
👍 Great, I'll take a look
❓ I thought the same but then I wondered how to have this as part of the application when the application having So far, I only see sub-classing
👍 I will remove the short option.
Sure, that was on my list to create an issue. I will first create a reproducer to ensure I did not had a weird setup leading to this behavor. |
One idea is to have a custom |
👍 Ok, I'll write a test to for this too |
loicrouchon
changed the title
Before moving forward with documentation and better test coverage (Sub-command mode, It only handles dynamic completions (path/host) and not static ones Let me know. |
|
Your Thank you for putting in all this effort! I feel a bit guilty that you are putting in all this effort but I don't feel we have arrived at the best solution yet. I was thinking... Perhaps it would be nicer to not have to specify type names on the command line. I can see that now. I thought of an alternative (there may be other ideas too, let me know). Then applications can annotate the option, something like this: class MyCommand {
@picocli.AutoComplete.FileCompletion
@Option(names = "...")
List<Repository> repositories;Not sure if that can be made to work without eventually pulling this into the An alternative is to annotate the custom type itself: @picocli.AutoComplete.FileCompletion
class Repository { ... }This would be easier to implement (in the Thoughts? |
|
Haha, don't worry about this, I totally understand your points and I know of experience things are easy until you get into details. I'm not a big fan of the second approach for the exact reason you mentioned. In case you don't have control over the type itself (third-party library). But also for a question of separation of concerns as it would make this type aware of being used in a CLI environment. Regarding the extra annotation on the options/argument itself, it's nice as it would be possible to only read it in the An alternative approach which I'm not completely satisfied with, is to put this annotation on implementations of the But then the I tried to summarize below the different approaches with their pros/cons.
PS 1: I did not include the fact you don't like this info to be part of the PS 2: If the solution we go for can also support future evolution (for example a dynamic option that would call the app itself with the current command line context) that would be amazing. |
This reverts commit b9fdd69.
loicrouchon
force-pushed
the
feature/1346-path-completion-for-custom-types
branch
from
516b4b9
to
0cb42e7
Compare
|
Above force push was to fix the author email in the commits. Regarding the possibles solutions, I can't think of any other viable/acceptable ones for the moment. Looking at the previously mentioned ones, my preference goes to the annotations on the
Regarding the semantics of the WDYT? |
|
Sorry for my late reply. It is getting hard recently to find time to sit down and really think about this and other picocli tasks... :-) Annotations on the TypeConverterOne disadvantage of the TypeConverter idea is that I believe many type converters may be specified as a lambda, like this: new CommandLine(new MyApp())
.registerConverter(Cipher.class, s -> Cipher.getInstance(s))
.execute(args);...so, many type converters may not be a full-fledged class to begin with - meaning, no place to put an annotation. It also "feels" a bit like we are conflating two separate concerns here; the job of a type converter is to convert user input Strings into some other type. I can see how it could be made to work, but it seems that there must be a cleaner way to do this that is still convenient for users. Arguments annotations (@picocli.AutoComplete.FileCompletion, ...)In your analysis of various approaches and their pros and cons (very helpful, by the way, thank you for that!), about the first point:
I am not sure how much of a disadvantage this is: I imagine this annotation would only need to be specified on a handful of options (I could be wrong of course). Your point can also be taken as an advantage: is it possible that some applications want the built-in shell file/host completion for some options and custom Out of the options we have considered so far, I like argument annotations ( |
|
Hello, no problem 😉 Here is some context for: "Not centralized. To be repeated for every single @Option/@parameters of the given type". I'm currently working on a project named symly (the name might change) where I have two important notions: The project would consist of several subcommands: Having to repeat it is not the end of the world, but testing the auto-complete is properly generated is not super handy. So I fear I'll forget the annotations somewhere and have a not so great auto-complete script. To summarize: I like the annotations approach. Putting on I will give it a few thoughts, this week, but I have a few other questions:
From what I see the general approach in picocli is to have big annotations like |
Reading your questions I got a strong sense that this approach (annotations on options/parameters) is the right approach. Still a lot to discover and flesh out, but this feels like the right direction!
Picocli's
To me, marking something as deprecated expresses an intention to eventually remove it. I do not have such an intention (picocli is very big on being backwards compatible), so it is not necessary to deprecate the
That question (of what shape the solution should take) will come up at some point, and we are not that far away from that point, but I would like to explore the problem space a bit further first. (Thinking out loud) The "dynamic" one is interesting but needs more thought: This "runtime" we are speaking of, is this not actually auto-completion script generation time (as opposed to truly auto-completion script runtime)? What can we do at completion script generation time, and what can we do at completion script runtime? Can we/Would we want to support an annotation (or annotation attribute) where end users can specify a bash snippet that is executed at auto-completion script runtime to generate completion candidates? (Or zsh, or some other shell scripting language?) |
👍 The problem we are trying to solve:
This can be generalized as:
Which can then be generalized again as:
This means:
Possible behaviorsThe possible auto-complete behaviors I can think of are:
Default completionDisable the completion for the Static list of valuesThis list could be built automatically ( Path completionPath completion could be automatically generated for
Host completionHost completion could be automatically generated for User completionUser completion auto-complete with system user names Group completionGroup completion auto-complete with system group names Job completionJob completion auto-complete with system jobs. Service completionService completion auto-complete with system service names. Signal completionSignal completion auto-complete with system signal names. Environment variable completionEnvironment variable completion auto-complete with shell environment variable names. Variable completionVariable completion auto-complete with system shell variable names. Dynamic shell completionDynamic shell completion would allow to execute on It also could be supported to allow the user to define shell functions that would be included in the auto-complete shell scripts, so that they can be referenced here. Dynamic command completionDynamic completion would allow to execute on Dynamic Picocli completionDynamic Picocli completion would allow to execute on Custom completionCustom completion would allow the user to specify the autocomplete command. Those proposition are based on what is currently possible to implement in bash as per the bash completion documentation. The above list is for brainstorming only, they might(/should?) not all be implemented, at least, not in this ticket. I hope this clarifies the list of things that could be supported |
|
@loicrouchon, glad to see this effort, as I'm in need of the "Custom completion" functionality you've described above. The details of my use case are specific to my application, but for our purposes here, I'm looking to create completion functionality similar to the way and it allows for tab-completing a partially written ref out to its full name: This is obviously Git-specific functionality, and Git needs to look into its own object database to generate the completion candidates. I need to be able to do something similar in my own application. Ideally, I'd like to be able to tell Picocli that auto-completion candidates for a given I would want to be able to run any arbitrary code in that implementation, and I would want to have access to the Picocli context it's running in, e.g. to have access to the current (sub)command's I would think this most general kind of candidate provider hook would be the first thing to implement, and then more specific conveniences such as the ones @loicrouchon mentioned above could be built on top of it. @remkop, do you generally agree? @loicrouchon, do you plan to return to work on this effort? Thanks to you both. |
|
Hello @cbeams Yes, I would like to continue to work on this. At least on the narrow scope of the path/host completion for custom types. But the dynamic completion will not be part of this pr as it has its own dedicated issue: #506. However I feel both issues need some common design which we started to discuss above. As I'm not the owner of the project I can propose ideas, and submit a PR. But I would need some feedback from @remkop to be able to move forward and propose a solution in line with the picocli (great) way of doing things. |
|
Looking at recent comments by @gunnarmorling and @yschimke in #506, it may be interesting to tie these together. (Thinking out loud) what if we introduce a new annotation, say public class AutoComplete { // existing class picocli.AutoComplete
// new nested interface
@Target({ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER, ElementType.TYPE})
@Retention(RetentionPolicy.RUNTIME)
public @interface Generator {
String value(); // command to be executed by the shell to generate completion candidates
String shell() default "bash";
}
//...This annotation can be used on any element that is also annotated with The This could then be implemented by a hidden subcommand. Here is an example of how this annotation could be used: @Command(name = "mycommand")
public class MyCommand implements Runnable {
@AutoComplete.Generator("mycommand generateCompletionsForMyCustomType")
@Option(names = {"-o", "--my-option"})
MyCustomType myType;
@Command(hidden = true) // hidden subcommand of mycommand
public void generateCompletionsForMyCustomType(@Unmatched String[] ignored) {
String[] myCompletions = dynamicallyGenerateCompletionsForMyCustomType();
System.out.println(String.join(" ", myCompletions);
}
public void run() { // business logic of MyCommand
}
}The picocli model would need to be updated to get the The One idea (again, just thinking out loud, this may not work...) would be: // AutoComplete.java
private static void generateCompletionCandidates(StringBuilder buff, OptionSpec f) {
CompletionGeneratorSpec custom = f.completionGenerator("bash");
if (custom != null) {
buff.append(format(" local %s_option_args=$(%s) # %s values\n",
bashify(f.paramLabel()),
custom.value(),
f.longestName()));
} else {
buff.append(format(" local %s_option_args=\"%s\" # %s values\n",
bashify(f.paramLabel()),
concat(" ", extract(f.completionCandidates())).trim(),
f.longestName()));
}
}
In the above example, the generator spec value is a hidden picocli subcommand, but it does not have to be, it can be any script or function. For example, for the use case where only a subset of an enum are valid completions for some subcommand, the value could simply use @Command
public class SomeCommand {
@AutoComplete.Generator("echo HOURS MINUTES SECONDS")
@Option(names = "--time-unit")
TimeUnit timeUnit;
}Or another use case: a @Command
public class SomeCommand {
@AutoComplete.Generator("ls . | grep *.txt")
@Option(names = "--file")
Path myFile;
}This would allow dynamic completion, and developers would not need to modify the generated bash completion script (which is what I believe @gunnarmorling and @yschimke ended up doing in #506). Thoughts? |
|
Hey @remkop, this looks like a good direction. A few thoughts and questions: @AutoComplete.Generator("mycommand generateCompletionsForMyCustomType")It's worth considering that Regarding shell-specific completion generators and the need for the @AutoComplete.Generator(value="mycommand generateBashCompletionsForMyCustomType", shell="bash")
@AutoComplete.Generator(value="mycommand generateZshCompletionsForMyCustomType", shell="zsh")
@Option(names = {"-o", "--my-option"})
MyCustomType myType;In the declaration of the hidden generator command, what is the significance or necessity of the public void generateCompletionsForMyCustomType(@Unmatched String[] ignored) {...}Regarding filtering autocompletion candidates based on partial user input:
This part is very important to my own use cases. If I understand correctly, you're talking about making sure it's possible to implement filtered completions, a la the way $ git branch foo
$ git branch bar
$ git branch baz
$ git checkout b<tab>
bar bazYou mention that this partial user input is passed to the Regarding Finally, regarding the string value provided to the @AutoComplete.Generator("mycommand generateCompletionsForMyCustomType")
@Option(names = {"-o", "--my-option"})
MyCustomType myType;Perhaps the fragile string could be avoided by (optionally) allowing the user to provide an implemantion of, say, @AutoComplete.Generator(type=MyCustomTypeAutoCompletionGenerator.class)
@Option(names = {"-o", "--my-option"})
MyCustomType myType;where |
Agreed. We should experiment with what gives the most robust result and document that.
Possibly yes once support is added for additional completion scripts like Zsh (#294), Fish (#725), Yori (#1062), PowerShell (#922) and clink (#921). This may never happen, but it impacts the API of the model, so I want to build it in up front. With API of the model I mean methods used by the OptionSpec option (...)
CompletionGeneratorSpec custom = option.completionGenerators().get("bash");
Yes, although strictly speaking we may then need to wrap it in a @AutoComplete.Generators({
@AutoComplete.Generator(value="mycommand generateBashCompletionsForMyCustomType", shell="bash"),
@AutoComplete.Generator(value="mycommand generateZshCompletionsForMyCustomType", shell="zsh")})
@Option(names = {"-o", "--my-option"})
MyCustomType myType;
In Bash completion, this is possible by passing We would need to document this. (See also the answer to your next question below.)
You are right, this is usually not necessary, unless in the annotation we include I was still debating on whether we always pass If You could then write it as: @Command
public void generateMyCompletions(@Parameters String[] partialInput) {...}
I appreciate that, but another way to look at it is that, since shell completion will always be very shell-specific, we do have some additional freedom that the pure Java world does not have. :-)
Maybe yes, especially from the point of view of integrating this in a JLine shell. In that scenario, the completion generator will be invoked in the same Java process that is running JLine and in which the command will be executed. This means it is easier to share state between the completion logic and the command, without going to the file system or a database, for example. I have not thought this scenario through enough though. I am not sure yet about the trade-off between string fragility on the one hand and the complexity of automatically generated hidden commands with some convention-based name on the other hand. The String-based solution seems simpler, which may be important since this stuff is already complex enough as it is... 😅 Great feedback! Thank you! |
This is an early PR for issue #1346
The goal is to ask for feedback before going further.
I haven't found a
CONTRIBUTING.md, therefore I have a few process oriented questions.build.gradle, but it seems travis only builds on JDK 8 and above?Concerning the issue in itself, I used a slightly different approach to avoid to pass the extra argument all the way around in the
AutoCompletecommand.This has the advantage that in case the auto-complete is embed as a subcommand, it is possible to programmatically register the path completion types as done for the type converters.
The path completion types are directly stored in the
CommandLineand not in theInterpreteras I thought it is does not belong to the interpreter. But I'm not 100% sure of what the interpreter is, so let me know if you think I should move it inside.What remains to be done:
AutoCompleteerror handling in case the class is not found in the classpathCommandLinemethodsWhat does not work:
Let me know what you think of the current approach and I'll move forward with your feedback
Regards,