From fdb57cd846efbb75166bf814e17f788a74d3a7a6 Mon Sep 17 00:00:00 2001 From: Jeremy Cook Date: Sat, 19 Jan 2019 07:41:56 -0500 Subject: [PATCH] Apply fixes suggested by Rubocop. --- lib/homesick.rb | 7 ++-- lib/homesick/actions/file_actions.rb | 47 ++++++++++-------------- lib/homesick/actions/git_actions.rb | 17 +++++---- lib/homesick/cli.rb | 10 +++-- lib/homesick/utils.rb | 9 +++-- lib/homesick/version.rb | 1 - spec/homesick_cli_spec.rb | 55 ++++++++++++++-------------- 7 files changed, 70 insertions(+), 76 deletions(-) diff --git a/lib/homesick.rb b/lib/homesick.rb index 8d84601..2385172 100644 --- a/lib/homesick.rb +++ b/lib/homesick.rb @@ -1,4 +1,3 @@ -# -*- encoding : utf-8 -*- require 'homesick/actions/file_actions' require 'homesick/actions/git_actions' require 'homesick/version' @@ -7,8 +6,8 @@ require 'homesick/cli' # Homesick's top-level module module Homesick - GITHUB_NAME_REPO_PATTERN = %r{\A([A-Za-z0-9_-]+/[A-Za-z0-9_-]+)\Z} - SUBDIR_FILENAME = '.homesick_subdir' + GITHUB_NAME_REPO_PATTERN = %r{\A([A-Za-z0-9_-]+/[A-Za-z0-9_-]+)\Z}.freeze + SUBDIR_FILENAME = '.homesick_subdir'.freeze - DEFAULT_CASTLE_NAME = 'dotfiles' + DEFAULT_CASTLE_NAME = 'dotfiles'.freeze end diff --git a/lib/homesick/actions/file_actions.rb b/lib/homesick/actions/file_actions.rb index 0a66387..d3bc6b3 100644 --- a/lib/homesick/actions/file_actions.rb +++ b/lib/homesick/actions/file_actions.rb @@ -5,9 +5,7 @@ module Homesick def mv(source, destination) source = Pathname.new(source) destination = Pathname.new(destination + source.basename) - if destination.exist? && (options[:force] || shell.file_collision(destination) { source }) - say_status :conflict, "#{destination} exists", :red - end + say_status :conflict, "#{destination} exists", :red if destination.exist? && (options[:force] || shell.file_collision(destination) { source }) FileUtils.mv source, destination unless options[:pretend] end @@ -42,41 +40,36 @@ module Homesick destination = Pathname.new(destination) FileUtils.mkdir_p destination.dirname - action = if destination.symlink? && destination.readlink == source - :identical - elsif destination.symlink? - :symlink_conflict - elsif destination.exist? - :conflict - else - :success - end + action = :success + action = :identical if destination.symlink? && destination.readlink == source + action = :symlink_conflict if destination.symlink? + action = :conflict if destination.exist? handle_symlink_action action, source, destination end def handle_symlink_action(action, source, destination) - case action - when :identical + if action == :identical say_status :identical, destination.expand_path, :blue - when :symlink_conflict, :conflict - if action == :conflict - say_status :conflict, "#{destination} exists", :red - else - say_status :conflict, - "#{destination} exists and points to #{destination.readlink}", - :red - end + return + end + message = generate_symlink_message action, source, destination + if %i[symlink_conflict conflict].include?(action) + say_status :conflict, message, :red if collision_accepted?(destination, source) FileUtils.rm_r destination, force: true unless options[:pretend] - FileUtils.ln_s source, destination, force: true unless options[:pretend] end else - say_status :symlink, - "#{source.expand_path} to #{destination.expand_path}", - :green - FileUtils.ln_s source, destination unless options[:pretend] + say_status :symlink, message, :green end + FileUtils.ln_s source, destination, force: true unless options[:pretend] + end + + def generate_symlink_message(action, source, destination) + message = "#{source.expand_path} to #{destination.expand_path}" + message = "#{destination} exists and points to #{destination.readlink}" if action == :symlink_conflict + message = "#{destination} exists" if action == :conflict + message end end end diff --git a/lib/homesick/actions/git_actions.rb b/lib/homesick/actions/git_actions.rb index a5b8cc4..c4d93a1 100644 --- a/lib/homesick/actions/git_actions.rb +++ b/lib/homesick/actions/git_actions.rb @@ -1,4 +1,3 @@ -# -*- encoding : utf-8 -*- module Homesick module Actions # Git-related helper methods for Homesick @@ -8,18 +7,20 @@ module Homesick major: 1, minor: 8, patch: 0 - } + }.freeze STRING = MIN_VERSION.values.join('.') def git_version_correct? info = `git --version`.scan(/(\d+)\.(\d+)\.(\d+)/).flatten.map(&:to_i) return false unless info.count == 3 - current_version = Hash[[:major, :minor, :patch].zip(info)] - return true if current_version.eql?(MIN_VERSION) - return true if current_version[:major] > MIN_VERSION[:major] - return true if current_version[:major] == MIN_VERSION[:major] && current_version[:minor] > MIN_VERSION[:minor] - return true if current_version[:major] == MIN_VERSION[:major] && current_version[:minor] == MIN_VERSION[:minor] && current_version[:patch] >= MIN_VERSION[:patch] - false + + current_version = Hash[%i[major minor patch].zip(info)] + major_equals = current_version.eql?(MIN_VERSION) + major_greater = current_version[:major] > MIN_VERSION[:major] + minor_greater = current_version[:major] == MIN_VERSION[:major] && current_version[:minor] > MIN_VERSION[:minor] + patch_greater = current_version[:major] == MIN_VERSION[:major] && current_version[:minor] == MIN_VERSION[:minor] && current_version[:patch] >= MIN_VERSION[:patch] + + major_equals || major_greater || minor_greater || patch_greater end # TODO: move this to be more like thor's template, empty_directory, etc diff --git a/lib/homesick/cli.rb b/lib/homesick/cli.rb index 093b6dc..b5ca84f 100644 --- a/lib/homesick/cli.rb +++ b/lib/homesick/cli.rb @@ -1,4 +1,3 @@ -# -*- encoding : utf-8 -*- require 'fileutils' require 'thor' @@ -33,19 +32,20 @@ module Homesick source = Pathname.new(source) return 'Unable to create diff: destination or content is a directory' if destination.directory? || source.directory? return super(destination, File.binread(source)) unless destination.symlink? + say "- #{destination.readlink}", :red, true say "+ #{source.expand_path}", :green, true end end desc 'clone URI CASTLE_NAME', 'Clone +uri+ as a castle with name CASTLE_NAME for homesick' - def clone(uri, destination=nil) + def clone(uri, destination = nil) destination = Pathname.new(destination) unless destination.nil? inside repos_dir do if File.exist?(uri) uri = Pathname.new(uri).expand_path - fail "Castle already cloned to #{uri}" if uri.to_s.start_with?(repos_dir.to_s) + raise "Castle already cloned to #{uri}" if uri.to_s.start_with?(repos_dir.to_s) destination = uri.basename if destination.nil? @@ -58,7 +58,7 @@ module Homesick destination = Pathname.new(Regexp.last_match[1].gsub(/\.git$/, '')).basename if destination.nil? git_clone uri, destination: destination else - fail "Unknown URI format: #{uri}" + raise "Unknown URI format: #{uri}" end setup_castle(destination) @@ -75,8 +75,10 @@ module Homesick destination = Pathname.new(name) homesickrc = destination.join('.homesickrc').expand_path return unless homesickrc.exist? + proceed = options[:force] || shell.yes?("#{name} has a .homesickrc. Proceed with evaling it? (This could be destructive)") return say_status 'eval skip', "not evaling #{homesickrc}, #{destination} may need manual configuration", :blue unless proceed + say_status 'eval', homesickrc inside destination do eval homesickrc.read, binding, homesickrc.expand_path.to_s diff --git a/lib/homesick/utils.rb b/lib/homesick/utils.rb index e18cd17..7979328 100644 --- a/lib/homesick/utils.rb +++ b/lib/homesick/utils.rb @@ -1,12 +1,11 @@ -# -*- encoding : utf-8 -*- require 'pathname' module Homesick # Various utility methods that are used by Homesick module Utils - QUIETABLE = [:say_status] + QUIETABLE = [:say_status].freeze - PRETENDABLE = [:system] + PRETENDABLE = [:system].freeze QUIETABLE.each do |method_name| define_method(method_name) do |*args| @@ -36,6 +35,7 @@ module Homesick def check_castle_existance(name, action) return if castle_dir(name).exist? + say_status :error, "Could not #{action} #{name}, expected #{castle_dir(name)} to exist and contain dotfiles", :red @@ -149,7 +149,8 @@ module Homesick end def collision_accepted?(destination, source) - fail "Arguments must be instances of Pathname, #{destination.class.name} and #{source.class.name} given" unless destination.instance_of?(Pathname) && source.instance_of?(Pathname) + raise "Arguments must be instances of Pathname, #{destination.class.name} and #{source.class.name} given" unless destination.instance_of?(Pathname) && source.instance_of?(Pathname) + options[:force] || shell.file_collision(destination) { source } end diff --git a/lib/homesick/version.rb b/lib/homesick/version.rb index a540369..d5e1fd3 100644 --- a/lib/homesick/version.rb +++ b/lib/homesick/version.rb @@ -1,4 +1,3 @@ -# -*- encoding : utf-8 -*- module Homesick # A representation of Homesick's version number in constants, including a # String of the entire version number diff --git a/spec/homesick_cli_spec.rb b/spec/homesick_cli_spec.rb index 5a2639c..dc06add 100644 --- a/spec/homesick_cli_spec.rb +++ b/spec/homesick_cli_spec.rb @@ -1,4 +1,3 @@ -# -*- encoding : utf-8 -*- require 'spec_helper' require 'capture-output' require 'pathname' @@ -158,8 +157,8 @@ describe Homesick::CLI do it 'accepts a destination', :focus do expect(homesick).to receive(:git_clone) - .with('https://github.com/wfarr/dotfiles.git', - destination: Pathname.new('other-name')) + .with('https://github.com/wfarr/dotfiles.git', + destination: Pathname.new('other-name')) homesick.clone 'wfarr/dotfiles', 'other-name' end @@ -334,7 +333,7 @@ describe Homesick::CLI do context 'when call and some files conflict' do it 'shows differences for conflicting text files' do - contents = {:castle => 'castle has new content', :home => 'home already has content'} + contents = { castle: 'castle has new content', home: 'home already has content' } dotfile = castle.file('text') File.open(dotfile.to_s, 'w') do |f| @@ -348,7 +347,7 @@ describe Homesick::CLI do end it 'shows message or differences for conflicting binary files' do # content which contains NULL character, without any parentheses, braces, ... - contents = {:castle => (0..255).step(30).map{|e| e.chr}.join(), :home => (0..255).step(30).reverse_each.map{|e| e.chr}.join()} + contents = { castle: (0..255).step(30).map(&:chr).join, home: (0..255).step(30).reverse_each.map(&:chr).join } dotfile = castle.file('binary') File.open(dotfile.to_s, 'w') do |f| @@ -532,9 +531,9 @@ describe Homesick::CLI do it 'prints an error message when trying to pull a non-existant castle' do expect(homesick).to receive('say_status').once - .with(:error, - /Could not pull castle_repo, expected .* to exist and contain dotfiles/, - :red) + .with(:error, + /Could not pull castle_repo, expected .* to exist and contain dotfiles/, + :red) expect { homesick.pull 'castle_repo' }.to raise_error(SystemExit) end @@ -544,9 +543,9 @@ describe Homesick::CLI do given_castle('glencairn') allow(homesick).to receive(:system).exactly(2).times.with('git pull --quiet') allow(homesick).to receive(:system).exactly(2).times - .with('git submodule --quiet init') + .with('git submodule --quiet init') allow(homesick).to receive(:system).exactly(2).times - .with('git submodule --quiet update --init --recursive >/dev/null 2>&1') + .with('git submodule --quiet update --init --recursive >/dev/null 2>&1') Capture.stdout do Capture.stderr { homesick.invoke 'pull', [], all: true } end @@ -563,7 +562,7 @@ describe Homesick::CLI do it 'prints an error message when trying to push a non-existant castle' do expect(homesick).to receive('say_status').once - .with(:error, /Could not push castle_repo, expected .* to exist and contain dotfiles/, :red) + .with(:error, /Could not push castle_repo, expected .* to exist and contain dotfiles/, :red) expect { homesick.push 'castle_repo' }.to raise_error(SystemExit) end end @@ -721,7 +720,7 @@ describe Homesick::CLI do it 'returns an error message when the given castle does not exist' do expect(homesick).to receive('say_status').once - .with(:error, /Could not cd castle_repo, expected .* to exist and contain dotfiles/, :red) + .with(:error, /Could not cd castle_repo, expected .* to exist and contain dotfiles/, :red) expect { homesick.cd 'castle_repo' }.to raise_error(SystemExit) end end @@ -744,7 +743,7 @@ describe Homesick::CLI do # Set the default editor to make sure it fails. allow(ENV).to receive(:[]).with('EDITOR').and_return(nil) expect(homesick).to receive('say_status').once - .with(:error, 'The $EDITOR environment variable must be set to use this command', :red) + .with(:error, 'The $EDITOR environment variable must be set to use this command', :red) expect { homesick.open 'castle_repo' }.to raise_error(SystemExit) end @@ -754,7 +753,7 @@ describe Homesick::CLI do # Set a default just in case none is set allow(ENV).to receive(:[]).with('EDITOR').and_return('vim') allow(homesick).to receive('say_status').once - .with(:error, /Could not open castle_repo, expected .* to exist and contain dotfiles/, :red) + .with(:error, /Could not open castle_repo, expected .* to exist and contain dotfiles/, :red) expect { homesick.open 'castle_repo' }.to raise_error(SystemExit) end end @@ -773,7 +772,7 @@ describe Homesick::CLI do it 'executes a single command with no arguments inside a given castle' do allow(homesick).to receive('inside').once.with(kind_of(Pathname)).and_yield allow(homesick).to receive('say_status').once - .with(be_a(String), be_a(String), :green) + .with(be_a(String), be_a(String), :green) allow(homesick).to receive('system').once.with('ls') Capture.stdout { homesick.exec 'castle_repo', 'ls' } end @@ -781,14 +780,14 @@ describe Homesick::CLI do it 'executes a single command with arguments inside a given castle' do allow(homesick).to receive('inside').once.with(kind_of(Pathname)).and_yield allow(homesick).to receive('say_status').once - .with(be_a(String), be_a(String), :green) + .with(be_a(String), be_a(String), :green) allow(homesick).to receive('system').once.with('ls -la') Capture.stdout { homesick.exec 'castle_repo', 'ls', '-la' } end it 'raises an error when the method is called without a command' do allow(homesick).to receive('say_status').once - .with(:error, be_a(String), :red) + .with(:error, be_a(String), :red) allow(homesick).to receive('exit').once.with(1) Capture.stdout { homesick.exec 'castle_repo' } end @@ -796,9 +795,9 @@ describe Homesick::CLI do context 'pretend' do it 'does not execute a command when the pretend option is passed' do allow(homesick).to receive('say_status').once - .with(be_a(String), match(/.*Would execute.*/), :green) + .with(be_a(String), match(/.*Would execute.*/), :green) expect(homesick).to receive('system').never - Capture.stdout { homesick.invoke 'exec', %w(castle_repo ls -la), pretend: true } + Capture.stdout { homesick.invoke 'exec', %w[castle_repo ls -la], pretend: true } end end @@ -806,8 +805,8 @@ describe Homesick::CLI do it 'does not print status information when quiet is passed' do expect(homesick).to receive('say_status').never allow(homesick).to receive('system').once - .with('ls -la') - Capture.stdout { homesick.invoke 'exec', %w(castle_repo ls -la), quiet: true } + .with('ls -la') + Capture.stdout { homesick.invoke 'exec', %w[castle_repo ls -la], quiet: true } end end end @@ -821,7 +820,7 @@ describe Homesick::CLI do it 'executes a command without arguments inside the root of each cloned castle' do allow(homesick).to receive('inside_each_castle').exactly(:twice).and_yield('castle_repo').and_yield('another_castle_repo') allow(homesick).to receive('say_status').at_least(:once) - .with(be_a(String), be_a(String), :green) + .with(be_a(String), be_a(String), :green) allow(homesick).to receive('system').at_least(:once).with('ls') Capture.stdout { homesick.exec_all 'ls' } end @@ -829,14 +828,14 @@ describe Homesick::CLI do it 'executes a command with arguments inside the root of each cloned castle' do allow(homesick).to receive('inside_each_castle').exactly(:twice).and_yield('castle_repo').and_yield('another_castle_repo') allow(homesick).to receive('say_status').at_least(:once) - .with(be_a(String), be_a(String), :green) + .with(be_a(String), be_a(String), :green) allow(homesick).to receive('system').at_least(:once).with('ls -la') Capture.stdout { homesick.exec_all 'ls', '-la' } end it 'raises an error when the method is called without a command' do allow(homesick).to receive('say_status').once - .with(:error, be_a(String), :red) + .with(:error, be_a(String), :red) allow(homesick).to receive('exit').once.with(1) Capture.stdout { homesick.exec_all } end @@ -844,9 +843,9 @@ describe Homesick::CLI do context 'pretend' do it 'does not execute a command when the pretend option is passed' do allow(homesick).to receive('say_status').at_least(:once) - .with(be_a(String), match(/.*Would execute.*/), :green) + .with(be_a(String), match(/.*Would execute.*/), :green) expect(homesick).to receive('system').never - Capture.stdout { homesick.invoke 'exec_all', %w(ls -la), pretend: true } + Capture.stdout { homesick.invoke 'exec_all', %w[ls -la], pretend: true } end end @@ -854,8 +853,8 @@ describe Homesick::CLI do it 'does not print status information when quiet is passed' do expect(homesick).to receive('say_status').never allow(homesick).to receive('system').at_least(:once) - .with('ls -la') - Capture.stdout { homesick.invoke 'exec_all', %w(ls -la), quiet: true } + .with('ls -la') + Capture.stdout { homesick.invoke 'exec_all', %w[ls -la], quiet: true } end end end