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

PicocliCommands.invoke returns null not command result. Bug or feature? #2353

Open
pford19 opened this issue Dec 4, 2024 · 3 comments
Open
Labels
theme: shell An issue or change related to interactive (JLine) applications type: bug 🐛
Milestone

Comments

@pford19
Copy link

pford19 commented Dec 4, 2024

  • Version 4.7.6
  • File: PicocliCommands.java
  • Class: picocli.shell.jline3.PicocliCommands extends CommandRegistry

I started with the Picocli/Jline3 example to implement a shell.

invoke always returns null, and discards the command result. Is this a bug or feature? If a "feature" how best to signal a command loop to quit? (Other than end of file?)

My code, based on the example, looks like this

   // COMMAND LOOP
                while (true) {
                    try {
                        systemRegistry.cleanUp();
                        line = reader.readLine(prompt, rightPrompt, (MaskingCallback) null, null);
                        Object result = systemRegistry.execute(line);
                        // Oops, result is always null, this will not work
                        if (result instanceof Integer && ((Integer) result) < 0) {
                                // terminate loop
                                return;
                        }
                 } catch { 
                    ....
                 } 
                 
                 ...

// COMMAND                 

    @Command(name = "quit", mixinStandardHelpOptions = true)
    static class QuitCommand implements Callable<Integer> {
        public Integer call() {
            System.out.println("Bye now.");
            return -1; // signals termination of the command loop
        }
    }

PicocliCommands source in question:

    // For JLine >= 3.16.0
    @Override
    public Object invoke(CommandRegistry.CommandSession session, String command, Object... args) throws Exception {
        List<String> arguments = new ArrayList<>();
        arguments.add( command );
        arguments.addAll( Arrays.stream( args ).map( Object::toString ).collect( Collectors.toList() ) );
        cmd.execute( arguments.toArray( new String[0] ) ); // <<<<<<<<<<<<< execute result is discarded
        return null;  // <<<<<<<<<<<<< always returns null
    }
@remkop
Copy link
Owner

remkop commented Dec 8, 2024

Good point.
I guess it's better to return cmd.getParseResult() instead of null.
What do you think?

@pford19
Copy link
Author

pford19 commented Dec 8, 2024

Seems like the obvious choice. Completely new to picocli, so not immediately clear what the difference between the result of cmd.execute() vs. cmd.getParseResult(). But one or the other would make sense.

BTW, picocli is great. I had build a poor man's shell and command parser, and have been refactoring over the last week, with much delight.

@remkop
Copy link
Owner

remkop commented Dec 9, 2024

Seems like the obvious choice. Completely new to picocli, so not immediately clear what the difference between the result of cmd.execute() vs. cmd.getParseResult(). But one or the other would make sense.

The cmd.execute() method returns an exit code, whereas
cmd.getParseResult returns an object with data built up by the parser. This is the richest information possible to return. Other values may be derived from the ParseResult. So it seems best to return ParseResult.

BTW, picocli is great. I had build a poor man's shell and command parser, and have been refactoring over the last week, with much delight.

Glad to hear it!

@remkop remkop added this to the 4.7.7 milestone Jan 5, 2025
@remkop remkop added theme: shell An issue or change related to interactive (JLine) applications type: bug 🐛 labels Jan 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: shell An issue or change related to interactive (JLine) applications type: bug 🐛
Projects
None yet
Development

No branches or pull requests

2 participants