Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added screen size parameters. #11

Merged
merged 3 commits into from Mar 17, 2015
Merged

Added screen size parameters. #11

merged 3 commits into from Mar 17, 2015

Conversation

detly
Copy link
Contributor

@detly detly commented Mar 16, 2015

I found myself wanting to set the size of the pty as seen by the subprocess (really for pexpect.spawn, but this is needed first). In some situations it can make parsing/interaction a lot easier.

Happy to change any stylistic things, eg.

  1. The dimensions=(24, 80) default arg could also be dimensions=None with some if dimensions is not None logic in spawn().
  2. It could be r=24, c=80 instead, but does it make sense to only have one dimension with a default? (Maybe, I don't know.)

I meant to add a test, but couldn't figure out a simple enough way to test this (eg. if there was a reliable command to check console size it could be like the other spawn tests, but I don't know of one).

@takluyver
Copy link
Member

Seems fine, thanks. I thought about asking for rows and cols for symmetry with setwinsize(), but I agree with you that it would be bizarre to set one without the other.

This does all the fork/exec type of stuff for a pty, and returns an
instance of PtyProcess.

If preexec_fn is supplied, it will be called with no arguments in the
child process before exec-ing the specified command.
It may, for instance, set signal handlers to SIG_DFL or SIG_IGN.

Dimensions of the psuedoterminal used for the subprocess can be
specified as a tuple, or the default (24, 80) will be used.
Copy link
Member

Choose a reason for hiding this comment

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

Let's be precise '...as a tuple (rows, cols), ...'

@detly
Copy link
Contributor Author

detly commented Mar 16, 2015

Heh, I hadn't thought about that inconsistency actually. Well, I'll leave
it up to you; sometimes consistency is worth it, sometimes no one will mind
:)
On 17/03/2015 7:50 AM, "Thomas Kluyver" notifications@github.com wrote:

Seems fine, thanks. I thought about asking for rows and cols for symmetry
with setwinsize(), but I agree with you that it would be bizarre to set
one without the other.


Reply to this email directly or view it on GitHub
#11 (comment).

@detly
Copy link
Contributor Author

detly commented Mar 16, 2015

I can make that change.
On 17/03/2015 7:53 AM, "Jason Heeris" jason.heeris@gmail.com wrote:

Heh, I hadn't thought about that inconsistency actually. Well, I'll leave
it up to you; sometimes consistency is worth it, sometimes no one will mind
:)
On 17/03/2015 7:50 AM, "Thomas Kluyver" notifications@github.com wrote:

Seems fine, thanks. I thought about asking for rows and cols for
symmetry with setwinsize(), but I agree with you that it would be
bizarre to set one without the other.


Reply to this email directly or view it on GitHub
#11 (comment).

@jquast
Copy link
Member

jquast commented Mar 16, 2015

+1

@takluyver
Copy link
Member

Thanks!

takluyver added a commit that referenced this pull request Mar 17, 2015
Added screen size parameters.
@takluyver takluyver merged commit 3942883 into pexpect:master Mar 17, 2015
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.

None yet

3 participants