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

Setting term_size to the current size if None #209

Merged
merged 3 commits into from
Jan 15, 2017

Conversation

timgabets
Copy link
Contributor

This improvement may be useful for the 'remote' debugging in a next terminal tab - in this case the debugger window will be opened with a full screen resolution, not with 80x24

@inducer
Copy link
Owner

inducer commented Dec 16, 2016

Thanks for your contribution!

I'd like to see more error handling here, so that even if this fails, it doesn't make matters worse. I'm also not a fan of doing this via a subprocess, since there are many situation where forking a aubprocess is undefined behavior, such as when a GPU context is active, or when the process is part of an MPI communicator that's backed by a DMA-based network such as Infiniband. Coincidentally, these are primary uses (for me) of remote debugging. I'd prefer this IOCTL-based solution.

Lastly, if available, this should just use Python's built-in facility to get the terminal size.

@timgabets
Copy link
Contributor Author

Sorry for a late reply.

Lastly, if available, this should just use Python's built-in facility to get the terminal size.

Umm, well, as I see, shutil.get_terminal_size() is only available for Python 3. Checking a python version inside of the set_trace() looks a little ugly to me, but if it's OK to you, I'll do it.

@inducer
Copy link
Owner

inducer commented Jan 12, 2017 via email

@timgabets
Copy link
Contributor Author

Done

@inducer inducer merged commit 9a339be into inducer:master Jan 15, 2017
@inducer
Copy link
Owner

inducer commented Jan 15, 2017

Thanks!

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