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

Action config #2423

Merged
merged 8 commits into from Mar 6, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 36 additions & 0 deletions Quicksilver/Code-QuickStepCore/QSAction.h
Expand Up @@ -30,9 +30,42 @@
- (SEL)action;
- (void)setAction:(SEL)newAction;

/**
* Use a predefined block as an action method. Useful for programatically creating many similar actions.
*
* @param actionBlock A block that does the action's work
* @param selectorName A selector name like "openThing:"
*
* @return YES if the action method was added to the provider, otherwise NO
*
* Example action block with no indirect object:
* QSObject *(^actionBlock)(id, QSObject *) = ^ QSObject *(id _self, QSObject *dObject) {
* // action code
* return nil;
* };
*/
- (BOOL)setActionUisngBlock:(QSObject *(^)(id, QSObject *))actionBlock selectorName:(NSString *)selName;
Copy link
Member

Choose a reason for hiding this comment

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

Uisng

Copy link
Member

Choose a reason for hiding this comment

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

Any specific reason you're using NSString * instead of SEL ? I'd prefer the latter (as well as a selector: parameter).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Because strings are “normal” and easy to create, while selectors are “weird” and difficult to create. “Difficult” meaning I had to look it up. I didn’t say it was a good reason. 😄 I was just trying to make it as simple as possible on the calling end.

Are you saying it should keep the string but also require you to create and pass in a SEL? Or drop the string?

Copy link
Member

Choose a reason for hiding this comment

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

I'd use -setActionUsingBlock:(…)block selector:(SEL)aSelector.

Take for example -mySelector:. If you mistype "mySelectr:" you will get a "missing method" at runtime. If you use @selector(mySelectr:), you willl/should get a compiler warning that it doesn't know about that particular selector.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, but the only time you ever type the name is during creation. It can be misspelled and still work. Also, I don’t think the compiler is aware of these methods even if you spell them correctly, so you’ll always get a warning. (I haven’t tested that today, but I think I was seeing such warnings during early attempts.)


/**
* Use a predefined block as an action method. Useful for programatically creating many similar actions.
*
* @param actionBlock A block that does the action's work
* @param selectorName A selector name like "openThing:usingOtherThing:"
*
* @return YES if the action method was added to the provider, otherwise NO
*
* Example action block with an indirect object:
* QSObject *(^actionBlock)(id, QSObject *, QSObject *) = ^ QSObject *(id _self, QSObject *dObject, QSObject *iObject) {
* // action code
* return nil;
* };
*/
- (BOOL)setActionWithIndirectUisngBlock:(QSObject *(^)(id, QSObject *, QSObject *))actionBlock selectorName:(NSString *)selName;

- (NSInteger)rank;
- (void)setRank:(NSInteger)newRank;
- (CGFloat)precedence;
- (void)setPrecedence:(CGFloat)precedence;
- (BOOL)enabled;
- (void)setEnabled:(BOOL)flag;
- (BOOL)menuEnabled;
Expand All @@ -46,6 +79,8 @@
- (BOOL)canThread;
- (BOOL)indirectOptional;
- (void)setIndirectOptional:(BOOL)flag;
- (BOOL)validatesObjects;
- (void)setValidatesObjects:(BOOL)flag;

// resolveProxy is a BOOL set in an action's dict to specify whether an object should be resolved
// before being sent to an action. Action's like 'assign abbreviation...' should not resolve the proxy
Expand All @@ -67,6 +102,7 @@
- (id)objectForKey:(NSString*)key;

- (NSString *)commandFormat;
- (void)setCommandFormat:(NSString *)commandFormat;
@end

@interface QSActionHandler : NSObject
Expand Down
51 changes: 49 additions & 2 deletions Quicksilver/Code-QuickStepCore/QSAction.m
Expand Up @@ -6,6 +6,7 @@
#import "QSResourceManager.h"
#import "NSBundle_BLTRExtensions.h"
#import "QSExecutor.h"
#import <objc/objc-runtime.h>

//static NSDictionary *actionPrecedence;

Expand Down Expand Up @@ -119,6 +120,12 @@ - (CGFloat)precedence {
return num ? [num doubleValue] : 0.0;
}

- (void)setPrecedence:(CGFloat)precedence
{
NSNumber *num = [NSNumber numberWithFloat:precedence];
[[self actionDict] setObject:num forKey:kActionPrecedence];
}

