cli

cli copied to clipboard

Hi

In case you use a command with wrong number of arguments, the error message will be the same as having an unexciting command entered.

Thank you @rezaebrh . You're right: the system should definitely discriminate against the two cases and print different error messages. Indeed, the library should provide also customizable messages (e.g., for messages in languages different than English). Unfortunately, I cannot merge your PR as is, because you're using std::cout to print the new error message, instead of CliSession ostream. In your code, the new error messages is printed on the local console (std::cout) even if the error happened on remote shells! All the output must be done using the CliSession ostream to ensure the output will happen in the right session (local or remote).

Yeah sorry for my carelessness. The output streamer fixed. Although in this PR I haven't done something in order to provide customizable error messages, but I'm looking forward to fix it too.

Thank you for your fix, @rezaebrh ! I'm planning to check your PR on Monday. As for the custom error message, it requires a little thinking about the overall library mechanism, and I would prefer to decide the design, before. I created a new issue for the custom error messages (#64 ), I will start to work on it as soon as it will get some thumb up :-)

Hi guys! Actually I was already working on making this functionality work too, but did not get time to finish that code yet. On the topic of this particular PR though, I don't think it will do the right thing. Consider the following condition (taken from examples/complete.cpp):

    rootMenu -> Insert(
            "add", {"first_term", "second_term"},
            [](std::ostream& out, int x, int y)
            {
                out << x << " + " << y << " = " << (x+y) << "\n";
            },
            "Print the sum of the two numbers" );

    rootMenu -> Insert(
            "add",
            [](std::ostream& out, int x, int y, int z)
            {
                out << x << " + " << y << " + " << z << " = " << (x+y+z) << "\n";
            },
            "Print the sum of the three numbers" );

If the current code is merged, whenever someone types add 1 2 3, it will actually just print out that error condition you wrote, because it will match the 2-arg handler for arg. It will not run the 3-arg handler.

This is because of this following code in the same file:

                    // check also for subcommands
                    std::vector<std::string > subCmdLine(cmdLine.begin()+1, cmdLine.end());
                    for (auto& cmd: *cmds)
                        if (cmd->Exec( subCmdLine, session )) return true;

@sakshamsharma you're right. A few ideas are coming to my mind since if I haven't changed the return value to true, I couldn't have been able to prevent multiple error messages, but I have to come up with a good idea to fix this. Thanks for reminding me.

Well, actually having an error message specific is a task almost impossible, because you can have the same command name with different parameter (different parameter number and different parameter types).

At the code level, Command::Exec should return an enum instead of a bool, and when iterating over the commands, if a match has not been found, check all the results: if they're all "command mismatch" you should return a "Command unknown" error, if there's at least one "type mismatch" return the message "Wrong type", if there's only "Number of parameters mismatch" return the message "Wrong number of parameters". It's not trivial, since the library iterates over the current menu commands, over the global commands and over the submenu commands.

Probably more trouble than it's worth. So, for the moment, I simply changed the single error message from "Unknown command" to "Wrong command" :-)