Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Edits to keep code DRY #111

Merged
merged 1 commit into from
Apr 26, 2014
Merged

Edits to keep code DRY #111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions lib/homesick/actions/file_actions.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def mv(source, destination, config = {})
destination = Pathname.new(destination + source.basename)

if destination.exist?
say_status :conflict, "#{destination} exists", :red unless options[:quiet]
say_status :conflict, "#{destination} exists", :red

FileUtils.mv source, destination if (options[:force] || shell.file_collision(destination) { source }) && !options[:pretend]
else
Expand All @@ -18,28 +18,28 @@ def mv(source, destination, config = {})
end

def rm_rf(dir)
say_status "rm -rf #{dir}", '', :green unless options[:quiet]
say_status "rm -rf #{dir}", '', :green
FileUtils.rm_r dir, force: true
end

def rm_link(target)
target = Pathname.new(target)

if target.symlink?
say_status :unlink, "#{target.expand_path}", :green unless options[:quiet]
say_status :unlink, "#{target.expand_path}", :green
FileUtils.rm_rf target
else
say_status :conflict, "#{target} is not a symlink", :red unless options[:quiet]
say_status :conflict, "#{target} is not a symlink", :red
end
end

def rm(file)
say_status "rm #{file}", '', :green unless options[:quiet]
say_status "rm #{file}", '', :green
FileUtils.rm file, force: true
end

def rm_r(dir)
say_status "rm -r #{dir}", '', :green unless options[:quiet]
say_status "rm -r #{dir}", '', :green
FileUtils.rm_r dir
end

Expand All @@ -64,16 +64,16 @@ def ln_s(source, destination, config = {})
def handle_symlink_action(action, source, destination)
case action
when :identical
say_status :identical, destination.expand_path, :blue unless options[:quiet]
say_status :identical, destination.expand_path, :blue
when :symlink_conflict
say_status :conflict,
"#{destination} exists and points to #{destination.readlink}",
:red unless options[:quiet]
:red

FileUtils.rm destination
FileUtils.ln_s source, destination, force: true unless options[:pretend]
when :conflict
say_status :conflict, "#{destination} exists", :red unless options[:quiet]
say_status :conflict, "#{destination} exists", :red

if collision_accepted?
FileUtils.rm_r destination, force: true unless options[:pretend]
Expand All @@ -82,7 +82,7 @@ def handle_symlink_action(action, source, destination)
else
say_status :symlink,
"#{source.expand_path} to #{destination.expand_path}",
:green unless options[:quiet]
:green
FileUtils.ln_s source, destination unless options[:pretend]
end
end
Expand Down
52 changes: 26 additions & 26 deletions lib/homesick/actions/git_actions.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ def git_clone(repo, config = {})
FileUtils.mkdir_p destination.dirname

if destination.directory?
say_status :exist, destination.expand_path, :blue unless options[:quiet]
say_status :exist, destination.expand_path, :blue
else
say_status 'git clone',
"#{repo} to #{destination.expand_path}",
:green unless options[:quiet]
system "git clone -q --config push.default=upstream --recursive #{repo} #{destination}" unless options[:pretend]
:green
system "git clone -q --config push.default=upstream --recursive #{repo} #{destination}"
end
end

Expand All @@ -26,10 +26,10 @@ def git_init(path = '.')

inside path do
if path.join('.git').exist?
say_status 'git init', 'already initialized', :blue unless options[:quiet]
say_status 'git init', 'already initialized', :blue
else
say_status 'git init', '' unless options[:quiet]
system 'git init >/dev/null' unless options[:pretend]
say_status 'git init', ''
system 'git init >/dev/null'
end
end
end
Expand All @@ -39,55 +39,55 @@ def git_remote_add(name, url)
existing_remote = nil if existing_remote == ''

if existing_remote
say_status 'git remote', "#{name} already exists", :blue unless options[:quiet]
say_status 'git remote', "#{name} already exists", :blue
else
say_status 'git remote', "add #{name} #{url}" unless options[:quiet]
system "git remote add #{name} #{url}" unless options[:pretend]
say_status 'git remote', "add #{name} #{url}"
system "git remote add #{name} #{url}"
end
end

def git_submodule_init(config = {})
say_status 'git submodule', 'init', :green unless options[:quiet]
system 'git submodule --quiet init' unless options[:pretend]
say_status 'git submodule', 'init', :green
system 'git submodule --quiet init'
end

def git_submodule_update(config = {})
say_status 'git submodule', 'update', :green unless options[:quiet]
system 'git submodule --quiet update --init --recursive >/dev/null 2>&1' unless options[:pretend]
say_status 'git submodule', 'update', :green
system 'git submodule --quiet update --init --recursive >/dev/null 2>&1'
end

def git_pull(config = {})
say_status 'git pull', '', :green unless options[:quiet]
system 'git pull --quiet' unless options[:pretend]
say_status 'git pull', '', :green
system 'git pull --quiet'
end

def git_push(config = {})
say_status 'git push', '', :green unless options[:quiet]
system 'git push' unless options[:pretend]
say_status 'git push', '', :green
system 'git push'
end

def git_commit_all(config = {})
say_status 'git commit all', '', :green unless options[:quiet]
say_status 'git commit all', '', :green
if config[:message]
system "git commit -a -m '#{config[:message]}'" unless options[:pretend]
system "git commit -a -m '#{config[:message]}'"
else
system 'git commit -v -a' unless options[:pretend]
system 'git commit -v -a'
end
end

