Page MenuHome

Fix T95257: Filter files on "name" and "relpath"
ClosedPublic

Authored by Harley Acheson (harley) on Jan 28 2022, 1:26 AM.

Details

Summary

When filtering File Browser items by name, use entry's "name" field as
well as the "relpath" field since they can vary.


When viewing file lists in File Browser you can filter the results by entering text in the box near the top-right. This filter input box says that it is "Name Filter" and that you can "filter by name". However this does not filter by "Name" but by "Filename", which is the field called "relpath". In most cases this is the same as the "name" field but the "name" field is meant to hold the display name, especially for times when they vary. This patch makes it so that both fields are checked.

Bug report T95257: Blender File View search behaves unpredictably when searching for fonts shows this well as it illustrates entering text that should filter the items, but it is doing so by filename rather than the more informative "name", which is this case is the more informative font family name and style.

Diff Detail

Repository
rB Blender

Event Timeline

I'd like Bastien to make the decision here. It's not clear to me why this was using the relative path, there may have been reasons for that.

Bastien Montagne (mont29) requested changes to this revision.Mar 15 2022, 10:04 AM

So... First of all, initial usage of FileListInternEntry.name was purely an 'internal' cache of filename itself (to help with sorting, in recursive/multi-directories case).

This was changed in rB8aa1c0a326a8: Fix T75028: Improved Font Names in File Manager, unfortunately without a proper update of the comment for that field in the FileListInternEntry struct. I think the name itself should also be changed, maybe to something like FileListInternEntry.ui_name ?


For this patch itself, I do not think that not filtering on relpath at all is a valid idea. I would rather keep current code, and add another filter function (is_filtered_file_ui_name) to also check on UI name (probably OR'ed with the result of is_filtered_file_relpath).

This revision now requires changes to proceed.Mar 15 2022, 10:04 AM

@Bastien Montagne (mont29) - For this patch itself, I do not think that not filtering on relpath at all is a valid idea. I would rather keep current code, and add another filter function (is_filtered_file_ui_name) to also check on UI name (probably OR'ed with the result of is_filtered_file_relpath).

Yes, that works much better, and is simpler too. Thanks!

...initial usage of FileListInternEntry.name was purely an 'internal' cache of filename itself (to help with sorting, in recursive/multi-directories case)...This was changed in rB8aa1c0a326a8: Fix T75028: Improved Font Names in File Manager...

I might have been the first to use that feature, but that had been part of the design for a while (and why I was told to use it). Its the reason why FileDirEntry will only free its name buffer if FILE_ENTRY_NAME_FREE, and why FileListInternEntry has a free_name bool for the same purpose. Agree that the comments need updating.

I think the name itself should also be changed, maybe to something like FileListInternEntry.ui_name

Can I do this in a different patch and fix those comments at the same time?

Harley Acheson (harley) retitled this revision from Fix T95257: Filter files on "name" not "relpath" to Fix T95257: Filter files on "name" and "relpath".Mar 17 2022, 8:10 PM
Harley Acheson (harley) edited the summary of this revision. (Show Details)

LGTM.

Can I do this in a different patch and fix those comments at the same time?

Yes, sounds good. You'll also have to update the name of the new is_filtered_file_name function.

This revision is now accepted and ready to land.Mar 18 2022, 9:29 AM