Merge pull request #155 from derekmorr/filesystem-refactor

address rubocop issues in filesystem driver

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2017-03-13 19:45:39 -0400 using RuboCop version 0.42.0.
+# on 2017-03-15 21:03:05 -0400 using RuboCop version 0.42.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -11,15 +11,15 @@ Lint/Loop:
   Exclude:
     - 'lib/browse_everything/driver/google_drive.rb'
 
-# Offense count: 10
+# Offense count: 8
 Metrics/AbcSize:
   Max: 31
 
-# Offense count: 3
+# Offense count: 2
 Metrics/CyclomaticComplexity:
   Max: 7
 
-# Offense count: 13
+# Offense count: 11
 # Configuration parameters: CountComments.
 Metrics/MethodLength:
   Max: 26
@@ -29,10 +29,6 @@ Metrics/MethodLength:
 Metrics/ParameterLists:
   Max: 7
 
-# Offense count: 1
-Metrics/PerceivedComplexity:
-  Max: 9
-
 # Offense count: 1
 Rails/OutputSafety:
   Exclude:
@@ -72,12 +68,11 @@ Style/FileName:
   Exclude:
     - 'lib/browse-everything.rb'
 
-# Offense count: 4
+# Offense count: 3
 # Configuration parameters: MinBodyLength.
 Style/GuardClause:
   Exclude:
     - 'lib/browse_everything/driver/box.rb'
-    - 'lib/browse_everything/driver/file_system.rb'
     - 'lib/browse_everything/driver/google_drive.rb'
     - 'lib/browse_everything/driver/sky_drive.rb'
 
@@ -125,9 +120,3 @@ Style/PredicateName:
 Style/RegexpLiteral:
   Exclude:
     - 'lib/browse_everything/driver/box.rb'
-
-# Offense count: 1
-# Cop supports --auto-correct.
-Style/RescueModifier:
-  Exclude:
-    - 'lib/browse_everything/driver/file_system.rb'

lib/browse_everything/driver/file_system.rb

@@ -6,53 +6,73 @@ def icon
       end
 
       def validate_config
-        unless config[:home]
-          raise BrowseEverything::InitializationError, 'FileSystem driver requires a :home argument'
-        end
+        return if config[:home].present?
+        raise BrowseEverything::InitializationError, 'FileSystem driver requires a :home argument'
       end
 
       def contents(path = '')
         relative_path = path.sub(%r{^[\/.]+}, '')
         real_path = File.join(config[:home], relative_path)
-        result = []
-        if File.directory?(real_path)
-          if relative_path.present?
-            result << details(File.expand_path('..', real_path), '..')
-          end
-          result += Dir[File.join(real_path, '*')].collect { |f| details(f) }
-        elsif File.exist?(real_path)
-          result += [details(real_path)]
-        end
-        result.sort do |a, b|
-          if b.container?
-            a.container? ? a.name.downcase <=> b.name.downcase : 1
-          else
-            a.container? ? -1 : a.name.downcase <=> b.name.downcase
-          end
-        end
+        entries = if File.directory?(real_path)
+                    make_directory_entry(relative_path, real_path)
+                  else
+                    [details(real_path)]
+                  end
+
+        sort_entries(entries)
+      end
+
+      def link_for(path)
+        full_path = File.expand_path(path)
+        file_size = file_size(full_path)
+        ["file://#{full_path}", { file_name: File.basename(path), file_size: file_size }]
+      end
+
+      def authorized?
+        true
       end
 
-      def details(path, display = nil)
+      def details(path, display = File.basename(path))
         return nil unless File.exist?(path)
         info = File::Stat.new(path)
         BrowseEverything::FileEntry.new(
-          Pathname.new(File.expand_path(path)).relative_path_from(Pathname.new(config[:home])),
+          make_pathname(path),
           [key, path].join(':'),
-          display || File.basename(path),
+          display,
           info.size,
           info.mtime,
           info.directory?
         )
       end
 
-      def link_for(path)
-        full_path = File.expand_path(path)
-        file_size = File.size(full_path).to_i rescue 0
-        ["file://#{full_path}", { file_name: File.basename(path), file_size: file_size }]
+      private
+
+      def make_directory_entry(relative_path, real_path)
+        entries = []
+        if relative_path.present?
+          entries << details(File.expand_path('..', real_path), '..')
+        end
+        entries + Dir[File.join(real_path, '*')].collect { |f| details(f) }
       end
 
-      def authorized?
-        true
+      def sort_entries(entries)
+        entries.sort do |a, b|
+          if b.container?
+            a.container? ? a.name.downcase <=> b.name.downcase : 1
+          else
+            a.container? ? -1 : a.name.downcase <=> b.name.downcase
+          end
+        end
+      end
+
+      def make_pathname(path)
+        Pathname.new(File.expand_path(path)).relative_path_from(Pathname.new(config[:home]))
+      end
+
+      def file_size(path)
+        File.size(path).to_i
+      rescue
+        0
       end
     end
   end