-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Revert FileSystemEventArgs/RenamedEventArgs FullPath/OldFullPath changes #68883
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
Revert FileSystemEventArgs/RenamedEventArgs FullPath/OldFullPath changes #68883
Conversation
|
Tagging subscribers to this area: @dotnet/area-system-io Issue DetailsAfter fixing #68566 with #68582, which was addressing a regression caused by #63051, which fixed #62606, I've reconsidered the original issue further. This PR reverts both #68582 and #63051 and returns The regression was reported involved using an empty directory name, and it was relying on I've confirmed that with these reverts,
I will follow up with a documentation issue that clarifies that
|
src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemEventArgs.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemEventArgs.cs
Outdated
Show resolved
Hide resolved
|
Thanks, @stephentoub. I was so focused on reverting the changes/behavior, I didn't look for the modernization opportunities there. |
jozkee
left a comment
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.
Thanks, Jeff.
After fixing #68566 with #68582, which was addressing a regression caused by #63051, which fixed #62606, I've reconsidered the original issue further. This PR reverts both #68582 and #63051 and returns
FileSystemEventArgsandRenamedEventArgsback to their previous behavior forFullPathandOldFullPaththat dates back to .NET Framework.The regression was reported involved using an empty directory name, and it was relying on
FullPathbeing relative to the process's directory. From that regression report, it's easy to imagine scenarios where the non-rooted nature ofFullPathis utilized and perhaps even leveraged to combine that full path to a different root. I remember such code I wrote myself back in .NET Framework days that would be broken by the changes we had made.I've confirmed that with these reverts,
FileSystemEventArgsandRenamedEventArgsare now functionally equivalent to what existed in .NET Core 1.0 and .NET Framework, with only these exceptions:ArgumentNullExceptioninstead ofNullReferenceExceptionwhendirectoryis nullI will follow up with a documentation issue that clarifies that
FullPathandOldFullPathare not rooted, but they are mere combinations of the specified directory and file name, and when directory is an empty string, the values will begin with a directory separator character.