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

Remove workaround for bug affecting very old platforms #40

Merged
merged 1 commit into from Feb 13, 2014

Conversation

Projects
None yet
2 participants
@emaste
Contributor

emaste commented Feb 13, 2014

#39

# termios.TIOCSWINSZ to be truncated. There was a hack here to work
# around this, but it caused problems with newer platforms so has been
# removed. For details see https://github.com/pexpect/pexpect/issues/39
TIOCSWINSZ = getattr(termios, 'TIOCSWINSZ', 2148037735)

This comment has been minimized.

@takluyver

takluyver Feb 13, 2014

Member

Can we leave the default in this line as it was? I assume FreeBSD sets TIOCSWINSZ, so the default won't be an issue for you, and I've yet to hear any complaints that the setwinsize method doesn't work.

This comment has been minimized.

@emaste

emaste Feb 13, 2014

Contributor

Sure, although the -ve default is wrong on both Linux and FreeBSD. Perpahs we should just drop it altogether?

This comment has been minimized.

@takluyver

takluyver Feb 13, 2014

Member

But the Linux and FreeBSD we're aware of don't fall back to the default. We know of no situation where the default is used and is wrong, but presumably there was once some situation where it was used and was right, so until I hear otherwise, I'm inclined to leave it alone.

This comment has been minimized.

@emaste

emaste Feb 13, 2014

Contributor

Fair enough. However I very much doubt there is a platform on which both of the TIOC{G,S}WINSZ defaults in this file are correct (regardless of whether this one is 2148037735 or -2146929561).

TIOCGWINSZ = getattr(termios, 'TIOCGWINSZ', 1074295912)
TIOCSWINSZ = getattr(termios, 'TIOCSWINSZ', -2146929561)

This comment has been minimized.

@takluyver

takluyver Feb 13, 2014

Member

You could very well be right.

takluyver added a commit that referenced this pull request Feb 13, 2014

Merge pull request #40 from emaste/master
Remove workaround for bug affecting very old platforms

@takluyver takluyver merged commit d7be1bd into pexpect:master Feb 13, 2014

1 check passed

default The Travis CI build passed
Details
@emaste

This comment has been minimized.

Contributor

emaste commented Feb 13, 2014

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment