-
-
Notifications
You must be signed in to change notification settings - Fork 115
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,13 +42,42 @@ private struct QuickLink | |
#endregion | ||
|
||
#region Inner Classes | ||
public class Filter | ||
|
||
public interface IFilter { | ||
/// <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 ) | ||
{ | ||
|
@@ -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 ) ) | ||
|
@@ -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 ) | ||
|
@@ -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; | ||
|
||
|
@@ -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 ) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can change it as follows and remove the isAllFilesFilter property: |
||
{ | ||
// In file selection mode, make sure that nonexisting files' extensions match one of the required extensions | ||
string fileExtension = GetExtensionFromFilename( filename, AllExtensionsHaveSingleSuffix ); | ||
|
@@ -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 ); | ||
|
||
|
@@ -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 ); | ||
|
@@ -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 ) ) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can change it as follows and remove the isValidExtension property: |
||
{ | ||
Instance.filtersDropdown.value = i; | ||
Instance.filtersDropdown.RefreshShownValue(); | ||
|
There was a problem hiding this comment.
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).