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

Clarify File.WriteAllText and friends' truncation behaviour #9394

Merged
merged 3 commits into from
Nov 16, 2023

Conversation

Smaug123
Copy link
Contributor

@Smaug123 Smaug123 commented Oct 18, 2023

Summary

Clarify in File.WriteAllText and friends that the files are truncated before being overwritten. The docs as stated do not define the result of overwriting hello world with bye: is it byelo world or bye?

I determined their actual behaviour empirically on aarch64-darwin on SDK 7.0.400 with File.WriteAllText : string * string -> unit and File.WriteAllBytes : string * byte[] -> unit, but have not actually checked the other overloads and methods; I have assumed that they are consistent with each other.

@Smaug123 Smaug123 requested a review from a team as a code owner October 18, 2023 19:18
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Oct 18, 2023
@ghost
Copy link

ghost commented Oct 18, 2023

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

Issue Details

Summary

Clarify in File.WriteAllText and friends that the files are truncated before being overwritten. The docs as stated do not define the result of overwriting hello world with bye: is it byelo world or bye?

I determined their actual behaviour empirically on aarch64-darwin on SDK 7.0.400 with File.WriteAllText : string * string -> unit, but have not actually checked the other overloads and methods; I have assumed that they are consistent with each other.

Author: Smaug123
Assignees: -
Labels:

area-System.IO, community-contribution

Milestone: -

@learn-build-service-prod
Copy link

Learn Build status updates of commit 27dc14f:

✅ Validation status: passed

File Status Preview URL Details
xml/System.IO/File.xml ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

@bartonjs
Copy link
Member

Stylistically, I'm torn between this being the right approach, or explaining it in terms of FileMode.Create (https://learn.microsoft.com/en-us/dotnet/api/system.io.filemode?view=net-7.0)

<summary>Specifies that the operating system should create a new file. If the file already exists, it will be overwritten. This requires <see cref="F:System.Security.Permissions.FileIOPermissionAccess.Write" /> permission. <see langword="FileMode.Create" /> is equivalent to requesting that if the file does not exist, use <see cref="F:System.IO.FileMode.CreateNew" />; otherwise, use <see cref="F:System.IO.FileMode.Truncate" />. If the file already exists but is a hidden file, an <see cref="T:System.UnauthorizedAccessException" /> exception is thrown.</summary>
.

I also didn't look to see if all relevant places were updated. So, if this (vs explaining it in terms of FileMode.Create) is the preferred approach and it's complete, LGTM.

@Smaug123
Copy link
Contributor Author

Personally I get the impression that since this is a high-level API, it shouldn't be described in terms of lower-level primitives; but that's an empirical question about the consistent style of the .NET docs which I'm not particularly well-placed to answer. (If I were writing the docs myself from scratch, I'd certainly say "This function is a high-level wrapper around the following lower-level calls", but my general vague impression is that the .NET docs don't do this.)

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.

Would you mind looking for other places that are appropriate to update. From a quick search of "overwrit" I found
Create, CreateText, Replace.

@gewarren
Copy link
Contributor

Would you mind looking for other places that are appropriate to update. From a quick search of "overwrit" I found Create, CreateText, Replace.

Ping @Smaug123

@Smaug123
Copy link
Contributor Author

I have now checked every use of the string "verwrit", and adjusted the ones where it seemed appropriate.

Copy link

Learn Build status updates of commit 9672a7d:

✅ Validation status: passed

File Status Preview URL Details
xml/System.IO/File.xml ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

Copy link

Learn Build status updates of commit 94ad57e:

✅ Validation status: passed

File Status Preview URL Details
xml/System.IO/File.xml ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

@jozkee jozkee enabled auto-merge (squash) November 16, 2023 23:10
@jozkee jozkee merged commit ca8d146 into dotnet:main Nov 16, 2023
2 checks passed
@Smaug123 Smaug123 deleted the file-write-all branch November 16, 2023 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.IO community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants