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.

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

Added FBPrintWindow command #239

Merged
merged 6 commits into from
May 29, 2018
Merged

Conversation

PeteTheHeat
Copy link
Contributor

I've been working with input accessory views quite a bit. They've been annoying to debug because they don't live in the main key window, they are hosted in the Keyboard's window. I added a short chisel command locally to make printing a window easy. Let me know if this is something useful for the library.

@kastiglione
Copy link
Contributor

Thanks Pete. I like it. I have some thoughts.

This is the same as pviews, except pviews is hard coded to use keyWindow. What do you think about making this a flag on pviews? For example: pviews -w 0, pviews -w 1, etc.

@kastiglione
Copy link
Contributor

@PeteTheHeat what do you think about my suggestion?

@PeteTheHeat
Copy link
Contributor Author

Yeah that makes sense! Let me update it.

Copy link
Contributor

@kastiglione kastiglione left a comment

Choose a reason for hiding this comment

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

Looks great. I have one comment.

if arguments[0] == '__keyWindow_dynamic__':
arguments[0] = '(id)[[UIApplication sharedApplication] keyWindow]'

if window > 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this check can be removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see now why you have this. Never mind :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup I was hoping the if/else flow was clear, if a window is passed in, do some new stuff, else fall back to what the code was doing before.

@@ -20,6 +20,7 @@
def lldbcommands():
return [
FBPrintViewHierarchyCommand(),
FBPrintWindow(),
Copy link
Contributor

@kastiglione kastiglione May 29, 2018

Choose a reason for hiding this comment

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

Now that pviews supports -w, I don't think it's worth having a separate command. A user could get pwindow by adding this to their ~/.lldbinit:

command alias pwindow pviews --window %1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, I tried to delete it but it came back in the merge somehow lol, I'm too used to hg these days. I'll remove it

@kastiglione kastiglione merged commit d0b4d2d into facebook:master May 29, 2018
@kastiglione
Copy link
Contributor

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants