Skip to content

Commit

Permalink
Use named parameters for File::new.
Browse files Browse the repository at this point in the history
This is a breaking change, but now is the time to do this as we've
already done the same for `Entry::new`.
  • Loading branch information
hainesr committed Jun 8, 2021
1 parent d9ce5d8 commit 45583da
Show file tree
Hide file tree
Showing 13 changed files with 43 additions and 60 deletions.
2 changes: 1 addition & 1 deletion .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ Style/NumericPredicate:
- 'test/file_split_test.rb'
- 'test/test_helper.rb'

# Offense count: 6
# Offense count: 1
# Configuration parameters: AllowedMethods.
# AllowedMethods: respond_to_missing?
Style/OptionalBooleanParameter:
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ input_filenames = ['image.jpg', 'description.txt', 'stats.csv']

zipfile_name = "/Users/me/Desktop/archive.zip"

Zip::File.open(zipfile_name, Zip::File::CREATE) do |zipfile|
Zip::File.open(zipfile_name, create: true) do |zipfile|
input_filenames.each do |filename|
# Two arguments:
# - The name of the file as it will appear in the archive
Expand Down Expand Up @@ -89,7 +89,7 @@ class ZipFileGenerator
def write
entries = Dir.entries(@input_dir) - %w[. ..]

