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

ß71: Don't set the label as an empty string #1257

Merged
merged 1 commit into from Nov 29, 2012

Conversation

pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Nov 29, 2012

I think when the OS doesn't have a localisation for a file (such as when the language is set to Welsh), mdls gives the kMDItemDisplayName as an empty string. Check for this before blindly using it as the label

This was only really causing problems for .prefPane files, but I think there were a few others. It means that an empty label @"" was being set and nothing was showing in the interface

http://cl.ly/image/0g3N1e3z4719

Here's what mdls gives for the Localization.prefPane file:

PaddyMBP:PreferencePanes patrick$ mdls Localization.prefPane
kMDItemAlternateNames          = (
    "Localization.prefPane"
)
kMDItemCFBundleIdentifier      = "com.apple.Localization"
kMDItemContentCreationDate     = 2012-06-20 22:08:38 +0000
kMDItemContentModificationDate = 2012-06-20 22:08:38 +0000
kMDItemContentType             = "com.apple.systempreference.prefpane"
kMDItemContentTypeTree         = (
    "com.apple.systempreference.prefpane",
    "com.apple.package",
    "public.directory",
    "public.item",
    "com.apple.bundle"
)
kMDItemDateAdded               = 2012-07-22 02:21:55 +0000
kMDItemDisplayName             = ""

Note kMDItemDisplayName = ""

I think when the OS doesn't have a localisation for a file (such as when the language is set to Welsh), `mdls` gives the `kMDItemDisplayName` as an empty string. Check for this before blindly using it as the label
@skurfer
Copy link
Member

@skurfer skurfer commented Nov 29, 2012

You think that's fun? Try mdls -name kMDItemDisplayName /usr/bin/2to3. (There are 6 files in /usr/bin that are all hard links and share the same kMDItemDisplayName as a result. The last one, alphabetically, seems to be the one that determines the eventual display name.)

So I was actually planning to revisit getNameFromFiles. Don't suppose you want to address it in this pull? :-)

I now question my decision to use kMDItemDisplayName first. [[NSFileManager defaultManager] displayNameAtPath:@"/usr/bin/2to3"] shows what you'd expect for that and most other things. As far as I can tell, preference panes are the only things that benefit from using the Spotlight attribute. I hate to add special cases, but maybe we should here.

It also seems that getting the MDItemRef impacts performance quite a bit, but I'm not sure how we can avoid that because we're also using the Spotlight data to get kMDItemFSName. I couldn't see anything in NSFileManager that would provide the same information.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Nov 29, 2012

Didn't we use kMDItemDisplayName becuase it works for localizations, whereas displayNameAtPath: doesn't?
The other thing I don't like about using Spotlight data is it means that people have to have spotlight enabled to use these things in QS. Not the end of the world, but some people like to have it disabled.

Anyway, that's a strange one you've found there., and something I don't think I'll address in this pull. I don't want to hold up the next pre-release, do I? ;-)

@skurfer
Copy link
Member

@skurfer skurfer commented Nov 29, 2012

Didn't we use kMDItemDisplayName becuase it works for localizations, whereas displayNameAtPath: doesn't?

No, I used it because it worked better for preference panes and it took care of the localization. From the documentation for displayNameAtPath::

Display names are user-friendly names for files. They are typically used to localize standard file and directory names according to the user’s language settings.

Couldn't make it your problem, eh? Well, it was worth a try. :-)

skurfer added a commit that referenced this issue Nov 29, 2012
ß71: Don't set the label as an empty string
@skurfer skurfer merged commit da9591c into quicksilver:release Nov 29, 2012
skurfer added a commit that referenced this issue Nov 29, 2012
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