Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Add trigger #563

Closed
wants to merge 3 commits into from

3 participants

@tiennou
Owner

This fixes #515.

I felt the blame about the first commit. Can't remember what I was thinking when I added a -release message to a Class object :-S. This prevented valid QSCommand objects from being returned directly, thus creating empty triggers.

The second is a quick hack to fix KVO notification, so that the Tree Controller behind the table sees that the trigger array has changed. I say 'hack' because I don't get why there's so many copies of the trigger array around here (QSTriggerCenter has the responsibility for triggers, but the pref pane had its own disconnected "copy" of it).

I guess this can be cleaned even more by making the Tree Controller listen to the real trigger array, and removing the useless triggerArray ivar and -setTriggerArray:.

@skurfer
Owner

Yep. I can see the trigger appear now. Are you going to do anything with the indirect object, or should we worry about that later?

@pjrobertson
Owner

Looks good to me. Removal of the optional argument is right I'd say.

I'll give it a run through if I have a minute

@tiennou
Owner

What about renaming the "Add Trigger" command to "Add Hotkey Trigger" (maybe keeping Add Trigger as an Alternate Name) ? Other trigger plugins could then have a "Add Blah Trigger" with their own specific optional argument and customize themselves based on that ?

At the moment there's no way of creating anything else that a Hotkey trigger, and you can't change a trigger's type after it has been created so people would get misled...

@skurfer
Owner

What about renaming the "Add Trigger" command to "Add Hotkey Trigger”

Sounds good to me.

@skurfer
Owner

This leads to a crash when you remove a trigger.

Process:         Quicksilver [89917]
Path:            /Applications/Quicksilver.app/Contents/MacOS/Quicksilver
Identifier:      com.blacktree.Quicksilver
Version:         ß63 (3912)
Code Type:       X86 (Native)
Parent Process:  launchd [214]

Date/Time:       2012-01-20 11:31:23.744 -0500
OS Version:      Mac OS X 10.7.2 (11C74)
Report Version:  9

Crashed Thread:  0  Dispatch queue: com.apple.main-thread

Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Codes: KERN_INVALID_ADDRESS at 0x00000000e08e51a7

VM Regions Near 0xe08e51a7:
    CG shared images       00000000c79b9000-00000000c7bd9000 [ 2176K] r--/r-- SM=SHM  
--> 
    Submap                 00000000ffff0000-00000000ffff2000          r-x/r-x process-only submap

Application Specific Information:
objc_msgSend() selector name: countByEnumeratingWithState:objects:count:
objc[89917]: garbage collection is OFF
Performing @selector(removeTrigger:) from sender QSGlossyBarButton 0xa3a3a00

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libobjc.A.dylib                 0x92060d47 objc_msgSend + 23
1   com.blacktree.Quicksilver       0x0001284c 0x1000 + 71756
@pjrobertson
Owner

@tiennou If you're about, would you be able to look over Rob's message about the crash?

I'm in pull request cleaning mode, so seeing you about I've jumped on this! :-)

@skurfer
Owner

I say we close this and attempt to recreate it (and fix the crash) after the 64-bit changes.

The original issue is still open and it can refer us here, which in turn can refer us to the branch that fixes things.

@skurfer
Owner

Are you OK with closing this @tiennou and rebasing/resubmitting it after the 64-bit stuff gets merged?

@tiennou tiennou closed this
@tiennou tiennou referenced this pull request
Merged

Add trigger the return #1345

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
2  Quicksilver/Code-App/QSTriggersPrefPane.m
@@ -279,6 +279,8 @@ - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(N
//}
- (IBAction)triggerChanged:(id)sender {
+ [self willChangeValueForKey:@"triggerArray"];
+ [self didChangeValueForKey:@"triggerArray"];
[triggerTable reloadData];
}
View
7 Quicksilver/Code-QuickStepCore/QSCommand.m
@@ -118,7 +118,7 @@ - (QSObject *)saveCommand:(QSObject *)dObject toPath:(QSObject *)iObject {
return [QSObject fileObjectWithPath:destination];
}
-- (QSObject*)addTrigger:(QSObject *)dObject withInfo:(QSObject*)iObject {
+- (QSObject*)addTrigger:(QSObject *)dObject {
#warning TODO: iObject contains a string which allows passing missing parameters (trigger type, mainly)
/* More TODO: Ask the trigger's trigger manager to parse the iObject stringValue,
* so that trigger manager get a chance to customize their properties */
@@ -179,9 +179,8 @@ + (QSCommand *)commandWithInfo:(id)info {
command = [QSCommand commandWithDictionary:info];
} else if ([info isKindOfClass:[NSString class]]) {
command = [QSCommand commandWithIdentifier:info];
- } else if (![info isKindOfClass:[QSCommand class]]) {
- [self release];
- return nil;
+ } else if ([info isKindOfClass:[QSCommand class]]) {
+ command = info;
}
return command;
}
View
2  Quicksilver/PlugIns-Main/QSCorePlugIn/QSCorePlugIn-Info.plist
@@ -634,7 +634,7 @@
<key>actionClass</key>
<string>QSCommandObjectHandler</string>
<key>actionSelector</key>
- <string>addTrigger:withInfo:</string>
+ <string>addTrigger:</string>
<key>directTypes</key>
<array>
<string>qs.command</string>
Something went wrong with that request. Please try again.