::Zip::File.open(@output_file, ::Zip::File::CREATE) do |zipfile|
::Zip::File.open(@output_file, create: true) do |zipfile|
write_entries entries, '', zipfile
end
end
Expand Down Expand Up @@ -304,7 +304,7 @@ Where X is an integer between 0 and 9, inclusive. If this option is set to 0 (`Z
This can also be set for each archive as an option to `Zip::File`:

```ruby
Zip::File.open('foo.zip', Zip::File::CREATE, {compression_level: 9}) do |zip|
Zip::File.open('foo.zip', create:true, compression_level: 9) do |zip|
zip.add ...
end
```
Expand Down
19 changes: 9 additions & 10 deletions lib/zip/file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ module Zip
#
# require 'zip'
#
# Zip::File.open("my.zip", Zip::File::CREATE) {
# Zip::File.open("my.zip", create: true) {
# |zipfile|
# zipfile.get_output_stream("first.txt") { |f| f.puts "Hello from ZipFile" }
# zipfile.mkdir("a_dir")
Expand All @@ -35,7 +35,7 @@ module Zip
#
# require 'zip'
#
# Zip::File.open("my.zip", Zip::File::CREATE) {
# Zip::File.open("my.zip", create: true) {
# |zipfile|
# puts zipfile.read("first.txt")
# zipfile.remove("first.txt")
Expand All @@ -45,7 +45,6 @@ module Zip
# interface for accessing the filesystem, ie. the File and Dir classes.

class File < CentralDirectory
CREATE = true
SPLIT_SIGNATURE = 0x08074b50
ZIP64_EOCD_SIGNATURE = 0x06064b50
MAX_SEGMENT_SIZE = 3_221_225_472
Expand All @@ -67,9 +66,9 @@ class File < CentralDirectory
# Returns the zip files comment, if it has one
attr_accessor :comment

# Opens a zip archive. Pass true as the second parameter to create
# Opens a zip archive. Pass create: true to create
# a new archive if it doesn't exist already.
def initialize(path_or_io, create = false, buffer = false, options = {})
def initialize(path_or_io, create: false, buffer: false, **options)
super()
options = DEFAULT_RESTORE_OPTIONS
.merge(compression_level: ::Zip.default_compression)
Expand Down Expand Up @@ -116,8 +115,8 @@ class << self
# Similar to ::new. If a block is passed the Zip::File object is passed
# to the block and is automatically closed afterwards, just as with
# ruby's builtin File::open method.
def open(file_name, create = false, options = {})
zf = ::Zip::File.new(file_name, create, false, options)
def open(file_name, create: false, **options)
zf = ::Zip::File.new(file_name, create: create, **options)
return zf unless block_given?

begin
Expand All @@ -130,7 +129,7 @@ def open(file_name, create = false, options = {})
# Same as #open. But outputs data to a buffer instead of a file
def add_buffer
io = ::StringIO.new
zf = ::Zip::File.new(io, true, true)
zf = ::Zip::File.new(io, create: true, buffer: true)
yield zf
zf.write_buffer(io)
end
Expand All @@ -139,7 +138,7 @@ def add_buffer
# stream, and outputs data to a buffer.
# (This can be used to extract data from a
# downloaded zip archive without first saving it to disk.)
def open_buffer(io, options = {})
def open_buffer(io, **options)
unless IO_METHODS.map { |method| io.respond_to?(method) }.all? || io.kind_of?(String)
raise "Zip::File.open_buffer expects a String or IO-like argument (responds to #{IO_METHODS.join(', ')}). Found: #{io.class}"
end
Expand All @@ -149,7 +148,7 @@ def open_buffer(io, options = {})
# https://github.com/rubyzip/rubyzip/issues/119
io.binmode if io.respond_to?(:binmode)

zf = ::Zip::File.new(io, true, true, options)
zf = ::Zip::File.new(io, create: true, buffer: true, **options)
return zf unless block_given?

yield zf
Expand Down
2 changes: 1 addition & 1 deletion lib/zip/filesystem.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ module Zip
#
# require 'zip/filesystem'
#
# Zip::File.open("my.zip", Zip::File::CREATE) {
# Zip::File.open("my.zip", create: true) {
# |zipfile|
# zipfile.file.open("first.txt", "w") { |f| f.puts "Hello world" }
# zipfile.dir.mkdir("mydir")
Expand Down
2 changes: 1 addition & 1 deletion samples/example_filesystem.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

File.delete(EXAMPLE_ZIP) if File.exist?(EXAMPLE_ZIP)

Zip::File.open(EXAMPLE_ZIP, Zip::File::CREATE) do |zf|
Zip::File.open(EXAMPLE_ZIP, create: true) do |zf|
zf.file.open('file1.txt', 'w') { |os| os.write 'first file1.txt' }
zf.dir.mkdir('dir1')
zf.dir.chdir('dir1')
Expand Down
2 changes: 1 addition & 1 deletion samples/example_recursive.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def initialize(input_dir, output_file)
def write
entries = Dir.entries(@input_dir) - %w[. ..]

::Zip::File.open(@output_file, ::Zip::File::CREATE) do |zipfile|
::Zip::File.open(@output_file, create: true) do |zipfile|
write_entries entries, '', zipfile
end
end
Expand Down
6 changes: 3 additions & 3 deletions test/case_sensitivity_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def test_add_case_sensitive
::Zip.case_insensitive_match = false

SRC_FILES.each { |fn, _en| assert(::File.exist?(fn)) }
zf = ::Zip::File.new(EMPTY_FILENAME, ::Zip::File::CREATE)
zf = ::Zip::File.new(EMPTY_FILENAME, create: true)

SRC_FILES.each { |fn, en| zf.add(en, fn) }
zf.close
Expand All @@ -38,7 +38,7 @@ def test_add_case_insensitive
::Zip.case_insensitive_match = true

SRC_FILES.each { |fn, _en| assert(::File.exist?(fn)) }
zf = ::Zip::File.new(EMPTY_FILENAME, ::Zip::File::CREATE)
zf = ::Zip::File.new(EMPTY_FILENAME, create: true)

assert_raises Zip::EntryExistsError do
SRC_FILES.each { |fn, en| zf.add(en, fn) }
Expand All @@ -50,7 +50,7 @@ def test_add_case_sensitive_read_case_insensitive
::Zip.case_insensitive_match = false

SRC_FILES.each { |fn, _en| assert(::File.exist?(fn)) }
zf = ::Zip::File.new(EMPTY_FILENAME, ::Zip::File::CREATE)
zf = ::Zip::File.new(EMPTY_FILENAME, create: true)

SRC_FILES.each { |fn, en| zf.add(en, fn) }
zf.close
Expand Down
2 changes: 1 addition & 1 deletion test/entry_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ def test_store_file_without_compression
z.write_zip64_support = false
end

zipfile = Zip::File.open(tmp_zip, Zip::File::CREATE)
zipfile = Zip::File.open(tmp_zip, create: true)

mimetype_entry = Zip::Entry.new(
zipfile, # @zipfile
Expand Down
2 changes: 1 addition & 1 deletion test/file_extract_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ def test_extract_incorrect_size
true_size = 500_000
fake_size = 1

::Zip::File.open(real_zip, ::Zip::File::CREATE) do |zf|
::Zip::File.open(real_zip, create: true) do |zf|
zf.get_output_stream(file_name) do |os|
os.write 'a' * true_size
end
Expand Down
20 changes: 10 additions & 10 deletions test/file_options_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@ def test_restore_permissions_true
::FileUtils.cp(TXTPATH, TXTPATH_755)
::File.chmod(0o755, TXTPATH_755)

::Zip::File.open(ZIPPATH, true) do |zip|
::Zip::File.open(ZIPPATH, create: true) do |zip|
zip.add(ENTRY_1, TXTPATH)
zip.add(ENTRY_2, TXTPATH_600)
zip.add(ENTRY_3, TXTPATH_755)
end

::Zip::File.open(ZIPPATH, false, restore_permissions: true) do |zip|
::Zip::File.open(ZIPPATH, restore_permissions: true) do |zip|
zip.extract(ENTRY_1, EXTPATH_1)
zip.extract(ENTRY_2, EXTPATH_2)
zip.extract(ENTRY_3, EXTPATH_3)
Expand All @@ -54,13 +54,13 @@ def test_restore_permissions_false
::FileUtils.cp(TXTPATH, TXTPATH_755)
::File.chmod(0o755, TXTPATH_755)

::Zip::File.open(ZIPPATH, true) do |zip|
::Zip::File.open(ZIPPATH, create: true) do |zip|
zip.add(ENTRY_1, TXTPATH)
zip.add(ENTRY_2, TXTPATH_600)
zip.add(ENTRY_3, TXTPATH_755)
end

::Zip::File.open(ZIPPATH, false, restore_permissions: false) do |zip|
::Zip::File.open(ZIPPATH, restore_permissions: false) do |zip|
zip.extract(ENTRY_1, EXTPATH_1)
zip.extract(ENTRY_2, EXTPATH_2)
zip.extract(ENTRY_3, EXTPATH_3)
Expand All @@ -79,7 +79,7 @@ def test_restore_permissions_as_default
::FileUtils.cp(TXTPATH, TXTPATH_755)
::File.chmod(0o755, TXTPATH_755)

::Zip::File.open(ZIPPATH, true) do |zip|
::Zip::File.open(ZIPPATH, create: true) do |zip|
zip.add(ENTRY_1, TXTPATH)
zip.add(ENTRY_2, TXTPATH_600)
zip.add(ENTRY_3, TXTPATH_755)
Expand All @@ -97,12 +97,12 @@ def test_restore_permissions_as_default
end

def test_restore_times_true
::Zip::File.open(ZIPPATH, true) do |zip|
::Zip::File.open(ZIPPATH, create: true) do |zip|
zip.add(ENTRY_1, TXTPATH)
zip.add_stored(ENTRY_2, TXTPATH)
end

::Zip::File.open(ZIPPATH, false, restore_times: true) do |zip|
::Zip::File.open(ZIPPATH, restore_times: true) do |zip|
zip.extract(ENTRY_1, EXTPATH_1)
zip.extract(ENTRY_2, EXTPATH_2)
end
Expand All @@ -112,12 +112,12 @@ def test_restore_times_true
end

def test_restore_times_false
::Zip::File.open(ZIPPATH, true) do |zip|
::Zip::File.open(ZIPPATH, create: true) do |zip|
zip.add(ENTRY_1, TXTPATH)
zip.add_stored(ENTRY_2, TXTPATH)
end

::Zip::File.open(ZIPPATH, false, restore_times: false) do |zip|
::Zip::File.open(ZIPPATH, restore_times: false) do |zip|
zip.extract(ENTRY_1, EXTPATH_1)
zip.extract(ENTRY_2, EXTPATH_2)
end
Expand All @@ -127,7 +127,7 @@ def test_restore_times_false
end

def test_restore_times_true_as_default
::Zip::File.open(ZIPPATH, true) do |zip|
::Zip::File.open(ZIPPATH, create: true) do |zip|
zip.add(ENTRY_1, TXTPATH)
zip.add_stored(ENTRY_2, TXTPATH)
end
Expand Down
2 changes: 1 addition & 1 deletion test/file_permissions_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def assert_matching_permissions(expected_file, actual_file)
end

def create_files
::Zip::File.open(ZIPNAME, ::Zip::File::CREATE) do |zip|
::Zip::File.open(ZIPNAME, create: true) do |zip|
zip.comment = 'test'
end

Expand Down
36 changes: 10 additions & 26 deletions test/file_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,7 @@ def test_create_from_scratch_to_buffer
def test_create_from_scratch
comment = 'a short comment'

zf = ::Zip::File.new(EMPTY_FILENAME, ::Zip::File::CREATE)
zf.get_output_stream('myFile') { |os| os.write 'myFile contains just this' }
zf.mkdir('dir1')
zf.comment = comment
zf.close

zf_read = ::Zip::File.new(EMPTY_FILENAME)
assert_equal(comment, zf_read.comment)
assert_equal(2, zf_read.entries.length)
end

def test_create_from_scratch_with_old_create_parameter
comment = 'a short comment'

zf = ::Zip::File.new(EMPTY_FILENAME, 1)
zf = ::Zip::File.new(EMPTY_FILENAME, create: true)
zf.get_output_stream('myFile') { |os| os.write 'myFile contains just this' }
zf.mkdir('dir1')
zf.comment = comment
Expand Down Expand Up @@ -184,7 +170,7 @@ def test_open_buffer_without_block
end

def test_cleans_up_tempfiles_after_close
zf = ::Zip::File.new(EMPTY_FILENAME, ::Zip::File::CREATE)
zf = ::Zip::File.new(EMPTY_FILENAME, create: true)
zf.get_output_stream('myFile') do |os|
@tempfile_path = os.path
os.write 'myFile contains just this'
Expand All @@ -208,9 +194,7 @@ def test_add_different_compression
sizes = []

files.each do |name, comp|
zf = ::Zip::File.new(
name, ::Zip::File::CREATE, false, { compression_level: comp }
)
zf = ::Zip::File.new(name, create: true, compression_level: comp)

zf.add(entry_name, src_file)
zf.close
Expand Down Expand Up @@ -243,7 +227,7 @@ def test_add_different_compression_as_default

files.each do |name, comp|
::Zip.default_compression = comp
zf = ::Zip::File.new(name, ::Zip::File::CREATE)
zf = ::Zip::File.new(name, create: true)

zf.add(entry_name, src_file)
zf.close
Expand All @@ -268,7 +252,7 @@ def test_add_stored
src_file = 'test/data/file2.txt'
entry_name = 'newEntryName.rb'
assert(::File.exist?(src_file))
zf = ::Zip::File.new(EMPTY_FILENAME, ::Zip::File::CREATE)
zf = ::Zip::File.new(EMPTY_FILENAME, create: true)
zf.add_stored(entry_name, src_file)
zf.close

Expand All @@ -294,7 +278,7 @@ def test_recover_permissions_after_add_files_to_archive
::File.chmod(0o664, src_zip)
assert_equal(0o100664, ::File.stat(src_zip).mode)

zf = ::Zip::File.new(src_zip, ::Zip::File::CREATE)
zf = ::Zip::File.new(src_zip, create: true)
zf.add('newEntryName.rb', 'test/data/file2.txt')
zf.close

Expand Down Expand Up @@ -385,7 +369,7 @@ def test_rename_with_each
::File.unlink(zf_name) if ::File.exist?(zf_name)
arr = []
arr_renamed = []
::Zip::File.open(zf_name, ::Zip::File::CREATE) do |zf|
::Zip::File.open(zf_name, create: true) do |zf|
zf.mkdir('test')
arr << 'test/'
arr_renamed << 'Ztest/'
Expand All @@ -398,7 +382,7 @@ def test_rename_with_each
zf = ::Zip::File.open(zf_name)
assert_equal(zf.entries.map(&:name), arr)
zf.close
Zip::File.open(zf_name, 'wb') do |z|
Zip::File.open(zf_name) do |z|
z.each do |f|
z.rename(f, "Z#{f.name}")
end
Expand Down Expand Up @@ -522,7 +506,7 @@ def test_commit
def test_double_commit(filename = 'test/data/generated/double_commit_test.zip')
::FileUtils.touch('test/data/generated/test_double_commit1.txt')
::FileUtils.touch('test/data/generated/test_double_commit2.txt')
zf = ::Zip::File.open(filename, ::Zip::File::CREATE)
zf = ::Zip::File.open(filename, create: true)
zf.add('test1.txt', 'test/data/generated/test_double_commit1.txt')
zf.commit
zf.add('test2.txt', 'test/data/generated/test_double_commit2.txt')
Expand Down Expand Up @@ -695,7 +679,7 @@ def test_preserve_file_order
def test_streaming
fname = ::File.join(__dir__, '..', 'README.md')
zname = 'test/data/generated/README.zip'
Zip::File.open(zname, Zip::File::CREATE) do |zipfile|
Zip::File.open(zname, create: true) do |zipfile|
zipfile.get_output_stream(File.basename(fname)) do |f|
f.puts File.read(fname)
end
Expand Down
2 changes: 1 addition & 1 deletion test/unicode_file_names_and_comments_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def test_unicode_file_name

def test_unicode_comment
str = '渠道升级'
::Zip::File.open(FILENAME, Zip::File::CREATE) do |z|
::Zip::File.open(FILENAME, create: true) do |z|
z.comment = str
end

Expand Down

0 comments on commit 45583da

Please sign in to comment.