Skip to content

Commit

Permalink
Style fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
lifo committed Jul 14, 2010
1 parent b7944e1 commit 4a06489
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 24 deletions.
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1471,7 +1471,7 @@ def []=(attr_name, value)
# user.send(:attributes=, { :username => 'Phusion', :is_admin => true }, false)
# user.is_admin? # => true
def attributes=(new_attributes, guard_protected_attributes = true)
return unless new_attributes.is_a? Hash
return unless new_attributes.is_a?(Hash)
attributes = new_attributes.stringify_keys

multi_parameter_attributes = []
Expand Down
40 changes: 20 additions & 20 deletions activerecord/lib/active_record/relation/query_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,84 +11,84 @@ module QueryMethods

def includes(*args)
args.reject! { |a| a.blank? }

This comment has been minimized.

Copy link
@rymai

rymai Jul 23, 2010

Contributor

And why not here ? :P

clone.tap { |r| r.includes_values += args if args.present? }
clone.tap {|r| r.includes_values += args if args.present? }

This comment has been minimized.

Copy link
@jeremy

jeremy Jul 14, 2010

Member

Generally, we do whitespace { |r| so this isn't a fix

This comment has been minimized.

Copy link
@lifo

lifo Jul 15, 2010

Author Member

This was inconsistent with the rest of the file.

end

def eager_load(*args)
clone.tap { |r| r.eager_load_values += args if args.present? }
clone.tap {|r| r.eager_load_values += args if args.present? }
end

def preload(*args)
clone.tap { |r| r.preload_values += args if args.present? }
clone.tap {|r| r.preload_values += args if args.present? }
end

def select(*args)
if block_given?
to_a.select { |*block_args| yield(*block_args) }
to_a.select {|*block_args| yield(*block_args) }
else
clone.tap { |r| r.select_values += args if args.present? }
clone.tap {|r| r.select_values += args if args.present? }
end
end

def group(*args)
clone.tap { |r| r.group_values += args if args.present? }
clone.tap {|r| r.group_values += args if args.present? }
end

def order(*args)
clone.tap { |r| r.order_values += args if args.present? }
clone.tap {|r| r.order_values += args if args.present? }
end

def reorder(*args)
clone.tap { |r| r.order_values = args if args.present? }
clone.tap {|r| r.order_values = args if args.present? }
end

def joins(*args)
args.flatten!
clone.tap { |r| r.joins_values += args if args.present? }
clone.tap {|r| r.joins_values += args if args.present? }
end

def where(*args)
value = build_where(*args)
clone.tap { |r| r.where_values += Array.wrap(value) if value.present? }
clone.tap {|r| r.where_values += Array.wrap(value) if value.present? }
end

def having(*args)
value = build_where(*args)
clone.tap { |r| r.having_values += Array.wrap(value) if value.present? }
clone.tap {|r| r.having_values += Array.wrap(value) if value.present? }
end

def limit(value = true)
clone.tap { |r| r.limit_value = value }
clone.tap {|r| r.limit_value = value }
end

def offset(value = true)
clone.tap { |r| r.offset_value = value }
clone.tap {|r| r.offset_value = value }
end

def lock(locks = true)
case locks
when String, TrueClass, NilClass
clone.tap { |r| r.lock_value = locks || true }
clone.tap {|r| r.lock_value = locks || true }
else
clone.tap { |r| r.lock_value = false }
clone.tap {|r| r.lock_value = false }
end
end

def readonly(value = true)
clone.tap { |r| r.readonly_value = value }
clone.tap {|r| r.readonly_value = value }
end

def create_with(value = true)
clone.tap { |r| r.create_with_value = value }
clone.tap {|r| r.create_with_value = value }
end

def from(value = true)
clone.tap { |r| r.from_value = value }
clone.tap {|r| r.from_value = value }
end

def extending(*modules, &block)
modules << Module.new(&block) if block_given?
clone.tap { |r| r.send(:apply_modules, *modules) }
clone.tap {|r| r.send(:apply_modules, *modules) }
end

def reverse_order
Expand Down Expand Up @@ -230,7 +230,7 @@ def build_select(arel, selects)
@implicit_readonly = false
# TODO: fix this ugly hack, we should refactor the callers to get an ARel compatible array.
# Before this change we were passing to ARel the last element only, and ARel is capable of handling an array
if selects.all? { |s| s.is_a?(String) || !s.is_a?(Arel::Expression) } && !(selects.last =~ /^COUNT\(/)
if selects.all? {|s| s.is_a?(String) || !s.is_a?(Arel::Expression) } && !(selects.last =~ /^COUNT\(/)
arel.project(*selects)
else
arel.project(selects.last)
Expand Down
4 changes: 1 addition & 3 deletions activerecord/test/cases/base_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,7 @@ def test_set_attributes

def test_set_attributes_without_hash
topic = Topic.new
assert_nothing_raised do
topic.attributes = ''
end
assert_nothing_raised { topic.attributes = '' }
end

def test_integers_as_nil
Expand Down

3 comments on commit 4a06489

@thisivan
Copy link

Choose a reason for hiding this comment

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

just curious... Why are you removing only the extra space after the opening brackets and leaving the space before the closing bracket in blocks?

@complex64
Copy link

Choose a reason for hiding this comment

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

Separation between the block itself and its contents, I guess.

@wincent
Copy link
Contributor

Choose a reason for hiding this comment

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

Funny, this commit introduces the asymmetrical bracketing. Then commit 3c300b3 removes it, then commit 0c2c893 puts it right back.

FWIW, I don't know of any style guide that advocates this kind of bracketing style:

{foo }

Please sign in to comment.