Skip to content
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

completionCandidates() called during help output even if not going to be used #1351

Closed
vstriz opened this issue Mar 17, 2021 · 8 comments
Closed
Labels
theme: auto-completion An issue or change related to auto-completion type: enhancement ✨
Milestone

Comments

@vstriz
Copy link

vstriz commented Mar 17, 2021

I have some cli commands that interact with the server. One of parameters has a completionCandidates setting that talks to the server to figure out the list of possible values, which can be dynamic. It looks like when I ask for "--help" of the command it also invokes the completionCandidates, even though the result is not going to be used. It gets it just in case it will need to use in the description if "${COMPLETION-CANDIDATES}" is specified. But in my case it's not used in description. My situation is even worse, sometimes I have no connection to the server, and therefore I'm unable to display the command help at all.

It would be nice, and more optimal in case of expensive operation, if the completionCandidates for help of commands was invoked only if needed. I.e. when "${COMPLETION-CANDIDATES}" is actually specified in description.

The code in question is in picocli.CommandLine.Model.ArgSpec.expandVariables(String[])

@remkop
Copy link
Owner

remkop commented Mar 17, 2021

I see, interesting.
Will you be able to provide a pull request for this, ideally with tests?
I would be happy to accept a patch but currently a bit overwhelmed with other commitments to work on this myself...

@remkop remkop added theme: auto-completion An issue or change related to auto-completion type: enhancement ✨ labels Mar 17, 2021
@vstriz
Copy link
Author

vstriz commented Mar 18, 2021

I'll see what I can do, but probably not for some time. Busy myself :)

I'm also on an older version picocli right now, which looks like might require some changes on my side to upgrade to latest. We'll have to look at that later.

@Lanninger08
Copy link

Hello, I wonder if this issue is still open, and I would like to work on this issue :).

@remkop
Copy link
Owner

remkop commented Apr 18, 2021

@Lanninger08 Hi!
As far as I know nobody has done any work on this.
Contributions are very welcome!

Great pull requests include tests and documentation (docs not always needed, like for bug fixes). Also nice to be consistent with the existing code style (spaces vs tabs, code layout etc), but that is not documented and I'm flexible.

Thanks for offering to spend time on this!

@Lanninger08
Copy link

Hi! @remkop
Sorry for the bothering but may I ask a question about the code..when will desc.length greater than 1? Thanks a lot for your help! :)

private String[] expandVariables(String[] desc) {
    if (desc.length == 0) { return desc; }
    StringBuilder candidates = new StringBuilder();
    if (completionCandidates() != null) {
        System.out.println("invoked here!");
        for (String c : completionCandidates()) {
            if (candidates.length() > 0) { candidates.append(", "); }
            candidates.append(c);
        }
    }
    String defaultValueString = defaultValueString(false); // interpolate later
    String fallbackValueString = isOption() ? ((OptionSpec) this).fallbackValue : ""; // interpolate later
    String mapFallbackValueString = String.valueOf(mapFallbackValue); // interpolate later
    String[] result = new String[desc.length];
    for (int i = 0; i < desc.length; i++) {
        result[i] = format(desc[i].replace(DESCRIPTION_VARIABLE_DEFAULT_VALUE, defaultValueString.replace("%", "%%"))
                .replace(DESCRIPTION_VARIABLE_FALLBACK_VALUE, fallbackValueString.replace("%", "%%"))
                .replace(DESCRIPTION_VARIABLE_MAP_FALLBACK_VALUE, mapFallbackValueString.replace("%", "%%"))
                .replace(DESCRIPTION_VARIABLE_COMPLETION_CANDIDATES, candidates.toString()));
    }
    return interpolate(result);
}

@remkop
Copy link
Owner

remkop commented Apr 19, 2021

@Lanninger08 desc.length can be between 0 and the max size of an array: the description of a command or option is an array that can contain multiple strings. For example:

@Command(description = {
            "line 1",
            "line 2",
            "line 3"})
class Example {
    @Option(name = "-x", description = {
            "line 1",
            "line 2",
            "line 3"})
    String x;
}

@wtd2 wtd2 mentioned this issue Apr 20, 2021
@remkop remkop closed this as completed in 0b47b17 Apr 21, 2021
remkop pushed a commit that referenced this issue Apr 21, 2021
@remkop remkop added this to the 4.6.2 milestone Apr 21, 2021
MarkoMackic pushed a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021
MarkoMackic pushed a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021
MarkoMackic pushed a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021
MarkoMackic pushed a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021
MarkoMackic added a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021
…onCandidates` optimization"

This reverts commit 05d952d.
MarkoMackic added a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021
MarkoMackic added a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021
MarkoMackic added a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021
This reverts commit 61eeba8.
MarkoMackic added a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021
…onCandidates` optimization"

This reverts commit 05d952d.
MarkoMackic added a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021
MarkoMackic added a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021
MarkoMackic added a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021
This reverts commit 61eeba8.
@vstriz
Copy link
Author

vstriz commented Nov 2, 2021

@remkop I am excited to see this issue fixed in upcoming release, but it's been a while since 4.6.1 was done. Any plan/word on when 4.6.2 will be released? Thanks.

@remkop
Copy link
Owner

remkop commented Nov 6, 2021

@vstriz picocli 4.6.2 has just been released. Enjoy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: auto-completion An issue or change related to auto-completion type: enhancement ✨
Projects
None yet
Development

No branches or pull requests

3 participants