Skip to content

Clarify output configuration option #244

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.

Sign up for GitHub

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

Jump to bottom
Merged
leehambley merged 8 commits into from
May 7, 2015
Merged

Clarify output configuration option #244

leehambley merged 8 commits into from
May 7, 2015

Conversation

@robd
Copy link
Contributor

@robd robd commented May 6, 2015

Currently the docs say we support assigning an io to the SSHKit.config.output configuration option:

SSHKit.config.output = $some_io

Unfortunately this doesn't work at all well as discussed in #243. Therefore, this PR updates the docs to remove that as a supported option.

It also simplifies some tests which were using the SSHKit.capture_output method. This assigned a String to SSHKit.config.output and then dynamically extended it with a custom write method from CaptureOutputMethods, which also added in newlines in some cases. These tests now use the output option in the way suggested in the docs, ie:

SSHKit.config.output = SSHKit::Formatter::SimpleText.new(@out)

This PR also re-enables and fixes the simple_netssh test on the TestNetssh functional test, which was being ignored.

Finally, it cleans up some setup methods on unit tests which were duplicating the method on UnitTest and adds some missing calls to super.

robd added 7 commits May 6, 2015 09:46
@robd
Remove skeleton TestCoordinator functional test
63a0c14
@robd
Introduced assertions in TestCoordinator
3a9c8c3 
assert_within_10_ms and assert_at_least_1_sec_apart
@robd
Replace capture_output with StringIO in TestCoordinator
2205237 
Be explicit about the parsed output format rather than replying on hidden behaviour in the CaptureOutputMethods module
@robd
Re-enabled and fixed the simple_netssh test in TestNetssh
9714689 
The simple_netssh test was missing the “test_” prefix, so wasn’t being executed. Re-enable the test and fix the assertions. Also replace capture_output / CaptureOutputMethods formatting with StringIO based approach.
@robd
Removed unused capture_output and CaptureOutputMethods test helpers
14f209f
@robd
Removed duplication and added missing super calls in unit test setup
92590aa
@robd
Added config test for :simpletext formatter
e4e0a1a
@robd
Copy link
Contributor Author

robd commented May 6, 2015

Hey @mattbrictson I don't think this should have any affect on Airbrussh, but I wanted to give you a heads up in case I've missed something.

@robd robd force-pushed the clarify-output-option branch from 6c40d95 to 0c7b33f Compare May 6, 2015 14:13
@leehambley
Copy link
Member

leehambley commented May 6, 2015

@mattbrictson I'll wait for an OK from you before merging this, if you'd be kind enough to test before hand!

@mattbrictson
Copy link
Member

mattbrictson commented May 6, 2015

I'll try to review later today (PDT).

@mattbrictson
Copy link
Member

mattbrictson commented May 6, 2015

Looks good to me! I tested this branch and it works fine. 👍

leehambley added a commit that referenced this pull request May 7, 2015
@leehambley
Merge pull request #244 from robd/clarify-output-option
bf9c6b2 
Clarify output configuration option
@leehambley leehambley merged commit bf9c6b2 into capistrano:master May 7, 2015
@leehambley
Copy link
Member

leehambley commented May 7, 2015

👥 teamwork!

jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Dec 14, 2015
Update ruby-net-ssh to 2.10.1.rc2.
cee0ba0 
## 1.8.1

  * Change license to MIT, thanks to all the patient contributors who gave
    their permissions.

