Closed
Conversation
Pathname support was added to list and getbinaryfile in 613a324. Fixes the following error when calling String#+ with a Pathname: TypeError: no implicit conversion of Pathname into String Found in the following Net::FTP instance methods: chdir delete gettextfile mdtm mkdir nlst putbinaryfile puttextfile rename rmdir size
Pathname support was added to list and getbinaryfile in 613a324. This commit normalizes these two methods to follow the pattern in the prior commit. I believe the interpolation form is more readable and creates less String objects.
fe419b2 to
b0d0952
Compare
Contributor
Author
|
I'm not quite sure why the CI tests are failing. I tried amending and force-pushing but a different test failed. |
Member
|
LGTM |
Contributor
Author
mmasaki
pushed a commit
to mmasaki/ruby
that referenced
this pull request
May 30, 2015
putbinaryfile, puttextfile, rename, rmdir, size): support Pathname. Patch by Joe Rafaniello. [fix rubyGH-828] git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@49552 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
matzbot
pushed a commit
that referenced
this pull request
Sep 3, 2021
Also flattens `@options.template_stylesheets` when parsing the command lines. Fixes #205 Fixes #828 too ruby/rdoc@857002a763
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Pathname support was added to Net::FTP list and getbinaryfile methods by @shugo in 613a324.
This pull request fixes the following error when calling String#+ with a Pathname:
TypeError: no implicit conversion of Pathname into String
Found in the following Net::FTP instance methods:
chdir
delete
gettextfile
mdtm
mkdir
nlst
putbinaryfile
puttextfile
rename
rmdir
size
Note, this pull request normalizes the changes from 613a324 so each of these methods support Pathnames in the same way, via String interpolation (see the second commit). It's my opinion this is more readable and seems to creates less String objects.
The third commit adds commented tests that I used to recreate the TypeError but don't actually test the methods correctly. I can either do correct tests with someone's help (nlst with returning filelists was not easy to test), remove the commented tests, or leave them commented.
I wasn't sure if I should add a ChangeLog entry or if someone on ruby-core is supposed to do that.
Please advise.
Thanks in advance for your review!
cc @tenderlove