use "display name" as the label for files #1017

Merged
merged 6 commits into from Aug 21, 2012

Conversation

Projects
None yet
4 participants
Owner

skurfer commented Jul 24, 2012

On my (English) system, this correctly shows preference panes. For example, Security.prefPane appears as “Security & Privacy” and you can search on either the filesystem name or the display name.

I also looked at printers and Quicksilver plug-ins and they seem to display correctly.

use "display name" as the label for files, close #730
other clean up:
* don't test for things that are guaranteed to be `nil`
* don't use the same method to set the name and label, only to wipe out label because they match
* reuse the existing `manager` object
Owner

HenningJ commented Jul 25, 2012

hmm...all of this is weird. For some pref panes, this works (but could be improved). For some it doesn't. And some are just plain...weird...

  1. The "Language & Text" pref pane (or Localization.prefPane) doesn't seem to have a kMDItemDisplayName, so it can't be found by searching for "Language & Text". At least on my 10.6 system. When I check in the command line, i get:

    [HenningJ: ~]$ mdls -name kMDItemDisplayName /System/Library/PreferencePanes/Localization.prefPane
    kMDItemDisplayName = (null)
    

    Some of my user pref panes (e.g. for flash player) have the same problem.

  2. It would be nice to have the pref panes show up as (for example) "Security Preference Pane" or something, instead of just "Security". I think that's what the localizedPrefPaneKind method in that file is supposed to do. But that doesn't seem be called...for most of the pref panes

  3. The mouse pref pane shows up as " preference pane" (without mouse in the name), so localizedPrefPaneKind seems to be called, but the actual name can't be found for some reason.

  4. While testing this, I ran across another bug: If you have the catalog preferences open and then do a catalog rescan from the normal interface (using ⌘R), there is an error while refreshing the catalog preferences (I think), and all the catalog entries disappear from the view. They are all still there, but don't show up in the catalog preferences. The error that occurs:

2012-07-25 10:39:15.344 Quicksilver[2457:903] *** Assertion failure in -[QSOutlineView _expandItemEntry:expandChildren:startLevel:], /SourceCache/AppKit/AppKit-1038.36/TableView.subproj/NSOutlineView.m:969
An exception of type NSInternalInconsistencyException occured.
(null) should not be expanded already!
Stack trace:
     1  +[NSException raise:format:arguments:] (in CoreFoundation) + 103
     2  -[NSAssertionHandler handleFailureInMethod:object:file:lineNumber:description:] (in Foundation) + 198
     3  -[NSOutlineView _expandItemEntry:expandChildren:startLevel:] (in AppKit) + 674
     4  -[NSOutlineView numberOfRows] (in AppKit) + 198
     5  -[NSOutlineView frameOfCellAtColumn:row:] (in AppKit) + 78
     6  -[NSTableView drawRow:clipRect:] (in AppKit) + 891
     7  -[QSOutlineView drawRow:clipRect:] (in QSInterface) (QSOutlineView.m:38)
     8  -[NSTableView drawRowIndexes:clipRect:] (in AppKit) + 369
     9  -[NSOutlineView drawRowIndexes:clipRect:] (in AppKit) + 131
    10  -[NSTableView drawRect:] (in AppKit) + 1302
    11  -[NSView _drawRect:clip:] (in AppKit) + 3390
    12  -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] (in AppKit) + 1325
    13  -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] (in AppKit) + 2199
    14  -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] (in AppKit) + 2199
    15  -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] (in AppKit) + 767
    16  -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] (in AppKit) + 4555
    17  -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] (in AppKit) + 4555
    18  -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] (in AppKit) + 4555
    19  -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] (in AppKit) + 4555
    20  -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] (in AppKit) + 4555
    21  -[NSThemeFrame _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] (in AppKit) + 254
    22  -[NSView _displayRectIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:] (in AppKit) + 2683
    23  -[NSView displayIue:] (in AppKit) + 155
    33  -[NSApplication run] (in AppKit) + 395
    34  NSApplicationMain (in AppKit) + 364
    35  main (in Quicksilver) (main.m:21)
    36  start (in Quicksilver) + 52
    37  0x00000001 (in Quicksilver)
Owner

skurfer commented Jul 25, 2012

The "Language & Text" pref pane (or Localization.prefPane) doesn't seem to have a kMDItemDisplayName, so it can't be found by searching for "Language & Text".

OK, so that must be relatively new. Under 10.7:

% mdls -name kMDItemDisplayName /System/Library/PreferencePanes/Localization.prefPane
kMDItemDisplayName = "Language & Text"

Can you see any way to get the correct name? (Or should we not worry about something as minor as a display name that only affects 10.6?)

It would be nice to have the pref panes show up as (for example) "Security Preference Pane" or something, instead of just "Security".

Nice, yes, but only if we can do it in some general way. We don't want to have to maintain a number of special cases for different types.

The mouse pref pane shows up as " preference pane" (without mouse in the name), so localizedPrefPaneKind seems to be called, but the actual name can't be found for some reason.

That appears to come from descriptiveNameForPackage:withKindSuffix:, which is getting the bundle name using the path, instead of using the name or label determined by the getNameFromFiles method. We should probably try to collapse the functionality of these into one place.

While testing this, I ran across another bug: If you have the catalog preferences open and then do a catalog rescan from the normal interface (using ⌘R), there is an error while refreshing the catalog preferences (I think), and all the catalog entries disappear from the view. They are all still there, but don't show up in the catalog preferences.

I'm glad you ran accross that. It's not new. I've seen all my catalog entries disappear from the prefs, but I didn't know how to reproduce it. Based on what you've found, there was probably an automatic scan taking place in the background when I just happened to have the prefs open. Not related to this pull request, though. :-)

Just one thing I've noticed, not a cause of your changes, but perhaps something that could be fixed in this pull request:

The catalog preset in the 'User' pane called 'Documents' doesn't seem to get localised. This means that the name displayed in the catalog prefs is always 'Documents' (in English) as well as the folder name in QS when you search for 'docu…'

I think this is just because the catalog preset names are taken from the localised.strings files, and then the sources are "included in the global catalog".

Totally fine if you don't think it's worth altering this. I can still search for the Dutch names fine (what I'm testing in), even though they're not called properly

Owner

skurfer replied Jul 27, 2012

I think this is just because the catalog preset names are taken from the localised.strings files, and then the sources are "included in the global catalog".

Aren't the preset names supposed to come from those files? It looks like the preset is only defined in English and French, and in both cases, the string is “Documents”. Do we just need to add something for other languages?

Also, do you know what change they're referring to in #952? Is that something we should fix here as well?

Owner

skurfer commented Jul 27, 2012

The last commit should get everything set right for files. Specifically:

  • name gets set to the actual name in the filesystem
  • label is set to a nice string (localized if possible)

Since both are searchable, this will hopefully satisfy the competing interests of both Asia and Europe. :-)

This is not ready to merge yet. The matching process seems to always prefer matching on name, causing that to be displayed in place of the label. In my opinion, if the search matches both the name and label, the label should continue to be displayed. That is, “Security & Privacy” should not change to “Security.prefPane” when I type “sec”. And I don't want to see .app every time I locate an application in the catalog. So, I’d like to see if I can fix that somehow. (Pointers appreciated. I’ve never messed with the matching algorithm.)

UPDATE I think I was working from cached data, but I've noticed that descriptiveNameForPackage:withKindSuffix: tends to make most things (applications and pref panes) look worse, but some things (qsplugins) look better. To get see the benefits from the last commit, it needs to be commented out. :-)

change the order of things to ensure the best label is chosen
Check the bundle for info first. If it just returns the filesystem name, look for a nicer "display name".
Owner

skurfer commented Jul 27, 2012

OK, this is in pretty good shape.

It seems that most (all?) pref panes don't have a CFBundleDisplayName, so the descriptiveNameForPackage:withKindSuffix: returns before ever looking at the extension. Do we want to just strike that test from the method?

The issue with preferring names over labels when matching is still there, but I've tracked it down to the string rankers, so I'll address it separately. The TextStart ranker, which I normally use, always prefers the name. The default ranker does much better. I only saw it grab the name when the label also matched

Owner

pjrobertson commented Jul 28, 2012

Seems to work really well here. Tried with both Japanese and Nederlands systems. It fixes #952 and #730 in one :)

I see no problems with this. @HenningJ ?

Owner

skurfer commented Jul 28, 2012

Good. There's just the question of whether or not to remove the test for a prefPane extension. I didn't start debugging it until I was on 10.8. If you step through descriptiveNameForPackage:withKindSuffix:, does it ever reach that line for a preference pane? I say we remove it.

Owner

pjrobertson commented Jul 28, 2012

Turns out this isn't an issue new to this pull request, but one I've only just come across:

If you use QS to rename a file (Rename… action), the label is not updated in the UI.

E.g.

abc.jpg > rename… > 123.jpg
reinvoke QS
Still shows abc.jpg, but the path shows correctly […]/123.jpg

Owner

skurfer commented Jul 30, 2012

If you use QS to rename a file (Rename… action), the label is not updated in the UI.

I see the same. Not sure why. If I step through fileObjectWithPath:, the name gets set correctly (and there is no label) and the correct label appears in the UI. Maybe it’s only a problem when it happens quickly? Still can’t explain it though.

I’ve also noticed that details doesn’t always appear right away or is incorrect. For instance, Downloads always has /Users/rob/Downloads when it matches and shows up, but if I hit ↓ then ↑ it immediately changes to ~/Downloads. Similar for some actions, like “Quit”. There are no details at first, but ↓ ↑ makes them appear.

remove a useless condition for preference panes
If the file is a preference pane, the method will return before this code is reached. And even if it does get run, on newer versions of OS X, the system will provide a suitable string for `kind`.
Owner

pjrobertson commented Aug 10, 2012

@HenningJ I think this is just waiting for your approval - that things work correctly for someone who actually uses QS in another language ;-)

I've tried and tested in various languages and it's all fine for me.

Owner

skurfer commented Aug 20, 2012

Follow-up continued from a discussion on #952:

I convered unicode string to utf-8 string by Python for checking what unicode string means.

Who doesn't love Python, right? :-)

QOSObjectLabel didn't show in log.

I think I know what's going on. First, the label is only set if it differs from name. It's possible they were the same in this case, so I'm not too worried about that. (Though I do wonder why the label would be 計算機.app and not just 計算機, as it is on an English system.)

As for why the name is "wrong", we're not actually looking at the filesystem to get the name in getNameFromFiles. We're taking the last path component (just the file name) from whatever string is passed in. My guess is that this string is the localized version of the path before we see it. So we need to either prevent such localization if possible, or somehow override it here and force it back to the name in the filesystem. I'll look into it.

correctly set the name to the filesystem name
Blindly calling `lastPathComponent` doesn't account for paths that were localized prior to being passed in.
Owner

skurfer commented Aug 20, 2012

OK, @heavenshell I've added another commit. Pull the issie730 branch again (and clear caches) and see if it makes a difference.

@skurfer

Awsome! It's works!

I typed Calculator and hit 計算機.app

"計算機" means calculator in Japanese.

2012/08/21 1:43:11.562 Quicksilver[3602]: Dictionary
{
    class = QSObject;
    data =     {
        NSFilenamesPboardType = "/Applications/Calculator.app";
    };
    properties =     {
        QSObjectDetails = "/Applications/Calculator.app";
        QSObjectLabel = "\U8a08\U7b97\U6a5f";
        QSObjectName = "Calculator.app";
        QSObjectObjectID = "/Applications/Calculator.app";
        QSObjectType = NSFilenamesPboardType;
    };
}

Thank you for your great work!!

Regards,

Owner

skurfer commented Aug 20, 2012

Great. Thanks for helping identify this problem before we released the changes.

Owner

skurfer commented Aug 21, 2012

Merging with @pjrobertson's blessing.

skurfer added a commit that referenced this pull request Aug 21, 2012

Merge pull request #1017 from skurfer/issue730
use "display name" as the label for files

@skurfer skurfer merged commit 121d05c into quicksilver:master Aug 21, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment