Page MenuHome

UI: Mac File Browser System List Changes
ClosedPublic

Authored by Yevgeny Makarov (jenkm) on Dec 11 2019, 6:30 PM.

Details

Summary

This patch adds a new FS_INTERNAL_PATH_CACHE FSMenuCategory for internal usage.
So we can store in it some paths for matching paths to icons and names.

This is used on macOS to add icons for System Bookmarks.
For all platforms, we can add the appropriate icons to the directories from User Preferences when they are added to the Bookmarks.

Diff Detail

Repository
rB Blender

Event Timeline

William Reynish (billreynish) requested changes to this revision.Dec 12 2019, 3:36 AM

Thanks for looking at this.

The good news:

  • It compiles! :)
  • The icons are showing!

Bad news:

  • The System source list is broken now. It doesn't get the correct folders from the Finder Sidebar.
  • Clicking on the directories in the System category doesn't work
  • It only shows icons in the System panel. The same folders in Recents and Bookmarks panels don't have icons.

It seems to me that this patch is trying to change too many things. Really we should not have to change the way we get the folders from the Finder Sidebar (which was working fine) but simply add the correct icons to the correct paths.

This revision now requires changes to proceed.Dec 12 2019, 3:36 AM

Abandoning this patch. Had made assumptions based on a report from someone else that is incorrect.

Just a little mistake here, you need replace "~" with the user home dir BLI_getenv("HOME").

@Yevgeny Makarov (jenkm) Maybe it makes sense if you can take over this revision fully? Since @Harley Acheson (harley) doesn't have a Mac to build and test on, it may be better if you can commander this?

@Yevgeny Makarov (jenkm) Maybe it makes sense if you can take over this revision fully? Since @Harley Acheson (harley) doesn't have a Mac to build and test on, it may be better if you can commander this?

That would be awesome! I don’t have a Mac to compile or test with so this was done completely blind.

Whoops! No, please don’t use this patch.

This particular one was abandoned because it was explicitly adding entries to the list, when I was under the assumption that there was only a small number of predictable items.

We have since decided to use the list of system-supplied special folders and assign icons based on substrings (these path names are not localized). I’m not sure my latest Mac-specific patch is on the tracker right now. Was shared with William for testing and with Campbell. I’ll sort these out and try to get all platforms in shortly after I get back in about eight days. There is some shared code and then small changes per each platform.

The shared things, along with the Windows-specific stuff is here: https://developer.blender.org/D6405 and would be first step.

I tried to add a new FSMenuCategory for storing known paths and modified fsmenu_insert_entry so it would always look for icons/names in this "known paths" category. It seems this way works well.

I don't know how to do it correctly, so I don't share the code, but here is the code with Mac paths:

const char *home = BLI_getenv("HOME");
char line[FILE_MAXDIR];

BLI_snprintf(line, sizeof(line), "%s/", home);
fsmenu_insert_entry(fsmenu, FS_CATEGORY_KNOWN_PATHS, line, NULL, ICON_HOME, FS_INSERT_LAST);

BLI_snprintf(line, sizeof(line), "%s/Desktop/", home);
fsmenu_insert_entry(fsmenu, FS_CATEGORY_KNOWN_PATHS, line, IFACE_("Desktop"), ICON_DESKTOP, FS_INSERT_LAST);

BLI_snprintf(line, sizeof(line), "%s/Documents/", home);
fsmenu_insert_entry(fsmenu, FS_CATEGORY_KNOWN_PATHS, line, IFACE_("Documents"), ICON_DOCUMENTS, FS_INSERT_LAST);

BLI_snprintf(line, sizeof(line), "%s/Downloads/", home);
fsmenu_insert_entry(fsmenu, FS_CATEGORY_KNOWN_PATHS, line, IFACE_("Downloads"), ICON_IMPORT, FS_INSERT_LAST);

BLI_snprintf(line, sizeof(line), "%s/Movies/", home);
fsmenu_insert_entry(fsmenu, FS_CATEGORY_KNOWN_PATHS, line, IFACE_("Movies"), ICON_FILE_MOVIE, FS_INSERT_LAST);

BLI_snprintf(line, sizeof(line), "%s/Music/", home);
fsmenu_insert_entry(fsmenu, FS_CATEGORY_KNOWN_PATHS, line, IFACE_("Music"), ICON_FILE_SOUND, FS_INSERT_LAST);

BLI_snprintf(line, sizeof(line), "%s/Pictures/", home);
fsmenu_insert_entry(fsmenu, FS_CATEGORY_KNOWN_PATHS, line, IFACE_("Pictures"), ICON_FILE_IMAGE, FS_INSERT_LAST);

BLI_snprintf(line, sizeof(line), "%s/Library/Fonts/", home);
fsmenu_insert_entry(fsmenu, FS_CATEGORY_KNOWN_PATHS, line, IFACE_("Fonts"), ICON_FILE_FONT, FS_INSERT_LAST);

fsmenu_insert_entry(fsmenu, FS_CATEGORY_KNOWN_PATHS, "/Library/Fonts/", IFACE_("Fonts"), ICON_FILE_FONT, FS_INSERT_LAST);

fsmenu_insert_entry(fsmenu, FS_CATEGORY_KNOWN_PATHS, "/Applications/", IFACE_("Applications"), ICON_WINDOW, FS_INSERT_LAST);

fsmenu_insert_entry(fsmenu, FS_CATEGORY_KNOWN_PATHS, "/", NULL, ICON_DISK_DRIVE, FS_INSERT_LAST);

@Harley Acheson (harley) just an explanation for the previous post.

In the Windows version of this patch you get icons/names from the System list, which are hardcoded for Windows and Linux.
But on Mac this list (taken from the Finder) is user-defined, so not all known system folders can be present in it.
So my idea is to have a separate list with all known system folders and take icons from it.

Yevgeny Makarov (jenkm) edited the summary of this revision. (Show Details)

@Yevgeny Makarov (jenkm) - With the following keep in mind that that I don't have a Mac, can't compile for Mac, and that my thoughts of the matter are far from what could be best. So this is just my thoughts on where I was going with all this stuff.

Following is a patch that should do similar things for the "System" list for Windows, Mac, and Linux:

I am trying to keep the lists as similar as possible between them while keeping platform differences. You will see that there is a couple common areas and then per-platform changes.

For Windows I am just asking the operating system for each desired special folder location. On Windows we don't have a way of getting a list of all of these, but this mechanism does give up paths that are always correct. even if changed by the user.

For Mac I am using the kLSSharedFileListFavoriteItems as supplied but just selecting icons based on substrings of the paths. I am using this list rather than constructing my own since this list can vary greatly between users. The patch strings are safe to parse on because they are always these even in non-English installations (and localized later for display in Finder)

For Linux I am just constructing a list manually as there isn't a common way to get a list. However each one needs to be checked for existence because these CAN change with a different language.

For all platforms there is two common changes. You'll see that when items are added to the Bookmarks section those items will use icon from System if the paths match. And a similar thing is done when showing large icons in the main file list: overlaying the icon used in the System list for those.

Again, what is done here is not necessarily better or more correct or anything, just what I was moving toward. The only patch I have currently on the tracker is for Windows-only (with those common changes) just to make the approval process easier.

@Harley Acheson (harley) can you check the current version of this patch on Windows? It's based on your Windows patch, and the code for Windows is untouched, so it should work.

There are some common changes here. I added a new category FS_CATEGORY_KNOWN_PATHS for storing icons/names of known folders. If you look at the screenshot above in the summary, you will see that the System list contains only four folders, (this is the default in Finder) but the folders in the Favorites and Recent have the proper icons. These icons are taken from the FS_CATEGORY_KNOWN_PATHS list.

This FS_CATEGORY_KNOWN_PATHS list also stores icons for folders from Preferences such as U.fontdir U.textudir, so if the user adds them to Bookmarks they will have an icon. Well, at least that's the idea.

