No longer strip whitespace/newlines in capture method on Netssh backend #239
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
leehambley
added a commit
that referenced
this pull request
Apr 27, 2015
No longer strip whitespace/newlines in capture method on Netssh backend
|
Cheers @robd, I'll make an announcement via Gem post install message, or something when we do the release that this might introduce breaking changes, no need to bump the API though, I don't think. |
|
@robd, indeed I should have made this change for Netssh too, but it never occurred to me (I only had the problem locally with parsing 'git' output). I agree that 'capture' staying pure seems to be best, and as Lee has pointed, people should read the message and test before upgrading. |
|
@townsen OK great - thanks for replying. I don't think anyone has actually come across the need for preserving whitespace with the Netssh backend (or at least not that I could find in the issues), so I don't think it was a problem, but good to have it consistent. |
robd
added a commit
to robd/sshkit
that referenced
this pull request
May 12, 2015
This reverses the decision made in capistrano#239. We now reinstate stripping by default to maintain backwards compatibility with the NetSSH backend, but support `strip: false` if full whitespace is needed.
jsonn
pushed a commit
to jsonn/pkgsrc
that referenced
this pull request
Dec 14, 2015
## 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
## 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a breaking change, which I wanted to discuss before merging - I'm not sure what the best course of action is here.
The old
capturebehaviour was tostripwhitespace and newlines from the captured results. I think the main reason for this was to remove trailing newlines from command results to make one liners easier to handle (egcapture(:uname)returnsLinuxas opposed toLinux\n). Unfortunately this meant that you couldn't capture command output with trailing whitespace / newlines. This was discussed in #165 with @townsen, and I believe it was decided to make this change, however this was only changed for theLocalbackend in 7d15a9a, but not for theNetsshone. I think this was an omission, so, if possible, I would like to fix this inconsistency now.From an API (purist?) perspective, I think it makes sense to remove the
stripmethod from theNetsshbackend too as per the original decision in #165 and this PR implements that. But this might cause quite widespread breakages in client code (although easily fixed as mentioned in #165). This might mean the need for a major release version bump - I'm not sure what the policy is here.Another option would be to revert the change made in 7d15a9a (ie put back the
stripbehaviour onLocalbackendcapture), and introduce a new method (egcapture_raw) which doesn't strip. Of course the downside of this is clutter on theAbstractbackend API which must be maintained, but this would mean that people wouldn't have to change their client code.