-
Notifications
You must be signed in to change notification settings - Fork 34
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.
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
Add depth option #86
base: master
Are you sure you want to change the base?
Add depth option #86
Conversation
I would love to see this merged in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is super nice! Thank you for all this work!
I have a few small suggestions.
I'm also thinking that for really large objects we might want to truncate nested objects rather than just put them on a single line. That could be a later enhancement.
end | ||
|
||
def reached_depth_limit | ||
!depth_limit.nil? && depth >= depth_limit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice for this to live in something like DepthTracker#reached_depth_limit?
!depth_limit.nil? && depth >= depth_limit | ||
end | ||
|
||
def single_line_if_reached_depth(&blk) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⛏️ what do you think of changing this method name to with_depth_tracking
?
@@ -44,6 +44,7 @@ raw: false, # Do not recursively format instance variables. | |||
sort_keys: false, # Do not sort hash keys. | |||
sort_vars: true, # Sort instance variables. | |||
limit: false, # Limit arrays & hashes. Accepts bool or int. | |||
depth: nil, # Limit number of nested objects the multiline can go deep. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than adding a new configuration, what do you think of allowing multiline
to be an integer? We could support both true/false and 0+ values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like @HarlemSquirrel's suggestions.
@gabrielcosta42 have you had a chance to look over the suggestions? |
@gabrielcosta42 checking in to see if you have some time to review my suggestions. |
@gabrielcosta42 Checking to see if you have any time to finish up this pull request for Hacktoberfest |
@gooroodev can you review this PR? |
Thank you for tagging me, @admsev!
Summary of Changes
Identified Issues
Proposed Fixes1. Ensure Block is Provided in
def track_depth
raise "No block given" unless block_given?
@depth += 1
yield
ensure
@depth -= 1
end 2. Rename
def force_single_line_if_depth_limit_reached(&blk)
multiline = options[:multiline]
options[:multiline] = false if reached_depth_limit
track_depth { yield }
ensure
options[:multiline] = multiline
end
def simple_array
force_single_line_if_depth_limit_reached do
if options[:multiline]
multiline_array
else
"[ #{array.map { |item| inspector.awesome(item) }.join(', ')} ]"
end
end
end
def simple_hash
force_single_line_if_depth_limit_reached do
if multiline_hash?
multiline_hash
else
"{ #{printable_hash.join(', ')} }"
end
end
end
single_line_if_reached_depth do
data = (options[:sort_vars] ? vars.sort : vars).map do |declaration, var|
key = left_aligned do
align(declaration, declaration.size)
end
unless options[:plain]
key = if key =~ /(@\w+)/
key.sub(Regexp.last_match(1), colorize(Regexp.last_match(1), :variable))
else
key.sub(/(attr_\w+)\s(:\w+)/, "#{colorize('\\1', :keyword)} #{colorize('\\2', :method)}")
end
end
indented do
key + colorize(' = ', :hash) + inspector.awesome(object.instance_variable_get(var))
end
end
if options[:multiline]
"#<#{awesome_instance}\n#{data.join(%(,\n))}\n#{outdent}>"
else
"#<#{awesome_instance} #{data.join(', ')}>"
end
end
single_line_if_reached_depth do
data = vars.sort.map do |declaration, var|
key = left_aligned do
align(declaration, declaration.size)
end
unless options[:plain]
key = if key =~ /(@\w+)/
key.sub(Regexp.last_match(1), colorize(Regexp.last_match(1), :variable))
else
key.sub(/(attr_\w+)\s(:\w+)/, "#{colorize('\\1', :keyword)} #{colorize('\\2', :method)}")
end
end
indented do
key + colorize(' = ', :hash) + inspector.awesome(struct.send(var))
end
end
if options[:multiline]
"#<#{awesome_instance}\n#{data.join(%(,\n))}\n#{outdent}>"
else
"#<#{awesome_instance} #{data.join(', ')}>"
end
end General ReviewThe code changes introduce a new feature to limit the depth of nested objects in the multiline output. The implementation is well-structured and integrates seamlessly with the existing codebase. The addition of the -- |
Feature requested on #84.
Adds a 'depth' option to limit how many times the multiline can be repeated on nested objects.
Applied to: