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

display file icons before attempting to load Quick Look previews #1554

Merged
merged 2 commits into from Aug 9, 2013
Merged

Conversation

skurfer
Copy link
Member

@skurfer skurfer commented Aug 6, 2013

As discussed in #1552, attempting to load Quick Look previews before giving up and showing the file's icon instead introduced a noticeable delay. I changed it to show the file's icon (if it exists) first. The obvious downsides are the extra work of loading two icons, and sending two notifications. But loading the file's icon is usually lightning quick. (Almost no one complained about it when it was done on the main thread.) And if there is no Quick Look preview, the second notification never happens.

skurfer added 2 commits Aug 5, 2013
It turns out that `[[NSWorkspace sharedWorkspace] iconForFileType:@"app"]` returns the generic document icon instead of the generic application icon.
There can be a noticeable delay when trying to get a preview, only to find there isn't one and the file's icon should be used instead. So, show that first.

If there is no QL preview, calling `setIcon:` a second time with the file's icon is a no-op, so it should be fine.

fixes #1552
@skurfer
Copy link
Member Author

@skurfer skurfer commented Aug 9, 2013

So if we can merge this guy, I think it's time for another pre-release.

Do we want to move 32-bit plug-ins to a disabled folder or something? I don't think it would make a difference for #1546, but it might help some things. Also, I'm wondering if we should call this 1.1.0 instead of 1.0.1, since there's a break with backward compatibility.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Aug 9, 2013

Moving disabled plugins to a separate folder isn't a bad idea. Seems Alcor did this before (but for all the plugins) - this used to be in QSController:

            if (lastVersion < [@"2000" hexIntValue]) {
                NSFileManager *fm = [NSFileManager defaultManager];
                [fm moveItemAtPath:QSApplicationSupportSubPath(@"PlugIns", NO) toPath:QSApplicationSupportSubPath(@"PlugIns (B40 Incompatible) ", NO) error:nil];
                [fm moveItemAtPath:@"/Library/Application Support/Quicksilver/PlugIns" toPath:@"/Library/Application Support/Quicksilver/PlugIns (B40 Incompatible) " error:nil];
            }

Maybe in the try/catch/throw code for plugin loading we could move disabled 32 bit plugins somewhere else. It'd clean up the Console a bit as well (on launch)

pjrobertson added a commit that referenced this issue Aug 9, 2013
display file icons before attempting to load Quick Look previews
@pjrobertson pjrobertson merged commit 7ea1936 into quicksilver:release Aug 9, 2013
@skurfer skurfer deleted the i1552 branch Aug 9, 2013
skurfer added a commit that referenced this issue Aug 9, 2013
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

2 participants