FileSystemEnumerator.ContinueOnError() needs overload with context

[JeremyKuhne]

I didn't provide context in the 2.1 release of the APIs as it is somewhat tricky to do without allocating.

Presumably some API like ContinueOnError(ReadOnlySpan<char> directory, ReadOnlySpan<char> filename, bool directoryOpen) that gets called if ContinueOnError() returns false and has a default return value of false.

I'm marking this up-for-grabs if anyone wants to tackle it before I find time to.

See #26704.

[CyberSinh]

Can we expect a such API in the .NET Core 3.0?

Why this API like ContinueOnError(Span<char> directory, Span<char> filename, bool directoryOpen) would be called only if ContinueOnError() returns false?

Thank you @JeremyKuhne.

[JeremyKuhne]

Can we expect a such API in the .NET Core 3.0?

Possibly. It depends on feedback and available time to allocate internal resources. If someone external wants to take this on it is more likely.

Why this API like ContinueOnError(Span directory, Span filename, bool directoryOpen) would be called only if ContinueOnError() returns false?

Because we want to be compatible with existing code. We can't call the new method, get true, then not call the old method. We can, however, call the old method, get true, then not call the new method as it would be documented behavior. This would be important if you're deriving from an enumerator that derived from the base only understanding the current behavior.

[nil4]

Should the directory and filename parameters be ReadOnlySpan<char>, or is it expected that the calee would mutate them?

[JeremyKuhne]

Should the directory and filename parameters be ReadOnlySpan, or is it expected that the calee would mutate them?

They should be. Bad habit on my part- I'll update the comment.

[danmoseley]

To clarify the top post -- writing up the API proposal is up for grabs. Until that is done and approved, there is no code to write

[CyberSinh]

Hi, do you have a plan to address this issue in the near future? Thanks.

[danmoseley]

@CyberSinh do you want to write the API proposal as suggested above? that is the next step.

[CyberSinh]

@danmosemsft, thank you for your trust but I think @JeremyKuhne is more qualified than I am to do it. That's why I wanted to know if Microsoft had this feature on its roadmap. Thanks again.

[danmoseley]

@CyberSinh that's fine. To answer your question, we don't have a plan to do this in the near future.

[iSazonov]

If ContinueOnError() returns false we throw on the error.
If ContinueOnError() returns true we would want to report the error.

        private bool InternalContinueOnError(Interop.ErrorInfo info, bool ignoreNotFound = false)
            => (ignoreNotFound && IsDirectoryNotFound(info)) || (_options.IgnoreInaccessible && IsAccessError(info))
               || (ContinueOnError(info.RawErrno) && ReportError(...);

[iSazonov]

Because we want to be compatible with existing code. We can't call the new method, get true, then not call the old method. We can, however, call the old method, get true, then not call the new method as it would be documented behavior. This would be important if you're deriving from an enumerator that derived from the base only understanding the current behavior.

We can:

        private bool InternalContinueOnError(string currentPath, Interop.ErrorInfo info, bool ignoreNotFound = false)
            => (ignoreNotFound && IsDirectoryNotFound(info)) || (_options.IgnoreInaccessible && IsAccessError(info))
               || ContinueOnError(currentPath, info.RawErrno) || ContinueOnError(info.RawErrno);

Since new overload for ContinueOnError() doesn't exist today and default is false we can put it before ContinueOnError(info.RawErrno) overload and existing code will continue work as expected.
For new code we can document that the new overload will be called before old one. Developers have no need to implement both in new code.

[iSazonov]

Update: replaced by #1953 (comment)

Proposal:

namespace System.IO.Enumeration
{
    public unsafe abstract partial class FileSystemEnumerator<TResult> : CriticalFinalizerObject, IEnumerator<TResult>
    {
        [Obsolete("Please use ContinueOnError(string currentPath, int error) instead.")]    // New
        protected virtual bool ContinueOnError(int error) => false;    // Exists
        protected virtual bool ContinueOnError(string currentPath, int error) => false;    // New
    }

    public class FileSystemEnumerable<TResult> : IEnumerable<TResult>
    {
        public delegate bool ContinueOnErrorPredicate(string currentPath, int error);    // New
        public ContinueOnErrorPredicate? ShouldContinueOnErrorPredicate { get; set; }    // New
    }
}