Skip to content

Conversation

@timdawborn
Copy link
Contributor

  • added support for unlink(const char *)
  • corrected fgetc(char, FILE *) -- missing return value
  • added support for getc(char, FILE *)

* corrected fgetc(char, FILE *) -- missing return value
* added support for getc(char, FILE *)
@max99x
Copy link
Contributor

max99x commented Jun 25, 2011

Could you elaborate on that unlink implementation? I can't quite see the use of deleting the local cache when you're asking for a filesystem-level deletion. Perhaps a synchronous HTTP DELETE request could be more appropriate for a browser environment?

@timdawborn
Copy link
Contributor Author

Hmm, I guess that comes down to what the virtual file system is meant to be. I was assuming it was meant to be mainly a local in-memory construct, but maybe making it fall back to a HTTP DELETE request upon file not found would make more sense?

@max99x
Copy link
Contributor

max99x commented Jun 25, 2011

I think since fopen() sends an XHR, it's logical to keep to the same level of abstraction for the rest of the file system functions.

@kripken
Copy link
Member

kripken commented Jun 25, 2011

For now, I suggest we just do a local in-memory thing. Later maybe we will expand the in-memory thing to an IndexedDB implementation (so it persists beyond the life of the current visit to the website). And perhaps after that, things like HTTP DELETE so it can even affect the remote website. There is a lot to think about here.

Ok, I want to merge this (thanks!), but (1) github says it can't be automatically merged, and (2) following github's recommended manual commands fails, on

$git pull https://github.com/timdawborn/emscripten.git 5142403
fatal: Couldn't find remote ref 5142403

replacing 5142403 with the full checksum (5142403) doesn't work either. Odd. But then I don't know too much about git merging...

I don't mind just applying the raw patch locally and pushing that. Any downside to my doing so?

@timdawborn
Copy link
Contributor Author

That is quite odd. You could just pull HEAD on my branch instead. The only additional thing in there was changing the default values in src/settings.js.

$ git remote add timdawborn https://github.com/timdawborn/emscripten.git
$ git fetch timdawborn
$ git merge timdawborn/master

@kripken
Copy link
Member

kripken commented Jun 25, 2011

Hmm, I don't want the settings change though, I want it to remain on the safest configuration by default.

I'll commit the patch locally directly.

@kripken
Copy link
Member

kripken commented Jun 25, 2011

Hmm there appears to be no way to just get the raw diff of a github commit. Googling for how just gives results of people complaining that the feature doesn't exist.

Just copying the file on top of the local one doesn't work, there are additional differences (maybe your branch isn't fully up to date?)

@kripken
Copy link
Member

kripken commented Jun 25, 2011

pushed as c9b968a

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