Add support for print to file and file flush. #27
Conversation
Codecov Report
@@ Coverage Diff @@
## master #27 +/- ##
==========================================
+ Coverage 64.67% 64.86% +0.18%
==========================================
Files 55 55
Lines 10044 10074 +30
==========================================
+ Hits 6496 6534 +38
+ Misses 3090 3079 -11
- Partials 458 461 +3
Continue to review full report at Codecov.
|
raff
force-pushed
the
print_to_file
branch
2 times, most recently
from
fb4a4f2 to
d8a60c1
Compare
builtin/builtin.go
Outdated
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| // FIXME ignoring file and flush |
There was a problem hiding this comment.
This comment is now out of date :-)
builtin/builtin.go
Outdated
| if shouldFlush, _ := py.MakeBool(flush); shouldFlush == py.True { | ||
| fflush, err := py.GetAttrString(file, "flush") | ||
| if err == nil { | ||
| py.Call(fflush, nil, nil) |
There was a problem hiding this comment.
This should be returning an error on failure I think?
There was a problem hiding this comment.
I'll see if I can reproduce a flush error in python 3 and fix if needed.
There was a problem hiding this comment.
Added check for "file already closed". I don't think flush can return any other error.
There was a problem hiding this comment.
I'd say we should as a matter of course return all the errors otherwise tools like errcheck (which I plan to run the codebase through) will complain that we aren't handling errors properly.
The error may need translating, that is for sure, but I think we should always return all errors!
Maybe we should make a generic error translator rather like the one here (from the open code)
if err != nil {
// XXX: should check for different types of errors
switch {
case os.IsExist(err):
return nil, ExceptionNewf(FileExistsError, err.Error())
case os.IsNotExist(err):
return nil, ExceptionNewf(FileNotFoundError, err.Error())
}
}
(Note that this particular bit of code is ignoring the err if it wasn't one of those two errors which it shouldn't be - that needs to be fixed)
py/file.go
Outdated
| @@ -138,6 +141,20 @@ func (o *File) Close() (Object, error) { | |||
| return None, nil | |||
| } | |||
|
|
|||
| func (o *File) Flush() (Object, error) { | |||
| _ = o.File.Sync() | |||
There was a problem hiding this comment.
Maybe this should return None, o.File.Sync() otherwise we are ignoring the error on Sync()
There was a problem hiding this comment.
I'll see if I can reproduce a flush error in python 3 and fix if needed.
py/file.go
Outdated
|
|
||
| func (o *File) M__exit__(exc_type, exc_value, traceback Object) (Object, error) { | ||
| o.Close() | ||
| return None, nil |
There was a problem hiding this comment.
Likewise here, return None, o.Close()
There was a problem hiding this comment.
o.Close() always return None - python File.close() never return an error.
py/file.go
Outdated
| @@ -250,3 +267,6 @@ func OpenFile(filename, mode string, buffering int) (Object, error) { | |||
| } | |||
|
|
|||
| // Check interface is satisfied | |||
| var _ I__enter__ = (*File)(nil) | |||
| var _ I__exit__ = (*File)(nil) | |||
| var _ I__exit__ = (*File)(nil) | |||
raff
force-pushed
the
print_to_file
branch
2 times, most recently
from
e5b841e to
24e39a3
Compare
|
Now all failed calls should properly return an error |
|
This looks great now thanks. Will merge :-) |
No description provided.