From a7ff007afc81aabc17c2a268d0f1be3a28ee4501 Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Thu, 20 Jan 2022 09:42:30 -0400 Subject: [PATCH 01/13] Require 'english' throughout, to define $INPUT_RECORD_SEPARATOR It was only required for the MemoryFileSystem, not the real FileSystem, so writing an array would concatenate all the lines together without any spaces or newlines. --- lib/dry/files.rb | 2 ++ lib/dry/files/memory_file_system/node.rb | 3 +-- spec/integration/dry/files_spec.rb | 7 ++++++- spec/unit/dry/files/file_system_spec.rb | 1 - spec/unit/dry/files/memory_file_system/node_spec.rb | 3 +-- spec/unit/dry/files/memory_file_system_spec.rb | 1 - 6 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/dry/files.rb b/lib/dry/files.rb index 5f7814c..d37b58a 100644 --- a/lib/dry/files.rb +++ b/lib/dry/files.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "english" + # dry-rb is a collection of next-generation Ruby libraries # # @api public diff --git a/lib/dry/files/memory_file_system/node.rb b/lib/dry/files/memory_file_system/node.rb index 7be84a3..81ce934 100644 --- a/lib/dry/files/memory_file_system/node.rb +++ b/lib/dry/files/memory_file_system/node.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require "English" require "stringio" module Dry @@ -216,7 +215,7 @@ def readlines # @since 0.1.0 # @api private def write(*content) - @content = StringIO.new(content.join($RS)) + @content = StringIO.new(content.join($INPUT_RECORD_SEPARATOR)) @mode = DEFAULT_FILE_MODE end diff --git a/spec/integration/dry/files_spec.rb b/spec/integration/dry/files_spec.rb index 9768e95..f4ea638 100644 --- a/spec/integration/dry/files_spec.rb +++ b/spec/integration/dry/files_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "securerandom" -require "English" RSpec.describe Dry::Files do let(:root) { Pathname.new(Dir.pwd).join("tmp", SecureRandom.uuid).tap(&:mkpath) } @@ -11,6 +10,12 @@ FileUtils.remove_entry_secure(root) end + describe "$INPUT_RECORD_SEPARATOR" do + it "is not nil" do + expect($INPUT_RECORD_SEPARATOR).not_to be_nil + end + end + describe "#touch" do it "creates an empty file" do path = root.join("touch") diff --git a/spec/unit/dry/files/file_system_spec.rb b/spec/unit/dry/files/file_system_spec.rb index d1b7ade..bedeaa5 100644 --- a/spec/unit/dry/files/file_system_spec.rb +++ b/spec/unit/dry/files/file_system_spec.rb @@ -2,7 +2,6 @@ require "dry/files/file_system" require "securerandom" -require "English" RSpec.describe Dry::Files::FileSystem do let(:root) { Pathname.new(Dir.pwd).join("tmp", SecureRandom.uuid).tap(&:mkpath) } diff --git a/spec/unit/dry/files/memory_file_system/node_spec.rb b/spec/unit/dry/files/memory_file_system/node_spec.rb index adffa30..c8640d4 100644 --- a/spec/unit/dry/files/memory_file_system/node_spec.rb +++ b/spec/unit/dry/files/memory_file_system/node_spec.rb @@ -1,12 +1,11 @@ # frozen_string_literal: true require "dry/files/memory_file_system/node" -require "English" RSpec.describe Dry::Files::MemoryFileSystem::Node do subject { described_class.new(path) } let(:path) { "/usr" } - let(:newline) { $RS } + let(:newline) { $INPUT_RECORD_SEPARATOR } describe ".root" do subject { described_class.root } diff --git a/spec/unit/dry/files/memory_file_system_spec.rb b/spec/unit/dry/files/memory_file_system_spec.rb index d455766..1c49c53 100644 --- a/spec/unit/dry/files/memory_file_system_spec.rb +++ b/spec/unit/dry/files/memory_file_system_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "dry/files/memory_file_system" -require "English" RSpec.describe Dry::Files::MemoryFileSystem do let(:newline) { $INPUT_RECORD_SEPARATOR } From fbfaed8a0636098a1c268527707e96cbbf7b475d Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Fri, 21 Jan 2022 10:31:34 -0400 Subject: [PATCH 02/13] Fix newlines on reading and writing Specifically, we don't want to *join* the lines, but rather append to each line. This lets us avoid having to manually add a newline to the end, and handle them all at once. Also, we chomp every line on read (and before write) so we don't duplicate any newlines --- lib/dry/files.rb | 52 ++++++++++--------------- lib/dry/files/file_system.rb | 11 +++++- spec/integration/dry/files_spec.rb | 18 ++++----- spec/unit/dry/files/file_system_spec.rb | 2 +- 4 files changed, 41 insertions(+), 42 deletions(-) diff --git a/lib/dry/files.rb b/lib/dry/files.rb index d37b58a..99b0cd6 100644 --- a/lib/dry/files.rb +++ b/lib/dry/files.rb @@ -27,8 +27,11 @@ class Files # # @since 0.1.0 # @api public - def initialize(memory: false, adapter: Adapter.call(memory: memory)) + def initialize(memory: false, + adapter: Adapter.call(memory: memory), + newline: $INPUT_RECORD_SEPARATOR) @adapter = adapter + @newline = newline end # Read file content @@ -66,14 +69,15 @@ def touch(path) # All the intermediate directories are created. # # @param path [String,Pathname] the path to file - # @param content [String, Array] the content to write + # @param lines [String, Array] the content to write # # @raise [Dry::Files::IOError] in case of I/O error # # @since 0.1.0 # @api public - def write(path, *content) - adapter.write(path, *content) + def write(path, lines) + joined_lines = Array(lines).flatten.map { |line| line.chomp.concat(newline) }.join + adapter.write(path, joined_lines) end # Returns a new string formed by joining the strings using Operating @@ -291,7 +295,7 @@ def executable?(path) # @api public def unshift(path, line) content = adapter.readlines(path) - content.unshift(newline(line)) + content.unshift(line) write(path, content) end @@ -311,8 +315,7 @@ def append(path, contents) mkdir_p(path) content = adapter.readlines(path) - content << newline unless newline?(content.last) - content << newline(contents) + content << contents write(path, content) end @@ -332,7 +335,7 @@ def append(path, contents) # @api public def replace_first_line(path, target, replacement) content = adapter.readlines(path) - content[index(content, path, target)] = newline(replacement) + content[index(content, path, target)] = replacement write(path, content) end @@ -352,7 +355,7 @@ def replace_first_line(path, target, replacement) # @api public def replace_last_line(path, target, replacement) content = adapter.readlines(path) - content[-index(content.reverse, path, target) - CONTENT_OFFSET] = newline(replacement) + content[-index(content.reverse, path, target) - CONTENT_OFFSET] = replacement write(path, content) end @@ -737,11 +740,6 @@ def remove_block(path, target) private - # @since 0.1.0 - # @api private - NEW_LINE = $/ # rubocop:disable Style/SpecialGlobalVars - private_constant :NEW_LINE - # @since 0.1.0 # @api private CONTENT_OFFSET = 1 @@ -781,17 +779,9 @@ def remove_block(path, target) # @api private attr_reader :adapter - # @since 0.1.0 - # @api private - def newline(line = nil) - "#{line}#{NEW_LINE}" - end - - # @since 0.1.0 + # @since x.x.x # @api private - def newline?(content) - content.end_with?(NEW_LINE) - end + attr_reader :newline # @since 0.1.0 # @api private @@ -819,7 +809,7 @@ def _inject_line_before(path, target, contents, finder) content = adapter.readlines(path) i = finder.call(content, path, target) - content.insert(i, newline(contents)) + content.insert(i, contents) write(path, content) end @@ -829,7 +819,7 @@ def _inject_line_after(path, target, contents, finder) content = adapter.readlines(path) i = finder.call(content, path, target) - content.insert(i + CONTENT_OFFSET, newline(contents)) + content.insert(i + CONTENT_OFFSET, contents) write(path, content) end @@ -837,13 +827,13 @@ def _inject_line_after(path, target, contents, finder) # @api private def _offset_block_lines(contents, offset) contents.map do |line| - if line.match?(NEW_LINE) - line = line.split(NEW_LINE) - _offset_block_lines(line, offset) + case line.lines(chomp: true) + in [line] + offset + line else - offset + line + NEW_LINE + _offset_block_lines(line.lines, offset) end - end.join + end end # @since 0.1.0 diff --git a/lib/dry/files/file_system.rb b/lib/dry/files/file_system.rb index e46d65c..2c83674 100644 --- a/lib/dry/files/file_system.rb +++ b/lib/dry/files/file_system.rb @@ -86,7 +86,16 @@ def read(path, *args, **kwargs) # @api private def readlines(path, *args) with_error_handling do - file.readlines(path, *args) + file.readlines(path, *args, chomp: true).then do |lines| + # The last item will be an empty string if the file has a + # trailing newline. We don't want that in our contents, + # especially since we append a newline to every line during `write` + if lines.last && lines.last.empty? + lines[0..-2] + else + lines + end + end end end diff --git a/spec/integration/dry/files_spec.rb b/spec/integration/dry/files_spec.rb index f4ea638..004e7d2 100644 --- a/spec/integration/dry/files_spec.rb +++ b/spec/integration/dry/files_spec.rb @@ -57,9 +57,9 @@ describe "#read" do it "reads file" do path = root.join("read") - subject.write(path, expected = "Hello#{newline}World") + subject.write(path, "Hello#{newline}World") - expect(subject.read(path)).to eq(expected) + expect(subject.read(path)).to eq("Hello#{newline}World#{newline}") end it "raises error when path is a directory" do @@ -90,7 +90,7 @@ subject.write(path, "Hello#{newline}World") expect(path).to exist - expect(path).to have_content("Hello#{newline}World") + expect(path).to have_content("Hello#{newline}World#{newline}") end it "creates intermediate directories" do @@ -98,7 +98,7 @@ subject.write(path, ":)") expect(path).to exist - expect(path).to have_content(":)") + expect(path).to have_content(":)#{newline}") end it "overwrites file when it already exists" do @@ -107,7 +107,7 @@ subject.write(path, "new words") expect(path).to exist - expect(path).to have_content("new words") + expect(path).to have_content("new words#{newline}") end it "raises error when path isn't writeable" do @@ -143,7 +143,7 @@ subject.cp(source, destination) expect(destination).to exist - expect(destination).to have_content("the source") + expect(destination).to have_content("the source#{newline}") end it "creates intermediate directories" do @@ -154,7 +154,7 @@ subject.cp(source, destination) expect(destination).to exist - expect(destination).to have_content("the source for intermediate directories") + expect(destination).to have_content("the source for intermediate directories#{newline}") end it "overrides already existing file" do @@ -166,7 +166,7 @@ subject.cp(source, destination) expect(destination).to exist - expect(destination).to have_content("the source") + expect(destination).to have_content("the source#{newline}") end it "raises error when source cannot be found" do @@ -460,7 +460,7 @@ class Unshift subject.write(path, content) subject.unshift(path, "root to: 'home#index'") - expected = "root to: 'home#index'#{newline}get '/tires', to: 'sunshine#index'" + expected = "root to: 'home#index'#{newline}get '/tires', to: 'sunshine#index'#{newline}" expect(path).to have_content(expected) end diff --git a/spec/unit/dry/files/file_system_spec.rb b/spec/unit/dry/files/file_system_spec.rb index bedeaa5..abc95fa 100644 --- a/spec/unit/dry/files/file_system_spec.rb +++ b/spec/unit/dry/files/file_system_spec.rb @@ -86,7 +86,7 @@ path = root.join("readlines-file") subject.write(path, "hello#{newline}world") - expect(subject.readlines(path)).to eq(%W[hello#{newline} world]) + expect(subject.readlines(path)).to eq(%w[hello world]) end it "reads empty file and returns empty array" do From b4e71069d1d84ec4582b1bbb330609f13969bb32 Mon Sep 17 00:00:00 2001 From: Ryan Bigg Date: Mon, 3 May 2021 19:13:25 +1000 Subject: [PATCH 03/13] Chomp lines for MemoryFileSystem#readline This is so when they're read out, they do not contain line breaks at the end of each line's string. Later on, when this content is passed to write, it is joined with , which will add newlines --- lib/dry/files/memory_file_system/node.rb | 2 +- spec/unit/dry/files/memory_file_system/node_spec.rb | 2 +- spec/unit/dry/files/memory_file_system_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/dry/files/memory_file_system/node.rb b/lib/dry/files/memory_file_system/node.rb index 81ce934..b159dc0 100644 --- a/lib/dry/files/memory_file_system/node.rb +++ b/lib/dry/files/memory_file_system/node.rb @@ -202,7 +202,7 @@ def readlines raise NotMemoryFileError, segment unless file? @content.rewind - @content.readlines + @content.readlines(chomp: true) end # Write file contents diff --git a/spec/unit/dry/files/memory_file_system/node_spec.rb b/spec/unit/dry/files/memory_file_system/node_spec.rb index c8640d4..af86157 100644 --- a/spec/unit/dry/files/memory_file_system/node_spec.rb +++ b/spec/unit/dry/files/memory_file_system/node_spec.rb @@ -144,7 +144,7 @@ expect(subject.readlines).to eq(["foo"]) subject.write(%w[foo bar]) - expect(subject.readlines).to eq(["foo#{newline}", "bar"]) + expect(subject.readlines).to eq(["foo", "bar"]) end it "raises error when not file" do diff --git a/spec/unit/dry/files/memory_file_system_spec.rb b/spec/unit/dry/files/memory_file_system_spec.rb index 1c49c53..228646c 100644 --- a/spec/unit/dry/files/memory_file_system_spec.rb +++ b/spec/unit/dry/files/memory_file_system_spec.rb @@ -73,7 +73,7 @@ path = subject.join("readlines-file") subject.write(path, "hello#{newline}world") - expect(subject.readlines(path)).to eq(%W[hello#{newline} world]) + expect(subject.readlines(path)).to eq(%w[hello world]) end it "reads empty file and returns empty array" do From 182c874b0f99e51afddd052ad06adc42ed2f93a4 Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Fri, 21 Jan 2022 11:05:19 -0400 Subject: [PATCH 04/13] Change file system adapters to only write Strings This separates the concerns, so the adapters only write **String**s, they don't do any joining or coercion. --- lib/dry/files/file_system.rb | 6 ++++-- lib/dry/files/memory_file_system.rb | 12 +++++++----- spec/unit/dry/files/file_system_spec.rb | 8 ++++++++ spec/unit/dry/files/memory_file_system_spec.rb | 8 ++++++++ 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/lib/dry/files/file_system.rb b/lib/dry/files/file_system.rb index 2c83674..58a6e03 100644 --- a/lib/dry/files/file_system.rb +++ b/lib/dry/files/file_system.rb @@ -126,13 +126,15 @@ def touch(path, **kwargs) # All the intermediate directories are created. # # @param path [String,Pathname] the path to file - # @param content [String, Array] the content to write + # @param content [String] the content to write # # @raise [Dry::Files::IOError] in case of I/O error # # @since 0.1.0 # @api private - def write(path, *content) + def write(path, content) + raise ArgumentError, "Must be string (use `join` or `to_s`)" unless content.is_a?(String) + mkdir_p(path) self.open(path, WRITE_MODE) do |f| diff --git a/lib/dry/files/memory_file_system.rb b/lib/dry/files/memory_file_system.rb index e52af17..fb44b40 100644 --- a/lib/dry/files/memory_file_system.rb +++ b/lib/dry/files/memory_file_system.rb @@ -11,7 +11,7 @@ class Files class MemoryFileSystem # @since 0.1.0 # @api private - EMPTY_CONTENT = nil + EMPTY_CONTENT = "" private_constant :EMPTY_CONTENT require_relative "./memory_file_system/node" @@ -104,12 +104,14 @@ def touch(path) # of an existing file for the given path and content # All the intermediate directories are created. # - # @param path [String, Array] the target path - # @param content [String, Array] the content to write + # @param path [Array] the target path + # @param content [String] the content to write # # @since 0.1.0 # @api private - def write(path, *content) + def write(path, content) + raise ArgumentError, "Must be string (use `join` or `to_s`)" unless content.is_a?(String) + path = Path[path] node = @root @@ -117,7 +119,7 @@ def write(path, *content) node = node.set(segment) end - node.write(*content) + node.write(content) node end diff --git a/spec/unit/dry/files/file_system_spec.rb b/spec/unit/dry/files/file_system_spec.rb index abc95fa..6950814 100644 --- a/spec/unit/dry/files/file_system_spec.rb +++ b/spec/unit/dry/files/file_system_spec.rb @@ -217,6 +217,14 @@ path.chmod(mode) end end + + it "raises error when trying to write non-string" do + path = root.join("write") + expect { subject.write(path, ["new", "words"]) }.to raise_error do |exception| + expect(exception).to be_kind_of(ArgumentError) + expect(exception.message).to eq("Must be string (use `join` or `to_s`)") + end + end end describe "#join" do diff --git a/spec/unit/dry/files/memory_file_system_spec.rb b/spec/unit/dry/files/memory_file_system_spec.rb index 228646c..5f39476 100644 --- a/spec/unit/dry/files/memory_file_system_spec.rb +++ b/spec/unit/dry/files/memory_file_system_spec.rb @@ -188,6 +188,14 @@ path.chmod(mode) end end + + it "raises error when trying to write non-string" do + path = subject.join("write") + expect { subject.write(path, ["new", "words"]) }.to raise_error do |exception| + expect(exception).to be_kind_of(ArgumentError) + expect(exception.message).to eq("Must be string (use `join` or `to_s`)") + end + end end describe "#join" do From 6a5f92dbec6a95db9569f9375aef21c0465b99cd Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Fri, 21 Jan 2022 11:19:49 -0400 Subject: [PATCH 05/13] Leave empty files empty (i.e. without trailing newline) --- lib/dry/files.rb | 1 + spec/integration/dry/files_spec.rb | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/lib/dry/files.rb b/lib/dry/files.rb index 99b0cd6..9521e38 100644 --- a/lib/dry/files.rb +++ b/lib/dry/files.rb @@ -77,6 +77,7 @@ def touch(path) # @api public def write(path, lines) joined_lines = Array(lines).flatten.map { |line| line.chomp.concat(newline) }.join + joined_lines = "" if joined_lines == "\n" # Leave it empty adapter.write(path, joined_lines) end diff --git a/spec/integration/dry/files_spec.rb b/spec/integration/dry/files_spec.rb index 004e7d2..09893a0 100644 --- a/spec/integration/dry/files_spec.rb +++ b/spec/integration/dry/files_spec.rb @@ -93,6 +93,14 @@ expect(path).to have_content("Hello#{newline}World#{newline}") end + it "creates an empty file (without trailing newline)" do + path = root.join("write") + subject.write(path, "") + + expect(path).to exist + expect(path).to have_content("") + end + it "creates intermediate directories" do path = root.join("path", "to", "file", "write") subject.write(path, ":)") From f1f43898c43a17178215f5960005e15007b6c361 Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Fri, 21 Jan 2022 11:24:41 -0400 Subject: [PATCH 06/13] Fix style guide violations --- spec/support/matchers.rb | 2 +- spec/unit/dry/files/file_system_spec.rb | 2 +- spec/unit/dry/files/memory_file_system/node_spec.rb | 2 +- spec/unit/dry/files/memory_file_system_spec.rb | 2 +- spec/unit/dry/files/path_spec.rb | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/support/matchers.rb b/spec/support/matchers.rb index 6a4e0be..18c5463 100644 --- a/spec/support/matchers.rb +++ b/spec/support/matchers.rb @@ -28,6 +28,6 @@ end failure_message do |actual| - "expected that `#{actual}' would have content '#{expected}', but it has '#{subject.read(actual)}'" # rubocop:disable Layout/LineLength + "expected that `#{actual}' would have content '#{expected}', but it has '#{subject.read(actual)}'" end end diff --git a/spec/unit/dry/files/file_system_spec.rb b/spec/unit/dry/files/file_system_spec.rb index 6950814..cc8614e 100644 --- a/spec/unit/dry/files/file_system_spec.rb +++ b/spec/unit/dry/files/file_system_spec.rb @@ -220,7 +220,7 @@ it "raises error when trying to write non-string" do path = root.join("write") - expect { subject.write(path, ["new", "words"]) }.to raise_error do |exception| + expect { subject.write(path, %w[new words]) }.to raise_error do |exception| expect(exception).to be_kind_of(ArgumentError) expect(exception.message).to eq("Must be string (use `join` or `to_s`)") end diff --git a/spec/unit/dry/files/memory_file_system/node_spec.rb b/spec/unit/dry/files/memory_file_system/node_spec.rb index af86157..1dd18eb 100644 --- a/spec/unit/dry/files/memory_file_system/node_spec.rb +++ b/spec/unit/dry/files/memory_file_system/node_spec.rb @@ -144,7 +144,7 @@ expect(subject.readlines).to eq(["foo"]) subject.write(%w[foo bar]) - expect(subject.readlines).to eq(["foo", "bar"]) + expect(subject.readlines).to eq(%w[foo bar]) end it "raises error when not file" do diff --git a/spec/unit/dry/files/memory_file_system_spec.rb b/spec/unit/dry/files/memory_file_system_spec.rb index 5f39476..028382e 100644 --- a/spec/unit/dry/files/memory_file_system_spec.rb +++ b/spec/unit/dry/files/memory_file_system_spec.rb @@ -191,7 +191,7 @@ it "raises error when trying to write non-string" do path = subject.join("write") - expect { subject.write(path, ["new", "words"]) }.to raise_error do |exception| + expect { subject.write(path, %w[new words]) }.to raise_error do |exception| expect(exception).to be_kind_of(ArgumentError) expect(exception.message).to eq("Must be string (use `join` or `to_s`)") end diff --git a/spec/unit/dry/files/path_spec.rb b/spec/unit/dry/files/path_spec.rb index c29e5c1..76c160f 100644 --- a/spec/unit/dry/files/path_spec.rb +++ b/spec/unit/dry/files/path_spec.rb @@ -5,7 +5,7 @@ RSpec.describe Dry::Files::Path do describe ".call" do let(:unix_file_separator) { "/" } - let(:windows_file_separator) { '\\' } + let(:windows_file_separator) { "\\" } context "when string" do it "recombines given path with system file separator" do From d456998ae7761de9f1cbce10d07c6d0457b79eee Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Fri, 21 Jan 2022 11:37:30 -0400 Subject: [PATCH 07/13] Extract CanOnlyWriteStringError and update API docs --- lib/dry/files/error.rb | 16 ++++++++++++++++ lib/dry/files/file_system.rb | 3 ++- lib/dry/files/memory_file_system.rb | 4 +++- lib/dry/files/memory_file_system/node.rb | 6 +++--- spec/unit/dry/files/file_system_spec.rb | 4 ++-- .../dry/files/memory_file_system/node_spec.rb | 4 ++-- spec/unit/dry/files/memory_file_system_spec.rb | 4 ++-- 7 files changed, 30 insertions(+), 11 deletions(-) diff --git a/lib/dry/files/error.rb b/lib/dry/files/error.rb index 7ec617d..c7f4857 100644 --- a/lib/dry/files/error.rb +++ b/lib/dry/files/error.rb @@ -116,5 +116,21 @@ def initialize(path) super("not a memory file `#{path}'") end end + + # Internal file system adapters can only take + # strings as arguments to their `write` methods. + # (The public API can take string or array of strings) + # + # @since x.x.x + # @api private + class CanOnlyWriteStringError < Error + # Instantiate a new error + # + # @since x.x.x + # @api private + def initialize + super("Can only write a String (use `join` or `to_s`)") + end + end end end diff --git a/lib/dry/files/file_system.rb b/lib/dry/files/file_system.rb index 58a6e03..ab5a0cb 100644 --- a/lib/dry/files/file_system.rb +++ b/lib/dry/files/file_system.rb @@ -129,11 +129,12 @@ def touch(path, **kwargs) # @param content [String] the content to write # # @raise [Dry::Files::IOError] in case of I/O error + # @raise [CanOnlyWriteStringError] if content param isn't a String # # @since 0.1.0 # @api private def write(path, content) - raise ArgumentError, "Must be string (use `join` or `to_s`)" unless content.is_a?(String) + raise CanOnlyWriteStringError unless content.is_a?(String) mkdir_p(path) diff --git a/lib/dry/files/memory_file_system.rb b/lib/dry/files/memory_file_system.rb index fb44b40..665f36f 100644 --- a/lib/dry/files/memory_file_system.rb +++ b/lib/dry/files/memory_file_system.rb @@ -107,10 +107,12 @@ def touch(path) # @param path [Array] the target path # @param content [String] the content to write # + # @raise [CanOnlyWriteStringError] if content param isn't a String + # # @since 0.1.0 # @api private def write(path, content) - raise ArgumentError, "Must be string (use `join` or `to_s`)" unless content.is_a?(String) + raise CanOnlyWriteStringError unless content.is_a?(String) path = Path[path] node = @root diff --git a/lib/dry/files/memory_file_system/node.rb b/lib/dry/files/memory_file_system/node.rb index b159dc0..44a50b4 100644 --- a/lib/dry/files/memory_file_system/node.rb +++ b/lib/dry/files/memory_file_system/node.rb @@ -208,14 +208,14 @@ def readlines # Write file contents # IMPORTANT: This operation turns a node into a file # - # @param content [String, Array] the file content + # @param content [String] the file content # # @raise [Dry::Files::NotMemoryFileError] if node isn't a file # # @since 0.1.0 # @api private - def write(*content) - @content = StringIO.new(content.join($INPUT_RECORD_SEPARATOR)) + def write(content) + @content = StringIO.new(content) @mode = DEFAULT_FILE_MODE end diff --git a/spec/unit/dry/files/file_system_spec.rb b/spec/unit/dry/files/file_system_spec.rb index cc8614e..56dce25 100644 --- a/spec/unit/dry/files/file_system_spec.rb +++ b/spec/unit/dry/files/file_system_spec.rb @@ -221,8 +221,8 @@ it "raises error when trying to write non-string" do path = root.join("write") expect { subject.write(path, %w[new words]) }.to raise_error do |exception| - expect(exception).to be_kind_of(ArgumentError) - expect(exception.message).to eq("Must be string (use `join` or `to_s`)") + expect(exception).to be_kind_of(Dry::Files::CanOnlyWriteStringError) + expect(exception.message).to eq("Can only write a String (use `join` or `to_s`)") end end end diff --git a/spec/unit/dry/files/memory_file_system/node_spec.rb b/spec/unit/dry/files/memory_file_system/node_spec.rb index 1dd18eb..2c7309e 100644 --- a/spec/unit/dry/files/memory_file_system/node_spec.rb +++ b/spec/unit/dry/files/memory_file_system/node_spec.rb @@ -143,7 +143,7 @@ subject.write("foo") expect(subject.readlines).to eq(["foo"]) - subject.write(%w[foo bar]) + subject.write("foo#{newline}bar") expect(subject.readlines).to eq(%w[foo bar]) end @@ -160,7 +160,7 @@ subject.write("foo") expect(subject.read).to eq("foo") - subject.write(%w[foo bar]) + subject.write("foo#{newline}bar") expect(subject.read).to eq("foo#{newline}bar") end diff --git a/spec/unit/dry/files/memory_file_system_spec.rb b/spec/unit/dry/files/memory_file_system_spec.rb index 028382e..f99c6d0 100644 --- a/spec/unit/dry/files/memory_file_system_spec.rb +++ b/spec/unit/dry/files/memory_file_system_spec.rb @@ -192,8 +192,8 @@ it "raises error when trying to write non-string" do path = subject.join("write") expect { subject.write(path, %w[new words]) }.to raise_error do |exception| - expect(exception).to be_kind_of(ArgumentError) - expect(exception.message).to eq("Must be string (use `join` or `to_s`)") + expect(exception).to be_kind_of(Dry::Files::CanOnlyWriteStringError) + expect(exception.message).to eq("Can only write a String (use `join` or `to_s`)") end end end From 8d4dbc3da6d4736523311d786941550fb656de1e Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Fri, 21 Jan 2022 11:56:10 -0400 Subject: [PATCH 08/13] Remove unnecessary chomp --- lib/dry/files.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dry/files.rb b/lib/dry/files.rb index 9521e38..730523b 100644 --- a/lib/dry/files.rb +++ b/lib/dry/files.rb @@ -828,7 +828,7 @@ def _inject_line_after(path, target, contents, finder) # @api private def _offset_block_lines(contents, offset) contents.map do |line| - case line.lines(chomp: true) + case line.lines in [line] offset + line else From bcbac684d024e8277442e75b2bbd57cb3e90bb5f Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Mon, 27 Jun 2022 17:09:29 -0500 Subject: [PATCH 09/13] Use in-place `#map!` to save memory --- lib/dry/files.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dry/files.rb b/lib/dry/files.rb index 730523b..d45dea9 100644 --- a/lib/dry/files.rb +++ b/lib/dry/files.rb @@ -76,7 +76,7 @@ def touch(path) # @since 0.1.0 # @api public def write(path, lines) - joined_lines = Array(lines).flatten.map { |line| line.chomp.concat(newline) }.join + joined_lines = Array(lines).flatten.map! { |line| line.chomp.concat(newline) }.join joined_lines = "" if joined_lines == "\n" # Leave it empty adapter.write(path, joined_lines) end From 1c5053ccfd06fb928dcf43a43364c65f2340113a Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Mon, 27 Jun 2022 17:15:53 -0500 Subject: [PATCH 10/13] Use `newline` instead of hard-coding --- lib/dry/files.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dry/files.rb b/lib/dry/files.rb index d45dea9..61430b8 100644 --- a/lib/dry/files.rb +++ b/lib/dry/files.rb @@ -77,7 +77,7 @@ def touch(path) # @api public def write(path, lines) joined_lines = Array(lines).flatten.map! { |line| line.chomp.concat(newline) }.join - joined_lines = "" if joined_lines == "\n" # Leave it empty + joined_lines = "" if joined_lines == newline # Leave it empty adapter.write(path, joined_lines) end From c8fa3d9feec7a2493cb141a1f06fdabb1d9d9040 Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Mon, 27 Jun 2022 17:26:01 -0500 Subject: [PATCH 11/13] Remove CanOnlyWriteStringError, due to it being private API --- lib/dry/files/error.rb | 16 ---------------- lib/dry/files/file_system.rb | 3 --- lib/dry/files/memory_file_system.rb | 4 ---- spec/unit/dry/files/file_system_spec.rb | 8 -------- spec/unit/dry/files/memory_file_system_spec.rb | 8 -------- 5 files changed, 39 deletions(-) diff --git a/lib/dry/files/error.rb b/lib/dry/files/error.rb index c7f4857..7ec617d 100644 --- a/lib/dry/files/error.rb +++ b/lib/dry/files/error.rb @@ -116,21 +116,5 @@ def initialize(path) super("not a memory file `#{path}'") end end - - # Internal file system adapters can only take - # strings as arguments to their `write` methods. - # (The public API can take string or array of strings) - # - # @since x.x.x - # @api private - class CanOnlyWriteStringError < Error - # Instantiate a new error - # - # @since x.x.x - # @api private - def initialize - super("Can only write a String (use `join` or `to_s`)") - end - end end end diff --git a/lib/dry/files/file_system.rb b/lib/dry/files/file_system.rb index ab5a0cb..f1cf063 100644 --- a/lib/dry/files/file_system.rb +++ b/lib/dry/files/file_system.rb @@ -129,13 +129,10 @@ def touch(path, **kwargs) # @param content [String] the content to write # # @raise [Dry::Files::IOError] in case of I/O error - # @raise [CanOnlyWriteStringError] if content param isn't a String # # @since 0.1.0 # @api private def write(path, content) - raise CanOnlyWriteStringError unless content.is_a?(String) - mkdir_p(path) self.open(path, WRITE_MODE) do |f| diff --git a/lib/dry/files/memory_file_system.rb b/lib/dry/files/memory_file_system.rb index 665f36f..fb1d4eb 100644 --- a/lib/dry/files/memory_file_system.rb +++ b/lib/dry/files/memory_file_system.rb @@ -107,13 +107,9 @@ def touch(path) # @param path [Array] the target path # @param content [String] the content to write # - # @raise [CanOnlyWriteStringError] if content param isn't a String - # # @since 0.1.0 # @api private def write(path, content) - raise CanOnlyWriteStringError unless content.is_a?(String) - path = Path[path] node = @root diff --git a/spec/unit/dry/files/file_system_spec.rb b/spec/unit/dry/files/file_system_spec.rb index 56dce25..abc95fa 100644 --- a/spec/unit/dry/files/file_system_spec.rb +++ b/spec/unit/dry/files/file_system_spec.rb @@ -217,14 +217,6 @@ path.chmod(mode) end end - - it "raises error when trying to write non-string" do - path = root.join("write") - expect { subject.write(path, %w[new words]) }.to raise_error do |exception| - expect(exception).to be_kind_of(Dry::Files::CanOnlyWriteStringError) - expect(exception.message).to eq("Can only write a String (use `join` or `to_s`)") - end - end end describe "#join" do diff --git a/spec/unit/dry/files/memory_file_system_spec.rb b/spec/unit/dry/files/memory_file_system_spec.rb index f99c6d0..228646c 100644 --- a/spec/unit/dry/files/memory_file_system_spec.rb +++ b/spec/unit/dry/files/memory_file_system_spec.rb @@ -188,14 +188,6 @@ path.chmod(mode) end end - - it "raises error when trying to write non-string" do - path = subject.join("write") - expect { subject.write(path, %w[new words]) }.to raise_error do |exception| - expect(exception).to be_kind_of(Dry::Files::CanOnlyWriteStringError) - expect(exception.message).to eq("Can only write a String (use `join` or `to_s`)") - end - end end describe "#join" do From d4d36fc6aa9a0e4ecd0ae7e03ae5a743da212f3f Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Mon, 27 Jun 2022 17:55:10 -0500 Subject: [PATCH 12/13] Fix typo --- spec/support/os.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support/os.rb b/spec/support/os.rb index 6473d90..62c7a80 100644 --- a/spec/support/os.rb +++ b/spec/support/os.rb @@ -13,7 +13,7 @@ def with_operating_system(os, &blk) when /darwin/ then :macos when /win32|mingw|bccwin|cygwin/ then :windows else - raise "unkwnown OS: `#{host_os}'" + raise "unknown OS: `#{host_os}'" end blk.call if result == os From d873b27e5e8157f6ea003ce8f492ca30dace400e Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Mon, 27 Jun 2022 19:27:54 -0500 Subject: [PATCH 13/13] Capitalize name of file --- lib/dry/files.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dry/files.rb b/lib/dry/files.rb index 61430b8..9226c82 100644 --- a/lib/dry/files.rb +++ b/lib/dry/files.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require "english" +require "English" # Required to load $INPUT_RECORD_SEPARATOR # dry-rb is a collection of next-generation Ruby libraries #