Newlines revamp (reading and writing)

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
@@ -25,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
@@ -64,14 +69,16 @@ def touch(path)
     # All the intermediate directories are created.
     #
     # @param path [String,Pathname] the path to file
-    # @param content [String, Array<String>] the content to write
+    # @param lines [String, Array<String>] 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
+      joined_lines = "" if joined_lines == "\n" # Leave it empty
+      adapter.write(path, joined_lines)
     end
 
     # Returns a new string formed by joining the strings using Operating
@@ -289,7 +296,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
@@ -309,8 +316,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
@@ -330,7 +336,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
@@ -350,7 +356,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
@@ -735,11 +741,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
@@ -779,17 +780,9 @@ def remove_block(path, target)
     # @api private
     attr_reader :adapter
 
-    # @since 0.1.0
+    # @since x.x.x
     # @api private
-    def newline(line = nil)
-      "#{line}#{NEW_LINE}"
-    end
-
-    # @since 0.1.0
-    # @api private
-    def newline?(content)
-      content.end_with?(NEW_LINE)
-    end
+    attr_reader :newline
 
     # @since 0.1.0
     # @api private
@@ -817,7 +810,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
 
@@ -827,21 +820,21 @@ 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
 
     # @since 0.1.0
     # @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
+        in [line]
+          offset + line
         else
-          offset + line + NEW_LINE
+          _offset_block_lines(line.lines, offset)
         end
-      end.join
+      end
     end
 
     # @since 0.1.0

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

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
 
@@ -117,13 +126,16 @@ def touch(path, **kwargs)
       # All the intermediate directories are created.
       #
       # @param path [String,Pathname] the path to file
-      # @param content [String, Array<String>] the content to write
+      # @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)
+      def write(path, content)
+        raise CanOnlyWriteStringError unless content.is_a?(String)
+
         mkdir_p(path)
 
         self.open(path, WRITE_MODE) do |f|

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,20 +104,24 @@ def touch(path)
       # of an existing file for the given path and content
       # All the intermediate directories are created.
       #
-      # @param path [String, Array<String>] the target path
-      # @param content [String, Array<String>] the content to write
+      # @param path [Array<String>] 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)
+      def write(path, content)
+        raise CanOnlyWriteStringError unless content.is_a?(String)
+
         path = Path[path]
         node = @root
 
         for_each_segment(path) do |segment|
           node = node.set(segment)
         end
 
-        node.write(*content)
+        node.write(content)
         node
       end
 

lib/dry/files/memory_file_system/node.rb

@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 
-require "English"
 require "stringio"
 
 module Dry
@@ -203,20 +202,20 @@ def readlines
           raise NotMemoryFileError, segment unless file?
 
           @content.rewind
-          @content.readlines
+          @content.readlines(chomp: true)
         end
 
         # Write file contents
         # IMPORTANT: This operation turns a node into a file
         #
-        # @param content [String, Array<String>] 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($RS))
+        def write(content)
+          @content = StringIO.new(content)
           @mode = DEFAULT_FILE_MODE
         end
 

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")
@@ -52,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
@@ -85,15 +90,23 @@
       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 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, ":)")
 
       expect(path).to exist
-      expect(path).to have_content(":)")
+      expect(path).to have_content(":)#{newline}")
     end
 
     it "overwrites file when it already exists" do
@@ -102,7 +115,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
@@ -138,7 +151,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
@@ -149,7 +162,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
@@ -161,7 +174,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
@@ -455,7 +468,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

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