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

Should be able to pass Custom CommandInvoker class to CLI #15

Closed
tjprescott opened this issue Jul 14, 2017 · 4 comments
Closed

Should be able to pass Custom CommandInvoker class to CLI #15

tjprescott opened this issue Jul 14, 2017 · 4 comments

Comments

@tjprescott
Copy link
Member

No description provided.

@tjprescott
Copy link
Member Author

If you run any commands that return a list, you'll get

{
  "currentPage": [],
  "nextLink": ""
}

You have to subclass CommandInvoker and largely clone this logic in order to make it work with lists. It would be better to have some kind of hook method that could be passed in or at least overwritten in the subclass in order to provide this handling without having to duplicate the core logic.

@tjprescott tjprescott changed the title CommandInvoker doesn work with Paged results CommandInvoker doesn't work with Paged results Jul 17, 2017
@tjprescott
Copy link
Member Author

Actually, you can't even subclass the Invoker. I had to monkey patch over it.

@tjprescott
Copy link
Member Author

Additionally, the Knack invoker has no support for validators, so I need to monkey patch the CommandInvoker.execute method to reintroduce this functionality.

@derekbekoe derekbekoe added this to the August 2017 milestone Aug 4, 2017
@tjprescott tjprescott changed the title CommandInvoker doesn't work with Paged results Should be able to pass Custom CommandInvoker class to CLI Aug 4, 2017
@derekbekoe
Copy link
Contributor

You can pass in your own Command Invoker here. With this, you shouldn't have to clone a lot of the code.

Also, support for validators was added in #21.

Closing but let me know if there is still are large block of code you have to clone just for a small change.

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

No branches or pull requests

2 participants