-
Notifications
You must be signed in to change notification settings - Fork 791
Adding --depth option to border command to recursively border subviews #126
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,33 +28,69 @@ def lldbcommands(): | |
|
|
||
|
|
||
| class FBDrawBorderCommand(fb.FBCommand): | ||
| colors = [ | ||
| "black", | ||
| "gray", | ||
| "red", | ||
| "green", | ||
| "blue", | ||
| "cyan", | ||
| "yellow", | ||
| "magenta", | ||
| "orange", | ||
| "purple", | ||
| "brown", | ||
| ] | ||
|
|
||
| def name(self): | ||
| return 'border' | ||
|
|
||
| def description(self): | ||
| return 'Draws a border around <viewOrLayer>. Color and width can be optionally provided.' | ||
| return 'Draws a border around <viewOrLayer>. Color and width can be optionally provided. Additionally depth can be provided in order to recursively border subviews.' | ||
|
|
||
| def args(self): | ||
| return [ fb.FBCommandArgument(arg='viewOrLayer', type='UIView/NSView/CALayer *', help='The view/layer to border. NSViews must be layer-backed.') ] | ||
|
|
||
| def options(self): | ||
| return [ | ||
| fb.FBCommandArgument(short='-c', long='--color', arg='color', type='string', default='red', help='A color name such as \'red\', \'green\', \'magenta\', etc.'), | ||
| fb.FBCommandArgument(short='-w', long='--width', arg='width', type='CGFloat', default=2.0, help='Desired width of border.') | ||
| fb.FBCommandArgument(short='-w', long='--width', arg='width', type='CGFloat', default=2.0, help='Desired width of border.'), | ||
| fb.FBCommandArgument(short='-d', long='--depth', arg='depth', type='int', default=0, help='Number of levels of subviews to border. Each level gets a different color beginning with the provided or default color'), | ||
| ] | ||
|
|
||
| def run(self, args, options): | ||
| colorClassName = 'UIColor' | ||
| isMac = runtimeHelpers.isMacintoshArch() | ||
| def setBorder(layer, width, color, colorClass): | ||
| lldb.debugger.HandleCommand('expr (void)[%s setBorderWidth:(CGFloat)%s]' % (layer, width)) | ||
| lldb.debugger.HandleCommand('expr (void)[%s setBorderColor:(CGColorRef)[(id)[%s %sColor] CGColor]]' % (layer, colorClass, color)) | ||
|
|
||
| depth = int(options.depth) | ||
| isMac = runtimeHelpers.isMacintoshArch() | ||
| color = options.color | ||
| assert color in self.colors, "Color must be one of the following: {}".format(" ".join(self.colors)) | ||
| colorClassName = 'UIColor' | ||
| if isMac: | ||
| colorClassName = 'NSColor' | ||
|
|
||
| layer = viewHelpers.convertToLayer(args[0]) | ||
| lldb.debugger.HandleCommand('expr (void)[%s setBorderWidth:(CGFloat)%s]' % (layer, options.width)) | ||
| lldb.debugger.HandleCommand('expr (void)[%s setBorderColor:(CGColorRef)[(id)[%s %sColor] CGColor]]' % (layer, colorClassName, options.color)) | ||
| if viewHelpers.isUIView(args[0]): | ||
| prevLevel = 0 | ||
| for view, level in viewHelpers.subviewsOfView(args[0]): | ||
| if level > depth: | ||
| break | ||
| if prevLevel != level: | ||
| color = self.nextColorAfterColor(color) | ||
| prevLevel = level | ||
| layer = viewHelpers.convertToLayer(view) | ||
| setBorder(layer, options.width, color, colorClassName) | ||
| else: | ||
| assert depth <= 0, "Recursive bordering is only supported for UIViews" | ||
| layer = viewHelpers.convertToLayer(args[0]) | ||
| setBorder(layer, options.width, color, colorClassName) | ||
|
|
||
| lldb.debugger.HandleCommand('caflush') | ||
|
|
||
| def nextColorAfterColor(self, color): | ||
| assert color in self.colors, "{} is not a supported color".format(color) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This assert shouldn't be necessary, since it's already checked earlier in the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this method shouldn't rely on being called from |
||
| return self.colors[(self.colors.index(color)+1) % len(self.colors)] | ||
|
|
||
| class FBRemoveBorderCommand(fb.FBCommand): | ||
| def name(self): | ||
|
|
@@ -63,14 +99,31 @@ def name(self): | |
| def description(self): | ||
| return 'Removes border around <viewOrLayer>.' | ||
|
|
||
| def options(self): | ||
| return [ | ||
| fb.FBCommandArgument(short='-d', long='--depth', arg='depth', type='int', default=0, help='Number of levels of subviews to unborder.') | ||
| ] | ||
|
|
||
| def args(self): | ||
| return [ fb.FBCommandArgument(arg='viewOrLayer', type='UIView/NSView/CALayer *', help='The view/layer to unborder.') ] | ||
|
|
||
| def run(self, args, options): | ||
| layer = viewHelpers.convertToLayer(args[0]) | ||
| lldb.debugger.HandleCommand('expr (void)[%s setBorderWidth:(CGFloat)%s]' % (layer, 0)) | ||
| lldb.debugger.HandleCommand('caflush') | ||
| def setUnborder(layer): | ||
| lldb.debugger.HandleCommand('expr (void)[%s setBorderWidth:(CGFloat)%s]' % (layer, 0)) | ||
|
|
||
| depth = int(options.depth) | ||
| if viewHelpers.isUIView(args[0]): | ||
| for view, level in viewHelpers.subviewsOfView(args[0]): | ||
| if level > depth: | ||
| break | ||
| layer = viewHelpers.convertToLayer(view) | ||
| setUnborder(layer) | ||
| else: | ||
| assert depth <= 0, "Recursive unbordering is only supported for UIViews" | ||
| layer = viewHelpers.convertToLayer(args[0]) | ||
| setUnborder(layer) | ||
|
|
||
| lldb.debugger.HandleCommand('caflush') | ||
|
|
||
| class FBMaskViewCommand(fb.FBCommand): | ||
| def name(self): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,6 +55,24 @@ def convertToLayer(viewOrLayer): | |
| else: | ||
| raise Exception('Argument must be a CALayer, UIView, or NSView.') | ||
|
|
||
| def isUIView(obj): | ||
| return fb.evaluateBooleanExpression('[(id)%s isKindOfClass:(Class)[UIView class]]' % obj) | ||
|
|
||
| # Generates a BFS of the views tree starting at the given view as root. | ||
| # Yields a tuple of the current view in the tree and its level (view, level) | ||
| def subviewsOfView(view): | ||
| views = [(view, 0)] | ||
| yield views[0] | ||
| while views: | ||
| (view, level) = views.pop(0) | ||
| subviews = fb.evaluateExpression('(id)[%s subviews]' % view) | ||
| subviewsCount = int(fb.evaluateExpression('(int)[(id)%s count]' % subviews)) | ||
| if subviewsCount > 0: | ||
| for i in range(0, subviewsCount): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, I adopted this code from one of the other files, so overlooked that. |
||
| subview = fb.evaluateExpression('(id)[%s objectAtIndex:%i]' % (subviews, i)) | ||
| views.append((subview, level+1)) | ||
| yield (subview, level+1) | ||
|
|
||
| def upwardsRecursiveDescription(view, maxDepth=0): | ||
| if not fb.evaluateBooleanExpression('[(id)%s isKindOfClass:(Class)[UIView class]]' % view) and not fb.evaluateBooleanExpression('[(id)%s isKindOfClass:(Class)[NSView class]]' % view): | ||
| return None | ||
|
|
||
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.
Did you want to have two separate asserts? One on the class, and another on depth?
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.
Can you please elaborate a bit, or give an example? Notice that I've changed the code a bit in a new commit, but assert still in place.
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 looks like it should be something more like:
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.
There might be a misunderstanding here. The purpose of this assert is to make sure recursive bordering is not used for layers (or anything that's not a UIView or NSView). However regular bordering is still permitted for layers. So if you're in the code path that takes you to this else block, we should validate that
depthis not provided to make sure recursive bordering won't happen.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 see. It reads like it's assert bad values on the depth, yet giving an unrelated error message.
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 see. It reads like it asserts bad values on the depth, yet giving an unrelated error message.
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.
Ok, I'll add a comment that should make it clearer.