Skip to content

Comments

py: implement input#252

Open
aisk wants to merge 1 commit intogo-python:mainfrom
aisk:input
Open

py: implement input#252
aisk wants to merge 1 commit intogo-python:mainfrom
aisk:input

Conversation

@aisk
Copy link
Contributor

@aisk aisk commented Feb 7, 2026

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.

Copy link
Member

@sbinet sbinet left a comment

Choose a reason for hiding this comment

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

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.`
Copy link
Member

Choose a reason for hiding this comment

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

On *nix systems, GNU readline is used if enabled

that probably isn't true for gpython, is it ?

Comment on lines +1226 to +1240
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)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

why do you go through the py.Object API here, while you don't when reading from stdin (and use the py.File API) ?

Comment on lines +1205 to +1207
if prompt != py.None {
promptStr = string(prompt.(py.String))
}
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't we use the ,-ok type assertion here ? instead of the panick-y one ?

Comment on lines +1210 to +1212
if err == io.EOF {
return nil, py.ExceptionNewf(py.EOFError, "EOF when reading a line")
}
Copy link
Member

Choose a reason for hiding this comment

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

perhaps we should prefer the errors.Is(err, io.EOF) variant ?

Comment on lines +1246 to +1248
if err.Error() == "EOF" {
return nil, py.ExceptionNewf(py.EOFError, "EOF when reading a line")
}
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't we use the errors.Is(err, io.EOF) here as well ?

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.

2 participants