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

Implement file create #908

Closed
wants to merge 2 commits into from
Closed

Implement file create #908

wants to merge 2 commits into from

Conversation

ztplz
Copy link
Contributor

@ztplz ztplz commented Oct 4, 2018

Prior art:
go: func Create(name string) (*File, error)
rust: tokio::fs::File::create<P>(path: P) -> CreateFuture<P>

js/files_test.ts Outdated

test(async function createFileSuccess() {
const path = "test.txt";
let f = await deno.create(path);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you create the test file in a temporary directory? Like using a path given by deno.makeTempDir

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.

Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

Hi - thanks for starting this - but create should be implemented in terms of open. Just the “mode” needs to be passed down.

export async function create(filename: string): Promise<File> {
const builder = new flatbuffers.Builder();
const filename_ = builder.createString(filename);
msg.Create.startCreate(builder);
Copy link
Member

Choose a reason for hiding this comment

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

I think this doesn’t need a separate msg. We should be able to reuse Open.


table CreateRes {
rid: int;
}
Copy link
Member

Choose a reason for hiding this comment

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

Reuse Open/OpenRes instead of adding this.

@@ -558,6 +559,41 @@ fn op_mkdir(
})
}

fn op_create(
Copy link
Member

Choose a reason for hiding this comment

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

Reuse op_open

@ztplz
Copy link
Contributor Author

ztplz commented Oct 5, 2018

@ry I think permission mode should be completed before restart it. Next I will try to work on permission design.

@hayd
Copy link
Contributor

hayd commented Oct 6, 2018

If create can be defined as writeFileSync('touched', new Uint8Array(), 0o666); does it need to be defined by deno?

@ry
Copy link
Member

ry commented Oct 15, 2018

@ztplz I'm going to close this one as it's getting dated - if you come back to this project please open a new PR and link to this one. Thanks!

@ry ry closed this Oct 15, 2018
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.

4 participants