Skip to content

std.fs: Absorb IterableDir into Dir#18076

Merged
andrewrk merged 5 commits intomasterfrom
revert-iterable-dir
Nov 23, 2023
Merged

std.fs: Absorb IterableDir into Dir#18076
andrewrk merged 5 commits intomasterfrom
revert-iterable-dir

Conversation

@andrewrk
Copy link
Member

This reverts #12060.

I was against this change originally, but approved it to keep an open mind. After a year of trying it in practice, I firmly believe that the previous way of doing it was better.

Furthermore, I took this opportunity to extract Dir and AtomicFile out from fs.zig into Dir.zig and AtomicFile.zig respectively, and also to flatten std.fs.file.File into std.fs.File.

This reverts commit da94227, reversing
changes made to 8f943b3.

I was against this change originally, but decided to approve it to keep
an open mind. After a year of trying it in practice, I firmly believe
that the previous way of doing it was better.
@andrewrk andrewrk added breaking Implementing this issue could cause existing code to no longer compile or have different behavior. standard library This issue involves writing Zig code for the standard library. labels Nov 22, 2023
@squeek502
Copy link
Member

squeek502 commented Nov 22, 2023

I think extracting Dir is a good change, but I think it will also increase confusion about the Absolute suffixed functions since they will seem like they have precedence being in the std.fs namespace proper (#16736).

EDIT: Nevermind, I was thinking about this wrong. The difference will only show up in the source code, the namespaces are still the same. People still might be (incorrectly, imo) drawn towards the Absolute functions if they're looking at fs.zig directly, though.

Would like to get your thoughts on my comment here while you're thinking about std.fs

@andrewrk andrewrk force-pushed the revert-iterable-dir branch from 375cb32 to 0a536a7 Compare November 22, 2023 22:25
@nektro
Copy link
Contributor

nektro commented Nov 22, 2023

I was against this change originally, but approved it to keep an open mind. After a year of trying it in practice, I firmly believe that the previous way of doing it was better.

what led you to this? type safety from comptime-knowable state is a good thing

@andrewrk
Copy link
Member Author

My comments on #12060 are still relevant, so check those out if you haven't already. To expand:

  1. It's often, but not always, comptime-knowable whether the directory needs iterating.
  2. The other directory opening flags have the same property: some subset of directory functionality is available based on their values, and they are also often but not always comptime-knowable. You can imagine that we don't want all of the following types:
    • Dir
    • IterableDir
    • AccessSubPathsDir
    • IterableAccessSubPathsDir
    • NoFollowDir
    • NoFollowIterableDir
    • NoFollowAccessSubPathsDir
    • NoFollowIterableAccessSubPathsDir
  3. std.fs is missing an "open any" API #16738

@andrewrk andrewrk enabled auto-merge November 22, 2023 23:36
@andrewrk andrewrk merged commit f4e426a into master Nov 23, 2023
@andrewrk andrewrk deleted the revert-iterable-dir branch November 23, 2023 06:44
krichprollsch added a commit to lightpanda-io/browser that referenced this pull request Dec 4, 2023
krichprollsch added a commit to lightpanda-io/browser that referenced this pull request Dec 4, 2023
krichprollsch added a commit to lightpanda-io/browser that referenced this pull request Dec 4, 2023
krichprollsch added a commit to lightpanda-io/browser that referenced this pull request Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Implementing this issue could cause existing code to no longer compile or have different behavior. standard library This issue involves writing Zig code for the standard library.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants