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

allow type-specific smart spacebar behavior #2072

Merged
merged 2 commits into from Jul 8, 2015
Merged

allow type-specific smart spacebar behavior #2072

merged 2 commits into from Jul 8, 2015

Conversation

skurfer
Copy link
Member

@skurfer skurfer commented Jul 8, 2015

A small enhancement to address some of the concerns raised in the discussion on #925.

Basically, for any type in QSTypeDefinitions, in addition to the name and icon, you can also specify an integer for one of the existing spacebar behaviors.

This made the special-case code for URLs unnecessary, so I removed it. I’ve added the necessary entry for URLs to the plist in the core plug-in. If this is merged, I’ll also add something for search URLs to the web search plug-ins, and iTunes tracks in that plug-in.

I’ll document the new key and its possible values in the plug-in ref, too.

@pjrobertson
Copy link
Member

pjrobertson commented Jul 8, 2015

All pretty simple and nice and clean. Go ahead and merge once you've added the consts (if you want to)

pjrobertson added a commit that referenced this pull request Jul 8, 2015
allow type-specific smart spacebar behavior
@pjrobertson pjrobertson merged commit b2e90dc into master Jul 8, 2015
@pjrobertson pjrobertson deleted the smartspace branch Jul 8, 2015
skurfer added a commit that referenced this pull request Jul 9, 2015
@skurfer
Copy link
Member Author

skurfer commented Jul 9, 2015

Thanks for the quick merge. I’ve updated the plug-in ref and put out a new Web Search plug-in. No one will see it because it requires build 4012. I did that because it also contains an incompatible change to work with @tiennou’s catalog entry updates (that I wasn’t aware of until I tried to build the plug-in).

I’ve also updated iTunes, but there are more changes planned there, so I haven’t released it.

@tiennou
Copy link
Member

tiennou commented Jul 9, 2015

About that, I'm currently cleaning up QSObjectSource, so there will be movement in that section (like backward-incompatible movement). I'm trying to keep the incompatible changes in a branch on top of my cleanup so we can decide when we want all plugins to be rebuild (unless I find a workaround that allows me to detect new-style source vs the current ones, which I tried a few times and failed at).

@skurfer
Copy link
Member Author

skurfer commented Jul 9, 2015

I'm currently cleaning up QSObjectSource

And doing something about #1766, I hope. 😉

I'm trying to keep the incompatible changes in a branch on top of my cleanup so we can decide when we want all plugins to be rebuild

Sounds good. I’ve got a list of plug-ins that were using QSCatalogEntryChanged and will need to be updated for the next release. Do you know of any other changes we need to make related to the catalog entry clean-up?

(unless I find a workaround that allows me to detect new-style source vs the current ones, which I tried a few times and failed at)

Are there any methods being added/renamed/removed? If so, respondsToSelector seems like a good candidate. But if upgrading the plug-ins won’t be too bad, I’d rather just do that instead of having a bunch of branching code in the core app to support legacy stuff.

@tiennou
Copy link
Member

tiennou commented Jul 9, 2015

And doing something about #1766, I hope. 😉

In fact, there's already the beginning of this, in the form of -enableEntry:/disableEntry:. It's just that the rest of the code expect notifications, and will brainlessly reload the entry's contents. I'll see what can be done about that, because it's sorely needed.

I'm trying to keep the incompatible changes in a branch on top of my cleanup so we can decide when we want all plugins to be rebuild

Sounds good. I’ve got a list of plug-ins that were using QSCatalogEntryChanged and will need to be updated for the next release. Do you know of any other changes we need to make related to the catalog entry clean-up?

(unless I find a workaround that allows me to detect new-style source vs the current ones, which I tried a few times and failed at)

Are there any methods being added/renamed/removed? If so, respondsToSelector seems like a good candidate. But if upgrading the plug-ins won’t be too bad, I’d rather just do that instead of having a bunch of branching code in the core app to support legacy stuff.

I'm making @protocol QSObjectSource, fixing the delegate methods to pass entries instead of dictionaries (that's the breaking change), and I found no easy way of knowing beforehand if an entry expects a dict or a real entry, which is why it's breaking. The ugly way was adding a -(BOOL)newStuff; method in the protocol that returns NO and that the entry can use to know what to pass to its source (new source would override it to YES). But then we'd have to cleanup that sometime later, and given the -selection/currentEntry mess there's already in I'd be in favor of just breaking it ;-).

@tiennou
Copy link
Member

tiennou commented Jul 9, 2015

(Bonus question. What's the difference between -lastScanDate and an entry's indexation date ? What happens to non-indexable entries ? 😩)

@skurfer
Copy link
Member Author

skurfer commented Jul 9, 2015

Moving this discussion to #1766

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

3 participants