Unclutter #command and #command_lines

smortex · smortex · commit 66f4f7702f53 · 2018-06-04T10:31:59.000+02:00
These internal function start to have too many arguments, and in order
to avoid chomping when not applicable, we will need to add one more
boolean.  Pass a hash of option instead of distinct arguments to make it
easier to scan the source code.

No functionnal change.
diff --git a/lib/git/lib.rb b/lib/git/lib.rb
@@ -37,7 +37,7 @@ def init(opts={})
       arr_opts = []
       arr_opts << '--bare' if opts[:bare]
 
-      command('init', arr_opts, false)
+      command('init', arr_opts, chdir: false)
     end
 
     # tries to clone the given repo
@@ -132,7 +132,7 @@ def log_commits(opts={})
 
       arr_opts += log_path_options(opts)
 
-      command_lines('log', arr_opts, true).map { |l| l.split.first }
+      command_lines('log', arr_opts, chdir: true).map { |l| l.split.first }
     end
 
     def full_log_commits(opts={})
@@ -143,7 +143,7 @@ def full_log_commits(opts={})
 
       arr_opts += log_path_options(opts)
 
-      full_log = command_lines('log', arr_opts, true)
+      full_log = command_lines('log', arr_opts, chdir: true)
 
       process_commit_log_data(full_log)
     end
@@ -411,7 +411,7 @@ def ls_files(location=nil)
     def ls_remote(location=nil)
       location ||= '.'
       Hash.new{ |h,k| h[k] = {} }.tap do |hsh|
-        command_lines('ls-remote', [location], false).each do |line|
+        command_lines('ls-remote', [location], chdir: false).each do |line|
           (sha, info) = line.split("\t")
           (ref, type, name) = info.split('/', 3)
           type ||= 'head'
@@ -450,7 +450,7 @@ def config_get(name)
     end
 
     def global_config_get(name)
-      command('config', ['--global', '--get', name], false)
+      command('config', ['--global', '--get', name], chdir: false)
     end
 
     def config_list
@@ -466,7 +466,7 @@ def config_list
     end
 
     def global_config_list
-      parse_config_list command_lines('config', ['--global', '--list'], false)
+      parse_config_list command_lines('config', ['--global', '--list'], chdir: false)
     end
 
     def parse_config_list(lines)
@@ -479,7 +479,7 @@ def parse_config_list(lines)
     end
 
     def parse_config(file)
-      parse_config_list command_lines('config', ['--list', '--file', file], false)
+      parse_config_list command_lines('config', ['--list', '--file', file], chdir: false)
     end
 
     # Shows objects
@@ -502,7 +502,7 @@ def config_set(name, value)
     end
 
     def global_config_set(name, value)
-      command('config', ['--global', name, value], false)
+      command('config', ['--global', name, value], chdir: false)
     end
 
     # updates the repository index using the working directory content
@@ -671,10 +671,10 @@ def unmerged
     def conflicts # :yields: file, your, their
       self.unmerged.each do |f|
         your = Tempfile.new("YOUR-#{File.basename(f)}").path
-        command('show', ":2:#{f}", true, "> #{escape your}")
+        command('show', ":2:#{f}", chdir: true, redirect: "> #{escape your}")
 
         their = Tempfile.new("THEIR-#{File.basename(f)}").path
-        command('show', ":3:#{f}", true, "> #{escape their}")
+        command('show', ":3:#{f}", chdir: true, redirect: "> #{escape their}")
         yield(f, your, their)
       end
     end
@@ -799,7 +799,7 @@ def commit_tree(tree, opts = {})
       arr_opts << tree
       arr_opts << '-p' << opts[:parent] if opts[:parent]
       arr_opts += [opts[:parents]].map { |p| ['-p', p] }.flatten if opts[:parents]
-      command('commit-tree', arr_opts, true, "< #{escape t.path}")
+      command('commit-tree', arr_opts, chdir: true, redirect: "< #{escape t.path}")
     end
 
     def update_ref(branch, commit)
@@ -845,13 +845,13 @@ def archive(sha, file = nil, opts = {})
       arr_opts << "--remote=#{opts[:remote]}" if opts[:remote]
       arr_opts << sha
       arr_opts << '--' << opts[:path] if opts[:path]
-      command('archive', arr_opts, true, (opts[:add_gzip] ? '| gzip' : '') + " > #{escape file}")
+      command('archive', arr_opts, chdir: true, redirect: (opts[:add_gzip] ? '| gzip' : '') + " > #{escape file}")
       return file
     end
 
     # returns the current version of git, as an Array of Fixnums.
     def current_command_version
-      output = command('version', [], false)
+      output = command('version', [], chdir: false)
       version = output[/\d+\.\d+(\.\d+)+/]
       version.split('.').collect {|i| i.to_i}
     end
@@ -872,8 +872,8 @@ def meets_required_version?
     # @return [<String>] the names of the EVN variables involved in the git commands
     ENV_VARIABLE_NAMES = ['GIT_DIR', 'GIT_WORK_TREE', 'GIT_INDEX_FILE', 'GIT_SSH']
 
-    def command_lines(cmd, opts = [], chdir = true, redirect = '')
-      cmd_op = command(cmd, opts, chdir)
+    def command_lines(cmd, *opts)
+      cmd_op = command(cmd, *opts)
       if cmd_op.encoding.name != "UTF-8"
         op = cmd_op.encode("UTF-8", "binary", {
           :invalid => :replace,
@@ -922,7 +922,18 @@ def with_custom_env_variables(&block)
       restore_git_system_env_variables()
     end
 
-    def command(cmd, opts = [], chdir = true, redirect = '', &block)
+    def command(cmd, *opts, &block)
+      command_opts = { chdir: true, redirect: '' }
+      if opts.last.is_a?(Hash)
+        command_opts.merge!(opts.pop)
+      end
+      command_opts.keys.each do |k|
+        raise ArgumentError.new("Unsupported option: #{k}") unless %i[chdir redirect].include?(k)
+      end
+
+      default_command_opts = { chdir: true, redirect: '' }
+      command_opts = default_command_opts.merge(command_opts)
+
       global_opts = []
       global_opts << "--git-dir=#{@git_dir}" if !@git_dir.nil?
       global_opts << "--work-tree=#{@git_work_dir}" if !@git_work_dir.nil?
@@ -931,7 +942,7 @@ def command(cmd, opts = [], chdir = true, redirect = '', &block)
 
       global_opts = global_opts.flatten.map {|s| escape(s) }.join(' ')
 
-      git_cmd = "#{Git::Base.config.binary_path} #{global_opts} #{cmd} #{opts} #{redirect} 2>&1"
+      git_cmd = "#{Git::Base.config.binary_path} #{global_opts} #{cmd} #{opts} #{command_opts[:redirect]} 2>&1"
 
       output = nil
 