- (NSInteger)rank { return rank; }
- (void)_setRank:(NSInteger)newRank {
[self willChangeValueForKey:@"rank"];
Expand Down Expand Up @@ -192,6 +199,32 @@ - (void)setAction:(SEL)newAction {
[[self actionDict] removeObjectForKey:kActionSelector];
}

- (BOOL)setActionUisngBlock:(QSObject *(^)(id, QSObject *))actionBlock selectorName:(NSString *)selName
{
if (![self provider]) {
NSLog(@"define provider before setting a block as the action");
Copy link
Member

Choose a reason for hiding this comment

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

I'd be tempted to NSAssert that, but maybe I'm being too paranoid…

Copy link
Member Author

Choose a reason for hiding this comment

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

It’s mostly there so a developer will catch it during the plug-in’s creation. Wouldn’t an assert crash QS if the mythical developer doesn’t fix things, potentially punishing end users?

Copy link
Member

Choose a reason for hiding this comment

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

Well, if the mythical lets a crashing assert come up in the wild, isn't the mythical to blame for its sloppiness 😉 ?

NSLogs can and will be ignored by the developer, and by the user. Exceptions will not. I'd say document that "It is a programming error to use this method if the action doesn't have a provider".

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, if the mythical lets a crashing assert come up in the wild, isn't the mythical to blame for its sloppiness 😉 ?

Of course, but it’s the user that’ll feel the pain. Not the developer. The creators of all these plug-ins aren’t exactly jumping on issues they day they get reported. Or even the year they get reported.

NSLogs can and will be ignored by the developer, and by the user.

I don’t expect the log message to be what gets their attention. But when they realize their new action doesn’t exist, the message will explain why.

If we’re assuming they’re going to fix the issue during development one way or another, I suppose an exception gets the job done. I’ll change it.

return NO;
}
IMP actionFunction = imp_implementationWithBlock(actionBlock);
SEL actionSelector = NSSelectorFromString(selName);
BOOL actionDefined = class_addMethod([[self provider] class], actionSelector, actionFunction, "@@:@");
[self setAction:actionSelector];
Copy link
Member

Choose a reason for hiding this comment

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

Maybe logging the failure and returning NO if the method couldn't be added would be more fool-proof 😉.

Copy link
Member Author

Choose a reason for hiding this comment

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

I’ve made this a little better.

return actionDefined;
}

- (BOOL)setActionWithIndirectUisngBlock:(QSObject *(^)(id, QSObject *, QSObject *))actionBlock selectorName:(NSString *)selName
{
if (![self provider]) {
NSLog(@"define provider before setting a block as the action");
return NO;
}
IMP actionFunction = imp_implementationWithBlock(actionBlock);
SEL actionSelector = NSSelectorFromString(selName);
BOOL actionDefined = class_addMethod([[self provider] class], actionSelector, actionFunction, "@@:@@");
[self setAction:actionSelector];
return actionDefined;
}

- (NSInteger)argumentCount {
id obj = [[self actionDict] objectForKey:kActionArgumentCount];
if (obj)
Expand Down Expand Up @@ -222,6 +255,15 @@ - (void)setIndirectOptional:(BOOL)flag {
[[self actionDict] setObject:[NSNumber numberWithInteger:flag] forKey:kActionIndirectOptional];
}

- (BOOL)validatesObjects
{
return [[[self actionDict] objectForKey:kActionValidatesObjects] boolValue];
}
- (void)setValidatesObjects:(BOOL)flag
{
[[self actionDict] setObject:[NSNumber numberWithInteger:flag] forKey:kActionValidatesObjects];
}

- (BOOL)resolvesProxy {
if ([[self actionDict] objectForKey:kActionResolvesProxy] == nil) {
return YES;
Expand Down Expand Up @@ -364,7 +406,7 @@ - (NSString *)commandFormat {

//Check the action dictionary
if (!format)
format = [[self actionDict] objectForKey:@"commandFormat"];
format = [[self actionDict] objectForKey:kActionCommandFormat];

// Check the main bundle
if (!format)
Expand All @@ -377,6 +419,11 @@ - (NSString *)commandFormat {
return format;
}

- (void)setCommandFormat:(NSString *)commandFormat
{
[[self actionDict] setObject:commandFormat forKey:kActionCommandFormat];
}

- (CGFloat)score {
return 0.0;
}
Expand Down Expand Up @@ -426,4 +473,4 @@ - (BOOL)loadIconForObject:(QSObject *)object {
return NO;
}

@end
@end
1 change: 1 addition & 0 deletions Quicksilver/Code-QuickStepCore/QSDefines.h
Expand Up @@ -58,6 +58,7 @@
#define kActionInitialize @"initialize"
#define kActionEnabled @"enabled"
#define kActionResolvesProxy @"resolvesProxy"
#define kActionCommandFormat @"commandFormat"

// NSNumber (float) :
#define kActionPrecedence @"precedence"
Expand Down