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

Attempt to add internal objs/messages to QS permanently #1373

Closed
wants to merge 1 commit into from

Conversation

pjrobertson
Copy link
Member

Internal Objs/messages currently only work if the catalog entries are enabled. Otherwise triggers etc. that use them just fail miserably.

This is an attempt to fix that, although it may not be the best. Open on the quicksilver repo for collab :)

+[QSObject cleanObjectDictionary] may well clear the items straight away, but testing seems to show that adding them to the catalog gives triggers etc. enough time to use the objects and hold on to them.

Note that `+[QSObject cleanObjectDictionary]` may well clear the items straight away, but testing seems to show that adding them to the catalog gives triggers etc. enough time to use the objects and hold on to them
@skurfer
Copy link
Member

skurfer commented Feb 21, 2013

How is that different from any other object? (Triggers fail if they aren't in the catalog.) I forget what brought this about, so maybe it deserves to be treated as a special case.

I also noticed (and this happens on master too) that there's an exception when trying to view the contents of "Internal Objects" in the catalog prefs. Seems to be because they're missing both name and label, so [[NSMutableAttributedString alloc] initWithString:nameString] is being called with nil. Still looking into why.

@skurfer
Copy link
Member

skurfer commented Feb 21, 2013

Never mind. Just re-read #1371. Sorry. :-)

@HenningJ
Copy link
Contributor

HenningJ commented Nov 6, 2013

Can one of the admins verify this patch?

@skurfer
Copy link
Member

skurfer commented Jul 26, 2016

Can we close this? Between #1648 and #1886, is the problem this was supposed to fix even there?

@pjrobertson
Copy link
Member Author

...no idea! Is what this was trying to fix actually fixed?
I'm kind of for the point that you shouldn't have to enable the catalog entry to use an object in a trigger.
If I want to add some file object that isn't in the catalog entry to a trigger, I can. If I want to use a URL that isn't in the catalog in a trigger, I can. If I wanted to use an iTunes artist in a trigger (but not keep it in the catalog, because that's too messy, then I should be able to)

Just checked - the answer is no. If I enable the 'internal objects' catalog, create a Trigger for 'Clipboard ⇥ Paste' then disable the catalog entry the object immediately disappears.

Again, this is probably an issue with how the object is stored in the trigger, which might be addressed in the "archiving objects" discussion. I agree this implementation is messy

@pjrobertson
Copy link
Member Author

Basically, the crux of it is that the object has an ID of QSClipboardObject and it can't be found/recreated with that ID ;-)

@skurfer
Copy link
Member

skurfer commented Jul 27, 2016

OK, then I think the general fix for #1277 should cover this, too.

@skurfer skurfer mentioned this pull request Jul 27, 2016
1 task
@pjrobertson
Copy link
Member Author

OK, I've created a 'places to test' list on that issue to keep a note of this problem. Whenever we come across somewhere that breaks because of this we can add it to that list to test them all (and perhaps create unit tests) before we merge the final PR

@pjrobertson pjrobertson deleted the internalObjs branch July 27, 2016 03:29
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