Skip to content

Allow empty directory values in FileSystemEventArgs/RenamedEventArgs ctors #68582

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.

Sign up for GitHub

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

Jump to bottom
Merged
jeffhandley merged 3 commits into from
Apr 29, 2022
Merged

Allow empty directory values in FileSystemEventArgs/RenamedEventArgs ctors #68582

jeffhandley merged 3 commits into from
Apr 29, 2022

Conversation

@jeffhandley
Copy link
Member

@jeffhandley jeffhandley commented Apr 27, 2022

Fixes #68566.

A regression was introduced by #63051. That PR introduced an intentional breaking change where FileSystemEventArgs.FullPath would become a fully-qualified path, but the change also included an unintentional breaking change that prevented FileSystemEventArgs and RenamedEventArgs from accepting a directory value that is null or whitespace. That PR added asserts for the empty directory case, ensuring exceptions were thrown, but in .NET 6, the behavior of the event args types was to infer a relative directory in that scenario.

This change allows empty directory values while still resulting in fully-qualified paths for FullPath and OldFullPath.

@jeffhandley jeffhandley added this to the 7.0.0 milestone Apr 27, 2022
@ghost ghost added the area-System.IO label Apr 27, 2022
@ghost ghost assigned jeffhandley Apr 27, 2022
@ghost
Copy link

ghost commented Apr 27, 2022

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #68566.

A regression was introduced by #63051. That PR introduced an intentional breaking change where FileSystemEventArgs.FullPath would become a fully-qualified path, but the change also included an unintentional breaking change that prevented FileSystemEventArgs and RenamedEventArgs from accepting a directory value that is null or whitespace. That PR added asserts for the empty directory case, ensuring exceptions were thrown, but in .NET 6, the behavior of the event args types was to infer a relative directory in that scenario.

This change allows empty directory values while still resulting in fully-qualified paths for FullPath and OldFullPath.

Author: jeffhandley
Assignees: -
Labels:

area-System.IO
Milestone: 7.0.0

adamsitnik
adamsitnik approved these changes Apr 27, 2022
View reviewed changes
Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

LGTM, @jeffhandley thank you for both fixing the issue and improving the tests!

jozkee
jozkee approved these changes Apr 27, 2022
View reviewed changes
Copy link
Member

@jozkee jozkee left a comment

Choose a reason for hiding this comment

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

Left a style suggestion, but LGTM, thanks.

@jeffhandley jeffhandley mentioned this pull request Apr 28, 2022
Closed
@stephentoub stephentoub mentioned this pull request Apr 28, 2022
Merged
@jeffhandley jeffhandley merged commit daec9dc into dotnet:main Apr 29, 2022
@jeffhandley jeffhandley deleted the jeffhandley/filesystemeventargs branch May 4, 2022 20:43
@jeffhandley jeffhandley mentioned this pull request May 5, 2022
Closed
jeffhandley added a commit to jeffhandley/runtime that referenced this pull request May 5, 2022
@jeffhandley
Revert daec9dc (PR dotnet#68582)
04a7fc5
@jeffhandley jeffhandley mentioned this pull request May 5, 2022
Merged
jeffhandley added a commit that referenced this pull request May 5, 2022
@jeffhandley
Revert FileSystemEventArgs/RenamedEventArgs FullPath/OldFullPath chan…
45f15a5 
…ges (#68883)

* Revert daec9dc (PR #68582)

* Revert 2a1b15d (PR #63051)

* Restore additional tests adapted to match prior behavior

* Remove unnecessary PathInternal inclusion

* PR feedback

* Fix unix tests (copy/paste error)
@ghost ghost locked as resolved and limited conversation to collaborators Jun 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@stephentoub stephentoub stephentoub left review comments

@jkotas jkotas jkotas left review comments

@jozkee jozkee jozkee approved these changes

@adamsitnik adamsitnik adamsitnik approved these changes

Assignees

@jeffhandley jeffhandley

Labels

area-System.IO

Projects

None yet

Milestone

7.0.0

Development

Successfully merging this pull request may close these issues.

FileSystemEventArgs constructor throws with empty directory value

5 participants

@jeffhandley @stephentoub @adamsitnik @jkotas @jozkee