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

Fix a bug where mouse trigger objects weren't being resolved #542

Merged
merged 1 commit into from Nov 7, 2011

Conversation

pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Oct 30, 2011

Fix: make sure the object has an identifier before returning it.

This is an absolutely tiny change.
Basically, sometimes when 'object' was set (see the line I've changed) it still didn't necessarily point to a resolved object.
If the identifier is set then it definitely points to a resolved object :)

Fix: make sure the object has an identifier before returning it.
@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Oct 30, 2011

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Oct 30, 2011

OK turns out that won't work fully.

The right thing to do is to just revert the change I made here:
0ed2054#diff-0

i.e. removing if(object) return object;

The reason my above change will not work is because not all dObjects respond to identifier. Most notably QSRanked objects - i.e. all objects in the 1st pane.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Oct 30, 2011

...so please don't merge this. Fixed pull coming tomorrow (?)

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Nov 4, 2011

OK - for those who've been looking at this (@skurfer perhaps?!)

After further debugging the change I make doesn't seem to be a problem at all (as I previously said it might), but fixes things correctly.

If anyone is running QS in debug could they stick a breakpoint in QSCommand.m (QSObject *)dObject; (any branch will do)
and just get a collection of what objects you get when you 'po object' in the command line.

Thanks

@skurfer
Copy link
Member

@skurfer skurfer commented Nov 4, 2011

OK. I couldn’t get this to work at all, but it’s because I set the trigger for “Drag Entered” instead of “Mouse Entered”. Now that I’ve sorted that out, the trigger appears to survive a restart. I’ll test a bit more and merge later.

@skurfer
Copy link
Member

@skurfer skurfer commented Nov 4, 2011

I put a breakpoint on line 310. It gets hit twice, so I ran the command each time …after I figured out where the command-line was.

(lldb) po object
(QSObject *) $20 = 0x0a3b0c00 QSProxyObject <0xa3b0c00>, QSCurrentApplicationProxy
(lldb) po object
(QSObject *) $41 = 0x0a3b0c00 QSProxyObject <0xa3b0c00>, QSCurrentApplicationProxy

Trying this from the latest master gives this:

(lldb) po object
(QSObject *) $121 = 0x0412d420 QSObject <0x412d420>, (null)

So it looks fixed to me.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Nov 6, 2011

after I figured out where the command-line was.

po is REALLY useful. You can po (print object) or just p (as in p var) if it's not a pointer (e.g. int)
You can see inside dictionaries, run commands like po [dObject identifier] or po [object resolvedObject] etc. :)

So it looks fixed to me.

So you didn't get the method called any other time? I thought over it while I was away this weekend and I think the fix I've done is fine. Worst case scenario: the object doesn't get returned instantly and it just has to be resolved again (how it used to be)

@skurfer
Copy link
Member

@skurfer skurfer commented Nov 7, 2011

po is REALLY useful. You can po (print object) or just p (as in p var) if it's not a pointer (e.g. int) You can see inside dictionaries, run commands like po [dObject identifier] or po [object resolvedObject] etc. :)

Good to know, though I wish I knew why that info wasn’t just showing up automatically on the left side of the debugger.

So you didn't get the method called any other time? I thought over it while I was away this weekend and I think the fix I've done is fine. Worst case scenario: the object doesn't get returned instantly and it just has to be resolved again (how it used to be)

Well, I hit the breakpoint when QS launches and whenever an object gets pulled into the first pane, etc. The “twice” I referred to is when activating the mouse trigger.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Nov 7, 2011

Well, I hit the breakpoint when QS launches and whenever an object gets
pulled into the first pane, etc. The twice I referred to is when
activating the mouse trigger.

It all seems good now, I've done some more testing. Now we have #555 to
deal with, but I think that's separate again.

On 7 November 2011 13:56, Rob McBroom <
reply@reply.github.com>wrote:

po is REALLY useful. You can po (print object) or just p (as in p var)
if it's not a pointer (e.g. int) You can see inside dictionaries, run
commands like po [dObject identifier] or po [object resolvedObject] etc. :)

Good to know, though I wish I knew why that info wasnt just showing up
automatically on the left side of the debugger.

So you didn't get the method called any other time? I thought over it
while I was away this weekend and I think the fix I've done is fine. Worst
case scenario: the object doesn't get returned instantly and it just has to
be resolved again (how it used to be)

Well, I hit the breakpoint when QS launches and whenever an object gets
pulled into the first pane, etc. The twice I referred to is when
activating the mouse trigger.


Reply to this email directly or view it on GitHub:
#542 (comment)

@skurfer
Copy link
Member

@skurfer skurfer commented Nov 7, 2011

Agreed.

skurfer added a commit that referenced this issue Nov 7, 2011
Fix a bug where mouse trigger objects weren't being resolved
@skurfer skurfer merged commit e9ff905 into quicksilver:master Nov 7, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants