Skip to content

Conversation

@miguelangel-dev
Copy link
Contributor

@miguelangel-dev miguelangel-dev commented Mar 9, 2020

Currently HelpCommand is launching a side-effect in run method (printing in the console the help). This PR fixes it and throw the effect to the next layer, who will catch it and treat the effect properly. Effects should be handled on the main method inside exit(withError:) or let to the user decided how to manage them

This change will let us write something like:

enum ParsableCommandError: Error {
    case general(String)
}

extension ParsableCommand {
    func parse(_ arguments: [String]? = nil) -> Result<ParsableCommand, ParsableCommandError> {
        do {
            let command = try Self.parseAsRoot(arguments)
            try command.run()
            return .success(command)
        } catch {
            let info = Self.fullMessage(for: error)
            return .failure(.general(info))
        }
    }
}

In the end, delegate the responsibility about when executing the effects and how handling

I have tested it using ParsableCommand with subcommands and without them. It works properly in both cases.

Checklist

  • I've added at least one test that validates that my change is working, if appropriate
  • I've followed the code style of the rest of the project
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary

@miguelangel-dev
Copy link
Contributor Author

@natecook1000 please, we are working on a migration in our lib nef and we need this change. Could you review it? Thanks

@natecook1000
Copy link
Member

Thanks @miguelangel-dev — I'm going to merge this change. The thing we're missing for full manual handling of errors is a way to determine whether an error is really an "error", since you'd want to treat a validation error separately from a help request. I'll fill that hole in a follow-up PR before version 0.0.3.

@natecook1000 natecook1000 merged commit 9af92b6 into apple:master Mar 13, 2020
@natecook1000
Copy link
Member

See #79 for the WIP new method — any feedback you have would be great!

@miguelangel-dev miguelangel-dev deleted the miguelangel/effects_help_command branch March 13, 2020 20:06
@miguelangel-dev
Copy link
Contributor Author

I'm going to merge this change. The thing we're missing for full manual handling of errors is a way to determine whether an error is really an "error", since you'd want to treat a validation error separately from a help request. I'll fill that hole in a follow-up PR before version 0.0.3.

It sounds good. Thanks for the info, I will keep an eye in the #79

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants