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

CommandSpec.remove(arg) does not removes the arg from command, which increases args in CommandSpec #1434

Closed
kaushalkumar opened this issue Sep 14, 2021 · 4 comments · Fixed by #1435
Milestone

Comments

@kaushalkumar
Copy link
Contributor

Hi,
Using picocli, we have developed a feature to hide/unhide options at runtime (based on user input). For this we used reference of 736. It works great, but CommandSpec keeps adding the argument to variable args, whose size keeps increasing, though they are not linked to any commandspec.

We could not figure out any solid reason of adding items to the list during remove. We faced a problem, as we used args() to get the list of arguments for a command and were assuming that it will provide unique set of args (associated to command), but it returned duplicate (unwanted/outdated) elements. Is it possible to remove the arg from args list when removing the arg.

For example, using the code given below, we see that args size grows from 12 to 212 (in 20 set of hide/unhide). This seems to be unnecessary so thought of indicating it.

import picocli.CommandLine.Command;
import picocli.CommandLine.Model.CommandSpec;
import picocli.CommandLine.Model.OptionSpec;
import picocli.CommandLine.Option;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;

@Command(mixinStandardHelpOptions = true)
public class TestHideUnhide implements Runnable {
    private static CommandLine testHideUnhideCommandLine;
    @Option(names = {"-f1", "--field1"}, hidden = true)
    private String field1;
    @Option(names = {"-f2", "--field2"}, hidden = true)
    private String field2;
    @Option(names = {"-f3", "--field3"}, hidden = true)
    private String field3;
    @Option(names = {"-f4", "--field4"}, hidden = true)
    private String field4;
    @Option(names = {"-f5", "--field5"}, hidden = true)
    private String field5;
    @Option(names = {"-f6", "--field6"})
    private String field6;
    @Option(names = {"-f7", "--field7"})
    private String field7;
    @Option(names = {"-f8", "--field8"})
    private String field8;
    @Option(names = {"-f9", "--field9"})
    private String field9;
    @Option(names = {"-f10", "--field10"})
    private String field10;

    public static void main(String[] args) {
        TestHideUnhide testHideUnhide = new TestHideUnhide();
        testHideUnhideCommandLine = new CommandLine(testHideUnhide);
        testHideUnhideCommandLine.execute(args);
    }

    private static void hideOptions(final CommandLine commandLine, final List<String> hiddenOptionNames) {
        CommandSpec commandSpec = commandLine.getCommandSpec();
        List<OptionSpec> optionSpecs = commandSpec.options();
        List<OptionSpec> optionSpecsToRemove = new ArrayList<>();
        List<OptionSpec> optionSpecsToAdd = new ArrayList<>();
        for (String hiddenOptionName : hiddenOptionNames) {
            for (OptionSpec optionSpec : optionSpecs) {
                if (Arrays.asList(optionSpec.names()).contains(hiddenOptionName)) {
                    OptionSpec hiddenOptionSpec = OptionSpec.builder(optionSpec).hidden(true).build();
                    optionSpecsToRemove.add(optionSpec);
                    optionSpecsToAdd.add(hiddenOptionSpec);
                    break;
                }
            }
        }
        optionSpecsToRemove.forEach(commandSpec::remove);
        optionSpecsToAdd.forEach(commandSpec::add);
        Map<String, CommandLine> subcommands = commandLine.getSubcommands();
        for (Map.Entry<String, CommandLine> entry : subcommands.entrySet()) {
            CommandLine subCommand = entry.getValue();
            hideOptions(subCommand, hiddenOptionNames);
        }
    }

    public void run() {
        for (int i = 0; i < 20; i++) {
            System.out.println("Iteration " + i);
            System.out.println("============");
            System.out.println("Number of arguments in command = " + testHideUnhideCommandLine.getCommandSpec().args().size());
            System.out.println("Number of options in command = " + testHideUnhideCommandLine.getCommandSpec().options().size());
            System.out.println("Unhiding>>>");
            List<String> hiddenOptions = unHideOptions(testHideUnhideCommandLine);
            System.out.println("Number of arguments in command (after unhiding) = " + testHideUnhideCommandLine.getCommandSpec().args().size());
            System.out.println("Number of options in command (after unhiding) = " + testHideUnhideCommandLine.getCommandSpec().options().size());
            System.out.println("Hiding>>>");
            hideOptions(testHideUnhideCommandLine, hiddenOptions);
            System.out.println("Number of arguments in command (after hiding) = " + testHideUnhideCommandLine.getCommandSpec().args().size());
            System.out.println("Number of options in command (after hiding) = " + testHideUnhideCommandLine.getCommandSpec().options().size());
        }
    }

    private List<String> unHideOptions(final CommandLine commandLine) {
        List<String> hiddenOptionNames = new ArrayList<>();
        CommandSpec commandSpec = commandLine.getCommandSpec();
        List<OptionSpec> optionSpecs = commandSpec.options();
        List<OptionSpec> optionSpecsToRemove = new ArrayList<>();
        List<OptionSpec> optionSpecsToAdd = new ArrayList<>();
        for (OptionSpec optionSpec : optionSpecs) {
            if (optionSpec.hidden()) {
                hiddenOptionNames.add(optionSpec.shortestName());
                OptionSpec unhiddenOptionSpec = OptionSpec.builder(optionSpec).hidden(false).build();
                optionSpecsToRemove.add(optionSpec);
                optionSpecsToAdd.add(unhiddenOptionSpec);
            }
        }
        optionSpecsToRemove.forEach(commandSpec::remove);
        optionSpecsToAdd.forEach(commandSpec::add);
        Map<String, CommandLine> subcommands = commandLine.getSubcommands();
        for (Map.Entry<String, CommandLine> entry : subcommands.entrySet()) {
            CommandLine subCommand = entry.getValue();
            hiddenOptionNames.addAll(unHideOptions(subCommand));
        }
        return hiddenOptionNames;
    }
}
@remkop
Copy link
Owner

remkop commented Sep 14, 2021

Thanks for raising this!
Will you be able to provide a pull request for this?

@kaushalkumar
Copy link
Contributor Author

Done. Please check 1435

@remkop remkop linked a pull request Sep 15, 2021 that will close this issue
@remkop
Copy link
Owner

remkop commented Sep 15, 2021

Wonderful, many thanks! I merged the PR.
Still need to update the release notes.

@remkop
Copy link
Owner

remkop commented Sep 15, 2021

Updated the release notes now, closing this ticket.
Thank you again for the contribution!

@remkop remkop closed this as completed Sep 15, 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
MarkoMackic added a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants