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

Order recent docs on 10.11 properly #2181

Merged
merged 1 commit into from Feb 2, 2016
Merged

Order recent docs on 10.11 properly #2181

merged 1 commit into from Feb 2, 2016

Conversation

pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Jan 30, 2016

It came to my attention recently that → into apps doesn't seem to leave files in the right order anymore under El Capitan. That's because they're no longer stored in the plists (.sfl files) in the order they were opened, but are stored in a random order.

Since they're stored as the new SFLListItem class (using a keyed archiver) in 10.11, which has a property order, we can use that to reorder the items.

I took inspiration from http://pastebin.com/dYxrGX7c

@pjrobertson pjrobertson force-pushed the recentDocsOrder branch 2 times, most recently from b52efef to dbeecc6 Compare Jan 30, 2016
@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Jan 30, 2016

I hope I managed to rebase and force push quick enough before people could see my poor indentation? ... 😟

I should really practice what I preach eh? ;-)

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Jan 30, 2016

@skurfer
Copy link
Member

@skurfer skurfer commented Jan 31, 2016

Looks good, works as advertised, and wipes out one of the edge cases I was worried about in the “sort by date” implementation.

My only concern is this:

Class SFLListItem is implemented in both /System/Library/Frameworks/CoreServices.framework/Versions/A/Frameworks/SharedFileList.framework/Versions/A/SharedFileList and /Applications/Quicksilver.app/Contents/Frameworks/QSFoundation.framework/Versions/A/QSFoundation. One of the two will be used. Which one is undefined.

Are you not content blindly sending messages to those objects without “knowing” what they are like I was? 😛 Can we just add a category to the class instead of re-implementing it?

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Feb 1, 2016

Are you not content blindly sending messages to those objects without “knowing” what they are like I was?

Hmmm… I didn’t see that error message!
Is there any way we can #import that header then, or is it totally private? Looks to me like it’s just a binary so I’d suggest totally private.
Perhaps we can make it a protocol, and say that the object is id <SharedFileListProtocol>, or maybe the best alternative is just to use a different name like QSSharedFileList

@tiennou what are your thoughts?

On 1 Feb 2016, at 02:56, Rob McBroom <notifications@github.com mailto:notifications@github.com> wrote:

Are you not content blindly sending messages to those objects without “knowing” what they are like I was?

@tiennou
Copy link
Member

@tiennou tiennou commented Feb 1, 2016

class-dump that framework, grab the header and drop that in External ? It might also be acceptable to just uncheck the implementation you provide, so the runtime warning goes away and the compiler finds its definition.

Changing the NSKeyedArchiver is prone to do weird things if ever we (or some Apple stuff) tries to unarchive some SFLListItem. I'm also interested if there's a behavioral difference between our code and the example code from the blog post (e.g. is it necessary to tell NSKeyedArchiver about that new class that it might already know about if we link against CoreServices ?).

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Feb 1, 2016

class-dump that framework, grab the header and drop that in External ? It might also be acceptable to just uncheck the implementation you provide, so the runtime warning goes away and the compiler finds its definition.

Yeah I thought about that, but we actually do need to implement initWithCoder: in our code.
I tried just doing what Rob was previously doing and calling

float order = [item order]

but this was returning really weird results. Doing print (float) [item order] in the debugger gave the right number, but saving it to a variable then trying to use it was giving some obviously incorrect number. I figured it was maybe something to do with casting to the wrong type or 64/32 bit or something but I couldn't figure it out - so I left it as it is.

Just realised using a protocol won't work since we need to call

// Set up SFLListItem for unarchiving. See SFLListItem_Private.h
[NSKeyedUnarchiver setClass:[SFLListItem class] forClassName:@"SFLListItem"];

Maybe the thing to do is:

  • class dump and add the header file
  • subclass it as QSSFLListItem
  • override initWithCoder: and call [super initWithCoder:aCoder]; first?

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Feb 1, 2016

is it necessary to tell NSKeyedArchiver about that new class that it might already know about if we link against CoreServices

They do it as well. See line 22-23

# Create the association for the class name
NSKeyedUnarchiver.setClass_forClassName_(SFLListItem, "SFLListItem")

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Feb 2, 2016

OK @tiennou @skurfer
I've force pushed (sorry) but it's now much cleaner. A simple class dump and #import was all that was required :-)

Add new 'SFLListItem class'
@skurfer
Copy link
Member

@skurfer skurfer commented Feb 2, 2016

I've force pushed (sorry) but it's now much cleaner.

No problem. I feel better about how it is now. After using the rough draft for a couple of days, I can’t believe I lived with the previous annoying behavior.

skurfer added a commit that referenced this issue Feb 2, 2016
Order recent docs on 10.11 properly
@skurfer skurfer merged commit 686deb9 into master Feb 2, 2016
2 checks passed
@skurfer skurfer deleted the recentDocsOrder branch Feb 2, 2016
skurfer added a commit that referenced this issue Feb 2, 2016
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