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

Make file filters extensible by turning them into an interface, instead of concrete class. #64

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 52 additions & 11 deletions Plugins/SimpleFileBrowser/Scripts/FileBrowser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,42 @@ private struct QuickLink
#endregion

#region Inner Classes
public class Filter

public interface IFilter {
Comment on lines 44 to +46
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please get rid of the free space and put the curly bracket on a new line for consistency (I'm asking this since this is a small commit).

/// <summary>Default extension for this filter.</summary>
string defaultExtension { get; }

/// <summary>'false' when some extensions have multiple suffixes like ".tar.gz"</summary>
bool allExtensionsHaveSingleSuffix { get; }

/// <summary>
/// Returns true if this filter is the default 'all files' filter. All custom filters should return false here.
/// </summary>
bool isAllFilesFilter { get; }

/// <summary>Returns true if this is one of the supported extensions for this filter.</summary>
bool isValidExtension( string extension );

/// <summary>
/// Returns true if the filter matches the given <see cref="extension"/>.
/// </summary>
/// <param name="extension">a file extension, like '.tar' or '.tar.gz'</param>
/// <param name="extensionMayHaveMultipleSuffixes">
/// true if <see cref="extension"/> may have multiple suffixes, like '.tar.gz'
/// </param>
bool MatchesExtension( string extension, bool extensionMayHaveMultipleSuffixes );

/// <summary>Shown in the user interface.</summary>
string ToString();
}

public class Filter : IFilter
{
public readonly string name;
public readonly string[] extensions;
public readonly HashSet<string> extensionsSet;
public readonly string defaultExtension;
public readonly bool allExtensionsHaveSingleSuffix; // 'false' when some extensions have multiple suffixes like ".tar.gz"
public string defaultExtension { get; private set; }
public bool allExtensionsHaveSingleSuffix { get; private set; }

internal Filter( string name )
{
Expand Down Expand Up @@ -92,6 +121,19 @@ public Filter( string name, params string[] extensions )
defaultExtension = extensions[0];
}

public bool isAllFilesFilter
{
get
{
return extensions == null;
}
}

public bool isValidExtension( string extension )
{
return extensionsSet != null && extensionsSet.Contains( extension );
}

public bool MatchesExtension( string extension, bool extensionMayHaveMultipleSuffixes )
{
if( extensionsSet == null || extensionsSet.Contains( extension ) )
Expand Down Expand Up @@ -271,7 +313,7 @@ public static string AllFilesFilterText

if( m_instance )
{
Filter oldAllFilesFilter = m_instance.allFilesFilter;
IFilter oldAllFilesFilter = m_instance.allFilesFilter;
m_instance.allFilesFilter = new Filter( value );

if( m_instance.filters.Count > 0 && m_instance.filters[0] == oldAllFilesFilter )
Expand Down Expand Up @@ -516,8 +558,8 @@ private static FileBrowser Instance
#pragma warning restore 0414
private StringBuilder multiSelectionFilenameBuilder;

private readonly List<Filter> filters = new List<Filter>();
private Filter allFilesFilter;
private readonly List<IFilter> filters = new List<IFilter>();
private IFilter allFilesFilter;

private bool showAllFilesFilter = true;

Expand Down Expand Up @@ -1470,7 +1512,7 @@ public void OnSubmitButtonClicked()
{
// This is a nonexisting file
string filename = filenameInput.Substring( startIndex, filenameLength );
if( m_pickerMode != PickMode.Folders && filters[filtersDropdown.value].extensions != null )
if( m_pickerMode != PickMode.Folders && !filters[filtersDropdown.value].isAllFilesFilter )
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can change it as follows and remove the isAllFilesFilter property: m_pickerMode != PickMode.Folders && filters[filtersDropdown.value] != allFilesFilter

{
// In file selection mode, make sure that nonexisting files' extensions match one of the required extensions
string fileExtension = GetExtensionFromFilename( filename, AllExtensionsHaveSingleSuffix );
Expand Down Expand Up @@ -2840,7 +2882,7 @@ public static void SetFilters( bool showAllFilesFilter, IEnumerable<Filter> filt
SetFiltersPostProcessing();
}

public static void SetFilters( bool showAllFilesFilter, params Filter[] filters )
public static void SetFilters( bool showAllFilesFilter, params IFilter[] filters )
{
SetFiltersPreProcessing( showAllFilesFilter );

Expand Down Expand Up @@ -2868,7 +2910,7 @@ private static void SetFiltersPreProcessing( bool showAllFilesFilter )

private static void SetFiltersPostProcessing()
{
List<Filter> filters = Instance.filters;
List<IFilter> filters = Instance.filters;

if( filters.Count == 0 )
filters.Add( Instance.allFilesFilter );
Expand Down Expand Up @@ -2915,8 +2957,7 @@ public static bool SetDefaultFilter( string defaultFilter )

for( int i = 0; i < Instance.filters.Count; i++ )
{
HashSet<string> extensions = Instance.filters[i].extensionsSet;
if( extensions != null && extensions.Contains( defaultFilter ) )
if( Instance.filters[i].isValidExtension( defaultFilter ) )
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can change it as follows and remove the isValidExtension property: Instance.filters[i].MatchesExtension( defaultFilter, defaultFilter.LastIndexOf( '.' ) != 0 ). Even better, the value of defaultFilter.LastIndexOf( '.' ) != 0 can be cached in a bool defaultFilterHasMultipleSuffixes variable before the for-loop.

{
Instance.filtersDropdown.value = i;
Instance.filtersDropdown.RefreshShownValue();
Expand Down