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 ConsoleFormatter's context configurable #130

Closed
pblesi opened this issue Sep 22, 2019 · 5 comments
Closed

Make ConsoleFormatter's context configurable #130

pblesi opened this issue Sep 22, 2019 · 5 comments

Comments

@pblesi
Copy link
Contributor

pblesi commented Sep 22, 2019

Right now Airbrussh, provides an Airbrussh::Rake::Context. This Context is used in the ConsoleFormatter and provides the following interface: current_task_name, register_new_command, and position. In the context of Capistrano, this context is enabled. When a new task is started, the current task changes and the history is reset. Outside the context of Capistrano, when using SSHKit directly, current_task_name always returns nil and position becomes an incrementing number for the entirety of execution.

It would be useful if Airbrussh::Rake::Context was abstracted so that a context could be set using configuration. This way SSHKit executors that do not use Capistrano/Rake (e.x. https://github.com/braintree/runbook) could define their own implementations of current_task_name, register_new_command, and position.

@mattbrictson
Copy link
Owner

Thanks for the suggestion! I'm interested in this idea.

What do you think the configuration syntax for this would look like? Is it something the user will put in their Capfile or deploy.rb?

@mattbrictson
Copy link
Owner

Is it something the user will put in their Capfile or deploy.rb?

Actually, I guess the answer is "neither" since this is intended for users who are not using Capistrano. In that case could you provide an example of what this configuration would look like with plain SSHKit or something like runbook?

@pblesi
Copy link
Contributor Author

pblesi commented Oct 7, 2019

Right. So this is how Runbook currently initializes Airbrussh through SSHKit:

https://github.com/braintree/runbook/blob/4a0f0770a8a2a7be135cf13ee435d981b5975a06/lib/runbook/configuration.rb#L92-L96

If Airbrussh took a context option as part of its format options, then this would be a way to configure the context. This class would default to Airbrussh::Rake::Context. Another class could be set that adheres to the specified interface (current_task_name, register_new_command, and position).

I believe the configuration could analogously be applied to the set :format_options interface used by capistrano:

https://github.com/mattbrictson/airbrussh#configuration

The syntax in the Airbrussh::Formatter initializer case would be:

      ssh_kit.output = Airbrussh::Formatter.new(
        $stdout,
        banner: nil,
        command_output: true,
        context: Runbook::Airbrussh::Context
      )

And in the Capistrano case would be:

set :format_options, color: false, truncate: 80, context: CustomAirbrusshContext

I'd be happy to throw together a PR

@mattbrictson
Copy link
Owner

Makes sense. Thanks for the writeup.

I'd be happy to throw together a PR

Yes please! 🙏

@mattbrictson
Copy link
Owner

Fixed via #131

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

No branches or pull requests

2 participants