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

Don't add multiple actions with the same ID. Fixes #1259 #1260

Merged
merged 1 commit into from Nov 30, 2012

Conversation

pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Nov 30, 2012

The code for dealing with duplicates was already in the code, but for some strange reason it just removed the directTypes and directFileTypes for the old action, before adding the new action as well.

This kind of make the old action (the one that was added first) pointless as it's direct types weren't set properly.

I've gone with the view that IDs should be unique. If there's already an action with the same ID that exists, then do nothing. Don't replace it, don't try and do anything fancy :)

I could have fixed #1259 in the services plugin but thought it was better to get it right from the start :)

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Nov 30, 2012

P.S. this could go into release I guess…?

@skurfer
Copy link
Member

@skurfer skurfer commented Nov 30, 2012

Lazy research: Where are the duplicates coming from? When a service supports more than one type, does it add the action once for each type? If that's the case, don't we want to add the additional supported types to the action, rather than leaving it untouched and only considering the first type advertised?

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Nov 30, 2012

They're actually exactly the same services that get added multiple times

Here's how we get the services:

// returns an array of path strings giving the paths of the bundle that contains the service
+ (NSArray *)allServiceActions {
    NSMutableSet *actionObjects = [NSMutableSet setWithCapacity:1];
    [actionObjects addObjectsFromArray:QSServicesPlugin_applicationProviders()];
    [actionObjects addObjectsFromArray:QSServicesPlugin_providersAtPath(@"/System/Library/Services/")];
    [actionObjects addObjectsFromArray:QSServicesPlugin_providersAtPath(@"/Library/Services/")];
    [actionObjects addObjectsFromArray:QSServicesPlugin_providersAtPath(@"~/Library/Services/")];

    return [actionObjects allObjects];
}

and using a set there doesn't actually work, because the objects are all unique (different addresses in memory). I s'pose we could possible fix that, but I still think we should ignore duplicate action IDs in the main app.

Unless two different apps define exactly the same service name, then this shouldn't be a problem. Is that likely?

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Nov 30, 2012

…and if they do, then we shouldn't just use the service name as the ID, but the service name + bundle ID or something

@skurfer
Copy link
Member

@skurfer skurfer commented Nov 30, 2012

OK. Sounds good. I had this issue and it seems to be fixed now.

skurfer added a commit that referenced this issue Nov 30, 2012
Don't add multiple actions with the same ID. Fixes #1259
@skurfer skurfer merged commit f2aac70 into quicksilver:master Nov 30, 2012
@skurfer
Copy link
Member

@skurfer skurfer commented Nov 30, 2012

You want it in release, or do you want 1.0 to be that much more impressive? :-)

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Nov 30, 2012

mmm…. release? Now that it's been brought to my attention, it's really
gonna annoy me :P

On 30 November 2012 15:42, Rob McBroom notifications@github.com wrote:

You want it in release, or do you want 1.0 to be that much more
impressive? :-)

skurfer added a commit that referenced this issue Nov 30, 2012
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.

2 participants