def git_add(file, config = {})
say_status 'git add file', '', :green unless options[:quiet]
system "git add '#{file}'" unless options[:pretend]
say_status 'git add file', '', :green
system "git add '#{file}'"
end

def git_status(config = {})
say_status 'git status', '', :green unless options[:quiet]
system 'git status' unless options[:pretend]
say_status 'git status', '', :green
system 'git status'
end

def git_diff(config = {})
say_status 'git diff', '', :green unless options[:quiet]
system 'git diff' unless options[:pretend]
say_status 'git diff', '', :green
system 'git diff'
end
end
end
Expand Down
24 changes: 12 additions & 12 deletions lib/homesick/cli.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,14 @@ def rc(name = DEFAULT_CASTLE_NAME)
if homesickrc.exist?
proceed = shell.yes?("#{name} has a .homesickrc. Proceed with evaling it? (This could be destructive)")
if proceed
shell.say_status 'eval', homesickrc
say_status 'eval', homesickrc
inside destination do
eval homesickrc.read, binding, homesickrc.expand_path.to_s
end
else
shell.say_status 'eval skip',
"not evaling #{homesickrc}, #{destination} may need manual configuration",
:blue
say_status 'eval skip',
"not evaling #{homesickrc}, #{destination} may need manual configuration",
:blue
end
end
end
Expand All @@ -78,7 +78,7 @@ def rc(name = DEFAULT_CASTLE_NAME)
def pull(name = DEFAULT_CASTLE_NAME)
if options[:all]
inside_each_castle do |castle|
shell.say castle.to_s.gsub(repos_dir.to_s + '/', '') + ':'
say castle.to_s.gsub(repos_dir.to_s + '/', '') + ':'
update_castle castle
end
else
Expand Down Expand Up @@ -156,9 +156,9 @@ def track(file, castle = DEFAULT_CASTLE_NAME)
target.delete
mv absolute_path, castle_path
else
shell.say_status(:track,
"#{target} already exists, and is more recent than #{file}. Run 'homesick SYMLINK CASTLE' to create symlinks.",
:blue) unless options[:quiet]
say_status(:track,
"#{target} already exists, and is more recent than #{file}. Run 'homesick SYMLINK CASTLE' to create symlinks.",
:blue)
end
else
mv absolute_path, castle_path
Expand Down Expand Up @@ -294,9 +294,9 @@ def exec(castle, *args)
full_command = args.join(' ')
say_status "exec '#{full_command}'",
"#{options[:pretend] ? 'Would execute' : 'Executing command'} '#{full_command}' in castle '#{castle}'",
:green unless options[:quiet]
:green
inside repos_dir.join(castle) do
system(full_command) unless options[:pretend]
system(full_command)
end
end

Expand All @@ -323,8 +323,8 @@ def exec_all(*args)
inside_each_castle do |castle|
say_status "exec '#{full_command}'",
"#{options[:pretend] ? 'Would execute' : 'Executing command'} '#{full_command}' in castle '#{castle}'",
:green unless options[:quiet]
system(full_command) unless options[:pretend]
:green
system(full_command)
end
end

Expand Down
16 changes: 16 additions & 0 deletions lib/homesick/utils.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,22 @@
module Homesick
# Various utility methods that are used by Homesick
module Utils
QUIETABLE = ['say_status']

PRETENDABLE = ['system']

QUIETABLE.each do |method_name|
define_method(method_name) do |*args|
super(*args) unless options[:quiet]
end
end

PRETENDABLE.each do |method_name|
define_method(method_name) do |*args|
super(*args) unless options[:pretend]
end
end
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking that say_status and the system methods could be refactored a tiny bit like this:

def boolean? object # it might be best to make this method private or protected
  object == true || object == false
end

def say_status(*args)
  quiet = args.count == 4 && boolean?(args.last) && args.pop
  super(*args) unless options[:quiet] || quiet
end

def system(*args)
  pretend = boolean?(args.last) && args.pop
  super(*args) unless options[:pretend] || pretend
end

Also, it would probably help to add some brief documentation comments for these methods either way. What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolasmccurdy I like that, it's neater than what I came up with. I'll add a commit with that.


protected

def home_dir
Expand Down
6 changes: 3 additions & 3 deletions spec/homesick_cli_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
end

expect_any_instance_of(Thor::Shell::Basic).to receive(:yes?).with(be_a(String)).and_return(true)
expect_any_instance_of(Thor::Shell::Basic).to receive(:say_status).with('eval', kind_of(Pathname))
expect(homesick).to receive(:say_status).with('eval', kind_of(Pathname))
homesick.clone local_repo

expect(castles.join('some_repo').join('testing')).to exist
Expand Down Expand Up @@ -129,7 +129,7 @@
end"
end

expect_any_instance_of(Thor::Shell::Basic).to receive(:say_status).with('eval', kind_of(Pathname))
expect(homesick).to receive(:say_status).with('eval', kind_of(Pathname))
homesick.rc castle

expect(castle.join('testing')).to exist
Expand All @@ -148,7 +148,7 @@
end"
end

expect_any_instance_of(Thor::Shell::Basic).to receive(:say_status).with('eval skip', /not evaling.+/, :blue)
expect(homesick).to receive(:say_status).with('eval skip', /not evaling.+/, :blue)
homesick.rc castle

expect(castle.join('testing')).not_to exist
Expand Down