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

Unify reading and writing files under a single API #2517

Merged
merged 1 commit into from
Oct 29, 2022

Conversation

Geod24
Copy link
Member

@Geod24 Geod24 commented Oct 28, 2022

This removes the range API and instead uses Appender, while using a limited and centralized set of functions to read/write from files.
The end goal is to permit dependency injection,
which is a pre-requisite to extending the test-suite.

@Geod24 Geod24 requested a review from WebFreak001 October 28, 2022 16:54
RangeFile openFile(string path, FileMode mode = FileMode.read)

/// Returns the content of a file
public void[] readFile(NativePath path)
Copy link
Member

Choose a reason for hiding this comment

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

Should probably return ubyte[], even if that makes no difference to the GC, but it would match vibe.d's readFile. Or do you want to implement vibe.d compatibility with the planned dependency injection interface?

Copy link
Member Author

Choose a reason for hiding this comment

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

I originally used void[] because that's what std.file returns, but I agree that's probably better.

Copy link
Member

@WebFreak001 WebFreak001 left a comment

Choose a reason for hiding this comment

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

Appender docs say you should construct appenders like this.

source/dub/recipe/io.d Outdated Show resolved Hide resolved
source/dub/internal/utils.d Outdated Show resolved Hide resolved
source/dub/internal/utils.d Outdated Show resolved Hide resolved
source/dub/project.d Outdated Show resolved Hide resolved
This removes the range API and instead uses Appender,
while using a limited and centralized set of functions
to read/write from files.
The end goal is to permit dependency injection,
which is a pre-requisite to extending the test-suite.
@Geod24
Copy link
Member Author

Geod24 commented Oct 29, 2022

Rebased and changed appender

@Geod24 Geod24 merged commit cfd3053 into dlang:master Oct 29, 2022
@Geod24 Geod24 deleted the openFile branch October 29, 2022 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants