Shen-Language / shen-sources

Shen language kernel sources for porters
Other
358 stars 41 forks source link

Open files in append mode #7

Open tizoc opened 8 years ago

tizoc commented 8 years ago

Append mode streams discussion

Shen truncates files when it opens them and there is no way to specify if files should be opened in stream mode.

Current implementations

TODO: add others

rkoeninger commented 7 years ago

I think the most consistent way to add this would be to have (open FilePath append) and avoid adding a new function.

I added it this way in my port: https://github.com/rkoeninger/ShenSharp/commit/620c042069ebd9ab022a733791a8bde757d6db03

tizoc commented 7 years ago

@rkoeninger have you tried it with the type checker enabled? because adding support in the backend is not enough since Shen's type-checker treats open in a special way (it recognizes uses of open, takes the last argument and then uses it to construct the type of the resulting value).

In your case, when you open with append you will get a result with type (stream append) which is incompatible with (stream out).

tizoc commented 7 years ago

To clarify, it is not something that is not fixeable, I know what code to change and where, but I would rather not introduce changes to existing functionality when possible.

Also I never really liked for open to work the way it works, I would rather have two separate functions, open-in and open-out, it is simpler that way, doesn't require anything special from the typechecked, and an extra parameter could be used to pass extra options, something like this for append for example:

(open-out "/some/path" [append create-if-not-exists])
(open-out "/some/path" [truncate])
(open-out "/some/path" [fail-if-exists])

etc etc (you get the idea).

This option leaves things open for further extension (because append is not the only possible modifier for how files are opened).

rkoeninger commented 7 years ago

I get a type error for (open "file.txt" append), but you're right - the open function is weird.

I'm guessing the additional options would all optional per platform.