No curses! #130
No curses! #130
Conversation
|
Ah, good. That should fix both #96 and #75. I'll look into this a bit better later. Thanks for the work! In the mean time, I do see some code style violations (missing spaces around operators, missing space after keywords (if, while), missing braces and enters after single-line ifs). If you could take care of those, that would make the review easier. |
|
Is there a standard guide somewhere? Yeah, I realized there where violations when I dove in there, but the original code in that area had no standardization anywhere. |
Do not initialize ncurses or do any screen clearing unless we have a tty. The fixes the broken stdout redirection.
|
Okay, I cleaned it up a fair bit. The initial patch tried to be as self-contained as possible since the original code base was just soo poorly formatted and confusing. I took some time to up the quality and dropping that dangerous function pointer cast as well. |
|
Actually, let me do some more work on this. I should move the |
|
Actually looks like those methods are coupled with screen internals and what was initially throwing me off: |
vodik
force-pushed
the
no-curses
branch
4 times, most recently
from
32c7c3d to
d25c54b
Compare
Move the signal handling out of the ncurses logic so that signals can be properly handled when curses isn't used (eg stdout redirection).
Logging to an error file requires that ncurses is initialized. Otherwise rotate_errorf is never called. Not a perfect fix, a lot of this logic should be decoupled from _screen_error.
|
Okay, as far as I know, this should be complete breaking no functionality. We've been using this patch set internally for some time but we have some different logging code (we write the screen contents out to different files in a log directory) that I'm hoping to also put up for consideration. |
Disables the use of ncurses if stdout is not a tty. Observe that that mode disables interactive mode key presses. The status screen is shown, but not if you abort prematurely with ^C. Starting without a proper TERM env now works as long as stdout is not a terminal (closes #75). Also closes #96 without the need for an extra command line option. Thanks @vodik!
Disables the use of ncurses if stdout is not a tty. Observe that that mode disables interactive mode key presses. The status screen is shown, but not if you abort prematurely with ^C. Starting without a proper TERM env now works as long as stdout is not a terminal (closes #75). Also closes #96 without the need for an extra command line option. Thanks @vodik!
|
Oh shit, sorry. |
This patches SIPp to conditionally only enables ncurses if and only if stdout is a tty. This means sipp will now work with its standard out redirected to file, no terminal corruption because of ncurses mode being initialized anyways.
In order to support this, however, the signal handling has to be decoupled from the ncurses support. Hence a slight bit of rewriting and migration of a lot of logic in
screen.cppintosipp.cpp.