@Yevgeny Makarov (jenkm) - can you check the current version of this patch on Windows? It's based on your Windows patch, and the code for Windows is untouched, so it should work.

I sure can. In just a bit...

I'm unclear (so far) on your other changes though. We have some items that are static and supplied by the system or created by us, the Volumes List and System List. Bookmarks is something that is maintained automatically but can be cleared by the user. And then "Favorites" contains user-defined shortcuts and can be added to, removed at will, etc.

So I would guess that all known folder locations should go in the System list. Again, not saying your approach is wrong, just missing something that you are seeing. Hell, if it were up to me I would combine "Volumes" and "System" into a single list and also lose the title above it. So one list that would contain drives, then special folders.

I'm also hoping that whatever arrangement of these things will also be as similar as possible on other platforms.

Looking at your patch you seem to be attempting to add a fifth list, but your capture only shows four, which is why I am confused on your intent.

So I would guess that all known folder locations should go in the System list.

These all "known folders" are not automatically added to any visible list, but if a user manually adds one of these "known paths" to Bookmarks, they will have the proper icon.

@Yevgeny Makarov (jenkm) - These "all known folder" are not added to any (visible) list automatically, but if the user adds them manually they will have an icon.

Oh, I see you are adding the list of known folders one by one manually. This was something I also started with, but then found that users can add any folder to their own "special folders" they see in the Finder. So some users might have a short list while others have piles of things on it.

Did you see what I did with those folders in my patch referenced above? There I use the list as of special folders from the OS, and just assign icons based on substrings. That way you always get a list that matches those in Finder.

On William's machine he got the following, for example:

If you are wanting to add more items to System than is supplied by default you can always use a hybrid approach. Add them all from the kLSSharedFileListFavoriteItems list as I did in my patch and then add more Mac-specific paths manually as you do afterward. AFAIK those list will ignore duplicates.

The point is, if I have, for example, the Image folder (known system folder) in Bookmarks list, but it's not in the System list (since on Mac, the System list is taken from Finder sidebar and can be any), it won't have a proper icon. In your approach, when you take icons from the System list. That is, I don't want to add more items, I just store matching paths and icons in case the user adds them manually.

@Yevgeny Makarov (jenkm) - The point is, if I have, for example, the Image folder (known system folder) in Bookmarks list, but it's not in the System list (since on Mac, the System list is taken from Finder sidebar and can be any), it won't have a proper icon.

But that is why I would add all the items from that system-supplied kLSSharedFileListFavoriteItems list first and then add any other items that you think should be there, including that Image folder. With the code above if the user also has that same Image folder in Favorites as well it would get the same icon. If users did not like having the same item in both lists they can easily remove one from Favorites.

This would seem to be the best of both worlds. For someone like William, who has added many new favorite places to his Finder list, he gets to see all the same items in the list. But it would also contain other special folders that you consider important.

I would add all the items from that system-supplied kLSSharedFileListFavoriteItems list first and then add any other items that you think should be there.

No, I don't want to add extra items, on Mac, the System list reproduces the Finder sidebar, as much as we can, as @William Reynish (billreynish) wishes.
Also note that I have prepared ten known system folder and also five folder from Preferences, so this will make a very long list.

In fact, I haven't changed the way folders are added to lists at all, I only reassign icons by taking them from the hidden/internal list, using the code we already have.

I guess the main reason I remain confused is that your patch indicates the addition of that fifth list (FS_CATEGORY_KNOWN_PATHS) but your capture shows only four. Can you add a capture showing all in use?

The FS_CATEGORY_KNOWN_PATHS is internal/hidden, it is not visible in the interface, it only stores all known paths :)

Ah! Well that is a great idea. LOL.

Now this was just me being brain-dead and not noticing your intent with that. But what would have helped would be increased patch context, which is more lines above and below the changes. If using command line “git” to make the patch that would mean adding “-U100” as in “git diff -U100 > whatever.diff”.

Okay, just made more work for you. LOL.

I just committed my changes for Windows, which obviously contains some changes that are common with yours. So when you next update your patch won't apply and you'll have to rebase and simplify it. I can give you a hand if needed.

But in your simpler patch you should consider renaming that FS_CATEGORY_KNOWN_PATHS identifier. For your use of it that name makes perfect sense. But it would be a bit confusing when seen from other platforms. For Windows or Linux I might want to put some other lesser-used paths in there, but there would be no need to put all of them in there as you are doing with Apple. It should probably have a name that implies that it is used as a cache of paths and that it is (unlike its peers) not displayed directly.

When adding your list of platform-agnostic paths (like U.fontdir), I would consider using explicit (translatable) names for those. Otherwise they will only show the last path part and that might not always make sense, depending on where they are.

When adding your list of platform-agnostic paths (like U.fontdir), I would consider using explicit (translatable) names for those. Otherwise they will only show the last path part and that might not always make sense, depending on where they are.

@Harley Acheson (harley) I think on the contrary, these will be user-defined folder names, maybe even different depending on the current project, so there is no need to change them to some common ones.

@William Reynish (billreynish) This can be reviewed now. Maybe some changes to the path list, names and icons used.

A much better approach than my Mac version - avoids all the ugly substring searching I was doing, and like the use of that path_cache.

@Yevgeny Makarov (jenkm) - ...maybe even different depending on the current project

yup, good point.

Just used #define in this version.

Tested - this seems to work correctly now.

@Brecht Van Lommel (brecht) up to you to check if the code is ok.

This revision is now accepted and ready to land.Jan 29 2020, 12:13 AM
Brecht Van Lommel (brecht) requested changes to this revision.Mar 4 2020, 9:55 AM
Brecht Van Lommel (brecht) added inline comments.
source/blender/editors/include/ED_fileselect.h
171

See review comments from D7014.

source/blender/editors/space_file/fsmenu.c
604

The window icon to me does not seem helpful in indicating the applications folder.

I just wouldn't add any icon for this, why would you even favorite the applications folder in Blender?

This revision now requires changes to proceed.Mar 4 2020, 9:55 AM

I just wouldn't add any icon for this, why would you even favorite the applications folder in Blender?

It's present in the System list, which is simply copied from Finder. I suggested exclude this folder.

@William Reynish (billreynish) maybe the "Applications" folder should be excluded from the System list as well?

@Yevgeny Makarov (jenkm) Why should it be excluded? I mean, sure, it doesn't seem terribly useful, but IMO we should simply try and reproduce the Finder sidebar as much as we can.

Ultimately it is up to you Mac users, but... I am hoping you would at least consider changing...

FS_MACOS_PATH("%s/", NULL, ICON_HOME) to FS_MACOS_PATH("%s/", IFACE_("Home"), ICON_HOME)

Would be okay to me to keep as you have it, but changing it would make it more consistent with what we are showing in the same list for Windows and Linux. So instructions and tutorials would be more similar. And that word can be translated nicely. Following is what that list looks like on Windows in Chinese:

This is how it appears in the Finder, we're just try to reproduce it, and I think it's more familiar/understandable for the user.

But basically I don't care. The "Home" is also fine.

@Yevgeny Makarov (jenkm) - But basically I don't care. The "Home" is also fine.

No, doesn't matter a lot to me really, I just (softly) prefer to keep them similar if possible.

In your Mac Finder list, is that "jenkm" on your capture always the directory name or is it your user name? Or are they always the same, even case? So if you change your name slightly does the directory name change or is that not allowed, etc?

Just curious since leaving that NULL would always show the directory path, not username if ever different. So if username and you really want that we have that somewhere too.

Actually we got a tie breaker. William would like it as you have it, so showing folder name.

Looks good to me.

As a Mac user yeah the folder name for Home is always the short user name so much rather see that than 'Home'

This revision is now accepted and ready to land.Mar 5 2020, 6:55 PM
This revision was automatically updated to reflect the committed changes.