Conversation
sbinet
left a comment
There was a problem hiding this comment.
thanks for the PR.
I have a couple of comments, see below.
also, it'd be great to add tests for this.
thanks again.
| The prompt string, if given, is printed to standard output without a | ||
| trailing newline before reading input. | ||
| If the user hits EOF (*nix: Ctrl-D, Windows: Ctrl-Z+Return), raise EOFError. | ||
| On *nix systems, GNU readline is used if enabled.` |
There was a problem hiding this comment.
On *nix systems, GNU readline is used if enabled
that probably isn't true for gpython, is it ?
| if prompt != py.None { | ||
| write, err := py.GetAttrString(stdout, "write") | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| _, err = py.Call(write, py.Tuple{prompt}, nil) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| flush, err := py.GetAttrString(stdout, "flush") | ||
| if err == nil { | ||
| py.Call(flush, nil, nil) | ||
| } | ||
| } |
There was a problem hiding this comment.
why do you go through the py.Object API here, while you don't when reading from stdin (and use the py.File API) ?
| if prompt != py.None { | ||
| promptStr = string(prompt.(py.String)) | ||
| } |
There was a problem hiding this comment.
shouldn't we use the ,-ok type assertion here ? instead of the panick-y one ?
| if err == io.EOF { | ||
| return nil, py.ExceptionNewf(py.EOFError, "EOF when reading a line") | ||
| } |
There was a problem hiding this comment.
perhaps we should prefer the errors.Is(err, io.EOF) variant ?
| if err.Error() == "EOF" { | ||
| return nil, py.ExceptionNewf(py.EOFError, "EOF when reading a line") | ||
| } |
There was a problem hiding this comment.
shouldn't we use the errors.Is(err, io.EOF) here as well ?
Since liner takes over control of stdin in the REPL, an InputHook is added and registered in REPL mode, which will use liner to get the input instead of reading from stdin directly.