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

Make $commandProcessor 'protected' #90

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

solicomo
Copy link

so that it can be used by sub class of Runners.

so that it can be used by sub class of Runners.
@janpecha
Copy link
Contributor

Hello, what is your use-case? Why do you need this change?

@solicomo
Copy link
Author

Hello, what is your use-case? Why do you need this change?

I need to extend CliRunner to support more options. And that requires an extended CommandProcessor.
In the extended CliRunner, I need to use the extended CommmandProcessor.

In the meanwhile, I don't want to copy all of the code in CliRunner::run() to the extended CliRunner.

In on word, both CliRunner and the extended CliRunner need to access $commandProcessor.

@janpecha
Copy link
Contributor

janpecha commented Jun 1, 2023

I need to extend CliRunner to support more options. And that requires an extended CommandProcessor.

Can you give me some examples? Maybe exists better solution.

@solicomo
Copy link
Author

solicomo commented Jun 1, 2023

I need to extend CliRunner to support more options. And that requires an extended CommandProcessor.

Can you give me some examples? Maybe exists better solution.

For example, I need -c option to specify ssh key file which is not the default one. -c has to go to after git but before sub command.

The current implementation is CommandProcesser always put options after sub command.

Actually, I think -c is worth a set of specific API. So I created another PR#91.

This PR is still needed for other similar cases.

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