Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Some improvements #17

Merged
merged 3 commits into from

5 participants

@nickhutchinson

Hi there,

I've made a number of changes, the most significant of which is using the file(1) command to determine whether a file is textual or binary, and its encoding. Although I haven't tested it, I suspect this is much more efficient than using -[stringWithContentsOfURL] to slurp in an entire file, especially in pathological conditions like a 1GB text file on a slow network.

I've also moved the project to ARC and Modern Objective-C. This does loose you support for 10.6 32-bit, however.

Finally, thumbnail generation now works, and files are given an appropriate badge according to the heuristic described in the commit message.

I haven't contributed to an open source project before, so please let me know if I'm Doing It Wrong.

Cheers,
Nick

nickhutchinson added some commits
@nickhutchinson nickhutchinson Remove build folder, and add to .gitignore. b31aa53
@nickhutchinson nickhutchinson Make preview generation more robust; stop previewing binary files.
* Check candidate files with file(1) to decide whether they are text or binary, and to determine their encoding.
* Upgrade to ARC (note this drops support for 10.6 32-bit and below)
* Use QLPreviewSetURLRepresentation() instead of reading the entire file (!) with -[stringWithContentsOfURL].
* Add extra compiler warning flags

Note that the inclusion of BSD licensed RegexKitLite will require a credit.
c1369b3
@nickhutchinson nickhutchinson Add support for thumbnail generation.
According to the (awful) documentation in the QuickLook header files, for this to work the QLNeedsToBeRunInMainThread Info.plist key has to be set to true.

Thumbnails are badged according to the following heuristic:
- If the file has an extension of < 10 characters, use that.
- If the file has a well-known MIME type (determined by file(1)), look up its corresponding badge string and use that.
- If the file has no extension, but a well-known filename (e.g. "Makefile"), look up its corresponding badge string and use that.
- Is the file executable? If so, assume it's a script, and badge it with "SCRIPT".
- Otherwise, badge it with "TEXT".

This heuristic could probably be improved. Currently the mappings of well-known filenames and MIME-types to badge strings are hard-coded in GenerateThumbnailForURL.m. Perhaps these should be moved into an external plist.
c6cf191
@whomwah
Owner

Thanks for your contribution. I'll try and take a look at the code as soon as I can.

I've also moved the project to ARC and Modern Objective-C

I don't see the need in restricting people unless the feature absolutely requires it.

@dmmalam

+1

@KovuTheHusky

This pull request has some really cool stuff. Are you planning on accepting it or rejecting it? I only ask because it was posted 10 months ago.

@whomwah
Owner

I've also moved the project to ARC and Modern Objective-C. This does loose you support for 10.6 32-bit, however.

When this is removed, then I'd be happy to merge the work. I don't want to restrict people unless it is absolutely necessary, as the plugin currently works just fine for the majority of people.

@nickhutchinson
@GhostLyrics

As far as I know 10.7 Lion and 10.8 Mountain Lion were 64-bit only anyway. 10.9 will be released this autumn. Apple itself only supports the current and last-but-one release at any time. I'd argue it's okay to merge if 10.6 with 32-bit was your main reason for concern.

@KovuTheHusky

I would say the same. I don't know anybody running a 32-bit Mac - or a 32-bit computer at all for that matter.

@whomwah
Owner

You've convinced me chaps ...

@whomwah whomwah merged commit c6cf191 into whomwah:master
@whomwah
Owner

.. and thanks for your patch Nicholas!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 2, 2012
  1. @nickhutchinson
  2. @nickhutchinson

    Make preview generation more robust; stop previewing binary files.

    nickhutchinson authored
    * Check candidate files with file(1) to decide whether they are text or binary, and to determine their encoding.
    * Upgrade to ARC (note this drops support for 10.6 32-bit and below)
    * Use QLPreviewSetURLRepresentation() instead of reading the entire file (!) with -[stringWithContentsOfURL].
    * Add extra compiler warning flags
    
    Note that the inclusion of BSD licensed RegexKitLite will require a credit.
  3. @nickhutchinson

    Add support for thumbnail generation.

    nickhutchinson authored
    According to the (awful) documentation in the QuickLook header files, for this to work the QLNeedsToBeRunInMainThread Info.plist key has to be set to true.
    
    Thumbnails are badged according to the following heuristic:
    - If the file has an extension of < 10 characters, use that.
    - If the file has a well-known MIME type (determined by file(1)), look up its corresponding badge string and use that.
    - If the file has no extension, but a well-known filename (e.g. "Makefile"), look up its corresponding badge string and use that.
    - Is the file executable? If so, assume it's a script, and badge it with "SCRIPT".
    - Otherwise, badge it with "TEXT".
    
    This heuristic could probably be improved. Currently the mappings of well-known filenames and MIME-types to badge strings are hard-coded in GenerateThumbnailForURL.m. Perhaps these should be moved into an external plist.
Something went wrong with that request. Please try again.