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

Add FileSystem.Watcher.FilterList Property #25967

Closed
Anipik opened this issue Apr 22, 2018 · 10 comments
Closed

Add FileSystem.Watcher.FilterList Property #25967

Anipik opened this issue Apr 22, 2018 · 10 comments
Labels
api-approved API was approved in API review, it can be implemented area-System.IO
Milestone

Comments

@Anipik
Copy link
Contributor

Anipik commented Apr 22, 2018

Rationale

Currently there is no support for using multiple regex filters to monitor the file system using FileSystemWatcher Class. eg. If we want to monitor files with two extensions (.dll and .pdb), we wont be able to directly to due it.

There are 2 ways to work around this issue but non of them is optimal.

  1. Create an instance of fileSystemWatcher class and listen to all the files using "*" filter and then prune the files manually in the event handler.
eg. If (filename.endswith("*.txt") || filename.endswith("*.csv")) 
  // do 

This approach is highly inefficient because first of all we are listening to all files and then user have to manually filter the files which defeats the purpose of having the Filter in the first place. Here are some links where people asked for this new functionality but has to settle down for this approach

https://stackoverflow.com/questions/6965184/how-to-set-filter-for-filesystemwatcher-for-multiple-file-types
https://social.msdn.microsoft.com/Forums/vstudio/en-US/91ebc868-2661-4477-b825-5b796d020ab1/how-do-i-setup-filewatcher-filter-for-multiple-document-types?forum=csharpgeneral
https://stackoverflow.com/questions/38467389/filter-to-not-include-multiple-extensions

  1. The Second approach is two create multiple instances of FileSystemWatcher with different regex filters and hook them to the same event handlers.

Proposed API

namespace System.IO
{
    public class FileSystemWatcher
    {
        public Collection<string> FilterList { get; set; }
    }
}

Currently we use string _filter as a backing field for Filter property. I suggest using Collection<string> _filterList as a backing field for both the Filter and FilterList Property.

Filter property will always return the first element of the list (In Case of multiple Filters too)

Implementation Branch

https://github.com/Anipik/corefx/tree/Filter
This is the rough implementation of the api and how it will modify the constructor and Filter property.

what happens when you set Filter?

It just sets the first element and does not reduce the size of collection.

What happens if you set FilterList to null or empty or clear it or removeall items using removeAt?

In all the above cases, we change it to the collection with just only one element i.e *

What do we do if an element in Filters is null or empty? If we match everything, can/should we optimize?

We never have a null or empty element in the collection. We always replace it with * while adding to the collection

If we match everything, can/should we optimize?

We can optimise by not going further through the collection of the list if we encounter *.

What about a constructor overload that takes the filters?

we can use Initializer to initialize the FilterList

var watcher3 = new FileSystemWatcher() { FilterList = new Collection<string> { "*.pdb", ".exe", ".doc" } };
Assert.Equal(3, watcher3.FilterList.Count);

It will be breaking change as FileSystemWatcher("path", null) will be ambiguous call for FileSystemWater(string path, IEnumnerable<string> FilterList)

Usage

var watcher = new FileSystemWatcher();
watcher.FilterList[0] = "*.pdb";
watcher.FilterList.Add("*.exe");
watcher.FilterList.Add("*.dll");
Assert.Equal(3, watcher.FilterList.Count);

var watcher2 = new FileSystemWatcher( @"C:\git\corefx","*.pdb");
watcher2.FilterList.Add("*.exe");
watcher2.FilterList.Add("*.dll");
Assert.Equal(3, watcher2.FilterList.Count);

var watcher3 = new FileSystemWatcher() { FilterList = new Collection<string> { "*.pdb", ".exe", ".doc" } };
Assert.Equal(3, watcher3.FilterList.Count);

var watcher4 = new FileSystemWatcher() { FilterList = null };
Assert.Equal(1, watcher4.FilterList.Count);

var watcher5 = new FileSystemWatcher() { FilterList = new Collection<string> { null } };
Assert.Equal("*", watcher5.Filter);

var watcher6 = new FileSystemWatcher();
watcher6.FilterList = new Collection<string> { "*.pdb", ".exe", ".doc" };
Assert.Equal(3, watcher6.FilterList.Count);

cc @danmosemsft @JeremyKuhne

@danmoseley
Copy link
Member

danmoseley commented Apr 22, 2018

What is the advantage of Collection<string> over List<string> here? It is just a simple wrapper for the latter.

@Anipik
Copy link
Contributor Author

Anipik commented Apr 23, 2018

@danmosemsft I am overriding InsertItem and SetItem to always convert "." to "*" while adding to FilterList which can`t be done with List

@danmoseley
Copy link
Member

Ah...I missed that on my small screen.

@JeremyKuhne
Copy link
Member

@Anipik

  1. Can you put the API in the namespace & class for context?
  2. Can you please give some sample usage code?
  3. Can you also describe what happens when you set Filter? Does it set the collection size to 1? Does it just set the first element?
  4. What happens if you set Filters to null or empty?
  5. What do we do if an element in Filters is null or empty? If we match everything, can/should we optimize?
  6. What about a constructor overload that takes the filters?

@Anipik
Copy link
Contributor Author

Anipik commented Apr 23, 2018

I will update the proposal soon

@Anipik
Copy link
Contributor Author

Anipik commented Apr 23, 2018

  1. Can you also describe what happens when you set Filter? Does it set the collection size to 1? Does it just set the first element?

It just sets the first element

  1. What happens if you set Filters to null or empty?

we can not set it null as it will be just ReadOnlyProperty. We can although empty the collection using clear() or RemoveAt. We can avoid that by overridding RemoveItem function in such a way that we never have empty collection.

5 a) What do we do if an element in Filters is null or empty? If we match everything, can/should we optimize?

We never have a null or empty element in the collection. We always replace it with * while adding to the collection

5 b) If we match everything, can/should we optimize?

We can optimise by not going further through the collection of the list if we encounter *.

  1. What about a constructor overload that takes the filters?

It will be breaking change as FileSystemWatcher("path", null) will no longer compile

I will add the usage code soon

@JeremyKuhne
Copy link
Member

JeremyKuhne commented Apr 23, 2018

Can you put the API in the namespace & class for context?

I meant:

namespace System.IO
{
    public class FileSystemWatcher
    {
        public Collection<string> FilterList { get; }
    }
}

It will be breaking change as FileSystemWatcher("path", null) will no longer compile

Why not add one that takes IEnumerable<string>?

Please add the other details you've given above under "Implementation Details".

@Anipik
Copy link
Contributor Author

Anipik commented Apr 24, 2018

@JeremyKuhne Done and added some some examples too

@danmoseley
Copy link
Member

We can discuss in review, but it seems least confusing to me for setting filter to be equivalent to setting a one item filter list, and for getting filter to throw if there is more than one entry in filterlist.

@terrajobst
Copy link
Member

terrajobst commented Apr 24, 2018

Video

Looks a good. A few points:

  • We should just use a plural form instead of the List suffix, e.g. Filters.
  • The property shouldn't be settable as this avoids problems when the watcher needs to use a derived type in the implementation.
  • The existing API Filter will return the first item in Filters. If set, it will clear Filters and add the one item.
  • Adding a constructor that takes path and filters would be a source breaking change if people call it with a null value for the second argument (because we have an existing one that takes string). However, we could add a longer version that would also accept, say, the matcher type. But that would be a separate API review.

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 3.0 milestone Jan 31, 2020
@dotnet dotnet locked as resolved and limited conversation to collaborators Dec 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.IO
Projects
None yet
Development

No branches or pull requests

5 participants