## 1.8.0

  * add SSHKit::Backend::ConnectionPool#close_connections
    [PR #285](capistrano/sshkit#285)
    @akm
  * Clean up rubocop lint warnings
    [PR #275](capistrano/sshkit#275)
    @cshaffer
    * Prepend unused parameter names with an underscore
    * Prefer “safe assignment in condition”
    * Disambiguate regexp literals with parens
    * Prefer `sprintf` over `String#%`
    * No longer shadow `caller_line` variable in `DeprecationLogger`
    * Rescue `StandardError` instead of `Exception`
    * Remove useless `private` access modifier in `TestAbstract`
    * Disambiguate block operator with parens
    * Disambiguate between grouped expression and method params
    * Remove assertion in `TestHost#test_assert_hosts_compare_equal` that compares something with itself
  * Export environment variables and execute command in a subshell.
    [PR #273](capistrano/sshkit#273)
    @kuon
  * Introduce `log_command_start`, `log_command_data`, `log_command_exit` methods on `Formatter`
    [PR #257](capistrano/sshkit#257)
    @robd
    * Deprecate `@stdout` and `@stderr` accessors on `Command`
  * Add support for deprecation logging options.
    [README](README.md#deprecation-warnings),
    [PR #258](capistrano/sshkit#258)
    @robd
  * Quote environment variable values.
    [PR #250](capistrano/sshkit#250)
    @Sinjo - Chris Sinjakli
  * Simplified formatter hierarchy.
    [PR #248](capistrano/sshkit#248)
    @robd
    * `SimpleText` formatter now extends `Pretty`, rather than duplicating.
  * Hide ANSI color escape sequences when outputting to a file.
    [README](README.md#output-colors),
    [Issue #245](capistrano/sshkit#245),
    [PR #246](capistrano/sshkit#246)
    @robd
    * Now only color the output if it is associated with a tty,
      or the `SSHKIT_COLOR` environment variable is set.
  * Removed broken support for assigning an `IO` to the `output` config option.
    [Issue #243](capistrano/sshkit#243),
    [PR #244](capistrano/sshkit#244)
    @robd
    * Use `SSHKit.config.output = SSHKit::Formatter::SimpleText.new($stdin)` instead
  * Added support for `:interaction_handler` option on commands.
    [PR #234](capistrano/sshkit#234),
    [PR #242](capistrano/sshkit#242)
    @robd
  * Removed partially supported `TRACE` log level.
    [2aa7890](capistrano/sshkit@2aa7890)
    @robd
  * Add support for the `:strip` option to the `capture` method and strip by default on the `Local` backend.
    [PR #239](capistrano/sshkit#239),
    [PR #249](capistrano/sshkit#249)
    @robd
    * The `Local` backend now strips by default to be consistent with the `Netssh` one.
    * This reverses change [7d15a9a](capistrano/sshkit@7d15a9a) to the `Local` capture API to remove stripping by default.
    * If you require the raw, unstripped output, pass the `strip: false` option: `capture(:ls, strip: false)`
  * Simplified backend hierarchy.
    [PR #235](capistrano/sshkit#235),
    [PR #237](capistrano/sshkit#237)
    @robd
    * Moved duplicate implementations of `make`, `rake`, `test`, `capture`, `background` on to `Abstract` backend.
    * Backend implementations now only need to implement `execute_command`, `upload!` and `download!`
    * Removed `Printer` from backend hierarchy for `Local` and `Netssh` backends (they now just extend `Abstract`)
    * Removed unused `Net::SSH:LogLevelShim`
  * Removed dependency on the `colorize` gem. SSHKit now implements its own ANSI color logic, with no external dependencies. Note that SSHKit now only supports the `:bold` or plain modes. Other modes will be gracefully ignored. [#263](capistrano/sshkit#263)
  * New API for setting the formatter: `use_format`. This differs from `format=` in that it accepts options or arguments that will be passed to the formatter's constructor. The `format=` syntax will be deprecated in a future release. [#295](capistrano/sshkit#295)
  * SSHKit now immediately raises a `NameError` if you try to set a formatter that does not exist. [#295](capistrano/sshkit#295)
jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Dec 14, 2015
Update ruby-sshkit to 1.8.1.
e938525 
## 1.8.1

  * Change license to MIT, thanks to all the patient contributors who gave
    their permissions.

## 1.8.0

  * add SSHKit::Backend::ConnectionPool#close_connections
    [PR #285](capistrano/sshkit#285)
    @akm
  * Clean up rubocop lint warnings
    [PR #275](capistrano/sshkit#275)
    @cshaffer
    * Prepend unused parameter names with an underscore
    * Prefer “safe assignment in condition”
    * Disambiguate regexp literals with parens
    * Prefer `sprintf` over `String#%`
    * No longer shadow `caller_line` variable in `DeprecationLogger`
    * Rescue `StandardError` instead of `Exception`
    * Remove useless `private` access modifier in `TestAbstract`
    * Disambiguate block operator with parens
    * Disambiguate between grouped expression and method params
    * Remove assertion in `TestHost#test_assert_hosts_compare_equal` that compares something with itself
  * Export environment variables and execute command in a subshell.
    [PR #273](capistrano/sshkit#273)
    @kuon
  * Introduce `log_command_start`, `log_command_data`, `log_command_exit` methods on `Formatter`
    [PR #257](capistrano/sshkit#257)
    @robd
    * Deprecate `@stdout` and `@stderr` accessors on `Command`
  * Add support for deprecation logging options.
    [README](README.md#deprecation-warnings),
    [PR #258](capistrano/sshkit#258)
    @robd
  * Quote environment variable values.
    [PR #250](capistrano/sshkit#250)
    @Sinjo - Chris Sinjakli
  * Simplified formatter hierarchy.
    [PR #248](capistrano/sshkit#248)
    @robd
    * `SimpleText` formatter now extends `Pretty`, rather than duplicating.
  * Hide ANSI color escape sequences when outputting to a file.
    [README](README.md#output-colors),
    [Issue #245](capistrano/sshkit#245),
    [PR #246](capistrano/sshkit#246)
    @robd
    * Now only color the output if it is associated with a tty,
      or the `SSHKIT_COLOR` environment variable is set.
  * Removed broken support for assigning an `IO` to the `output` config option.
    [Issue #243](capistrano/sshkit#243),
    [PR #244](capistrano/sshkit#244)
    @robd
    * Use `SSHKit.config.output = SSHKit::Formatter::SimpleText.new($stdin)` instead
  * Added support for `:interaction_handler` option on commands.
    [PR #234](capistrano/sshkit#234),
    [PR #242](capistrano/sshkit#242)
    @robd
  * Removed partially supported `TRACE` log level.
    [2aa7890](capistrano/sshkit@2aa7890)
    @robd
  * Add support for the `:strip` option to the `capture` method and strip by default on the `Local` backend.
    [PR #239](capistrano/sshkit#239),
    [PR #249](capistrano/sshkit#249)
    @robd
    * The `Local` backend now strips by default to be consistent with the `Netssh` one.
    * This reverses change [7d15a9a](capistrano/sshkit@7d15a9a) to the `Local` capture API to remove stripping by default.
    * If you require the raw, unstripped output, pass the `strip: false` option: `capture(:ls, strip: false)`
  * Simplified backend hierarchy.
    [PR #235](capistrano/sshkit#235),
    [PR #237](capistrano/sshkit#237)
    @robd
    * Moved duplicate implementations of `make`, `rake`, `test`, `capture`, `background` on to `Abstract` backend.
    * Backend implementations now only need to implement `execute_command`, `upload!` and `download!`
    * Removed `Printer` from backend hierarchy for `Local` and `Netssh` backends (they now just extend `Abstract`)
    * Removed unused `Net::SSH:LogLevelShim`
  * Removed dependency on the `colorize` gem. SSHKit now implements its own ANSI color logic, with no external dependencies. Note that SSHKit now only supports the `:bold` or plain modes. Other modes will be gracefully ignored. [#263](capistrano/sshkit#263)
  * New API for setting the formatter: `use_format`. This differs from `format=` in that it accepts options or arguments that will be passed to the formatter's constructor. The `format=` syntax will be deprecated in a future release. [#295](capistrano/sshkit#295)
  * SSHKit now immediately raises a `NameError` if you try to set a formatter that does not exist. [#295](capistrano/sshkit#295)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@robd @leehambley @mattbrictson