Add support for config.command_output

[mattbrictson]

Normally airbrussh hides all stderr and stdout data returned by SSH commands. With this commit, users of airbrussh can selectively enable displaying of stderr and/or stdout data.

Addesses #3

@carlesso can you review?

[mattbrictson]

Sample output:

$ cap production deploy
Using airbrussh format.
Verbose output is being written to log/capistrano.log.
00:00 git:wrapper
      01 mkdir -p /tmp/matt-site/
    ✔ 01 deployer@xx.xx.xx.xx 0.700s
      Uploading /tmp/matt-site/git-ssh.sh 100.0%
      02 chmod +x /tmp/matt-site/git-ssh.sh
    ✔ 02 deployer@xx.xx.xx.xx 0.164s
00:01 git:check
      01 git ls-remote --heads git@bitbucket.org:xx.git
      01 28f6d5b8bc180225ea83bc2f87d3ec4fbe1fe88b   refs/heads/analytical
      01 a0d107abac1f7312c76da7cadd94e471cf29e02e   refs/heads/google-analytics
      01 7dea7e54db43a3df5a75f721f8a8e0cdaa74577f   refs/heads/master
      01 43b8fc41129d7afd05e70369d56025b11f07345a   refs/heads/pacific-time
      01 f3b9157c004d3edbc195fef1138f6d680199ff76   refs/heads/sitemap
    ✔ 01 deployer@xx.xx.xx.xx 1.609s

[mattbrictson]

One problem I see is that in a multi-server deployment, the output from different servers could be mixed together and hard to distinguish. The 01 number prefix indicates which command is associated with the output, but not which server.

However, it is probably only an issue for long-running commands… not sure if it is worth complicating the output.

[carlesso]

Hi @mattbrictson, everything looks good to me. Regarding multi-server deployment you might be right, right now I don't have any multi deployment where I can test it. But I guess that in such case the developer can switch between :sequence and :parallel capistrano behavior if he need. As soon as I'll be able to test it I'll let you know. Btw the same issue will cause the log/capistrano.log file to mixed. I'll think more on that.

I was testing the branch and I found out something strange. Can't tell who is guilty... I have this simple task:

namespace :logs do
  desc 'Tailf production logs'
  task :tail do
    on roles(:app) do
      Airbrussh.configuration.command_output = true if defined?(Airbrussh)
      execute :tail, '-f', release_path.join('log', "#{fetch(:rails_env, 'production')}.log")
    end
  end
end

And for same reason, the first 10 lines (the one coming from the existing files) will be separated by and empty line (a "\r\n" to be precise and always from stdout). The consecutive lines (aka the one coming in realtime) are not affected by this. Maybe adding .strip to check if the command is empty will solve this (capistrano does this too)

[mattbrictson]

@carlesso Good catch. I think the problem was that stdout.lines returns strings that still include the line separator, and then Console#print_line adds a newline of its own. I added a call to chomp to fix. Please re-test?

[carlesso]

The problem is that if line.chomp is empty, you will still print a blank line. Two options, imho. or change

airbrussh/lib/airbrussh/formatter.rb

Line 141 in 73704b2

next if output.empty?

from

    next if output.empty?
    # to
    next if output.chomp.empty?

Or here

airbrussh/lib/airbrussh/formatter.rb

Line 144 in 73704b2

print_line " #{number} #{line.chomp}"

change

          print_line "      #{number} #{line.chomp}"
          # to
          print_line "      #{number} #{line.chomp}" unless line.chomp.empty?

Maybe the latter is better.

[mattbrictson]

The problem is that if line.chomp is empty, you will still print a blank line.

Is that a problem, though? What if the tail output does contain legitimate blank lines? I'd think you'd want to see those in the output.

You're saying that there are situations where command.stdout contains exactly "\n" and that those should be discarded. I guess I'm not sure why that would ever happen. Do you have an example where that is happening?

I think this option is safer, since it less likely to skip legitimate blank lines:

next if output.chomp.empty?

[carlesso]

I'm not sure where the line is coming for (or how SSHKit::Command handles stdout/stderr). I was testing it in a local project with this (using ruby -I/path/to/my/clone):

#!/usr/bin/env ruby

require 'sshkit/dsl'
require 'airbrussh'
SSHKit.config.output = Airbrussh::Formatter.new($stdout)
Airbrussh.configuration.command_output = true

run_locally do
  execute 'tail', '-f', 'path/to/my.log'
end

And it is working fine. Maybe there is something strange going on with lograge that I'm using in that project. And you are totally right, if tail is giving me an empty line I want to see it. So your implementation is great, and I don't think the change with the chomp is needed.

[mattbrictson]

Great! I'll merge this and release 0.3.0 today. Thanks for your help. 🙌

[carlesso]

Thank you!