Add trigger the return #1345

Merged
merged 10 commits into from Feb 1, 2013

Projects

None yet

3 participants

@tiennou
Member
tiennou commented Jan 22, 2013

Fixes #515, see #563 for reference (except this one is more commit-y than the old).

I tried creating one trigger with the action ('pizza' > Large type, got that one from an old tutorial ;-)), then deleting it. No crash.

tiennou added some commits Nov 10, 2011
@tiennou tiennou Fix empty trigger creation after invoking "Add Trigger". ff4789b
@tiennou tiennou Use bindings instead of having a copy of the triggers just for the pr…
…eferences.

Also makes the QSTriggerManager informal protocol a formal one, and move it in QSTriggerManager.h.
The changes to the xib file are bindings for the infoButton & deleteButton.
d1e0410
@pjrobertson
Member

I tried creating one trigger

Only one? Rock solid testing ;-)

Cheers for getting this out. So... does that mean that there are no more outstanding v1.0 issues to fix? I'm scared!

@tiennou
Member
tiennou commented Jan 23, 2013

I just rebased the old add-trigger branch I had ;-).

@skurfer
Member
skurfer commented Jan 23, 2013

Yep. Looks good. No crashing this time. I reverse my opinion on renaming it to "Add Hotkey Trigger". I say we leave it, because once the trigger is created, you can change the type. (You have to edit it anyway, so I think that's fine.)

How hard would it be to select the new trigger when the preferences open? :-)

@pjrobertson

Could you just [nc removeObserver:self]
Saves 2 lines of code, not that important ;-)

Owner

Because I know which notifications I have registered, and the superclass (or one of our subclasses) might have others notifications registered. I'm being pedantic ;-).

So was I :P
But surely if an object is being dealloced then it doesn't matter if the super/sub classes are observers, the obj is being dealloced!

Anyway, maybe we should just move to ARC then we can't use dealloc - or argue ;-)

@tiennou
Member
tiennou commented Jan 23, 2013

Rebased. Now supports passing the trigger type as an indirect. I'll fix the trigger selection in the pref pane and await your opinions ;-).

@tiennou
Member
tiennou commented Jan 23, 2013

Also, I removed the xib file change from that list, but that didn't make it work :-(.

@pjrobertson
Member

Please see a couple of my commits here that fix a few small things. I haven't fixed the pixelated-ness of the iObjects ;-)

@skurfer skurfer merged commit 0aaec29 into quicksilver:master Feb 1, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment