GNOME Bugzilla – Bug 663862
Save and restore terminal settings
Last modified: 2012-06-28 11:38:33 UTC
Created attachment 201233 [details] [review] Patch to save and restore terminal settings If the program being debugged changes the terminal settings, and then crashes or is restarted before it has a chance to reset them, then the changed terminal settings can cause display errors in future runs. This patch causes nemiver to save the terminal settings before the debugger is first started, and then restore them when starting or restarting the target, and when shutting down.
Created attachment 216683 [details] [review] Patch to save and restore terminal settings Updated patch, rebased against current master.
Ooops, For some reason, this patch felt below my radar. Sorry for that, Tom. Next time, you might just send your patches to nemiver-list@gnome.org. This is the custom for the Nemiver project. But if you prefer sticking them to bugzilla, it's fine. :-) I'll try hard to review your patch this week. Thank you.
Created attachment 217069 [details] [review] Updated patch after review Hello Tom, Once again, thank you for spending time on this nice patch, and sorry for my late review. Please find below my observations. > Before starting the debugger for the first time, same the terminal > settings and then restore them whenever we start the target and when > we are exiting so that any changes made to the terminal setting by > the target are discarded. I understand that we restore the tty attributes whenever GDB exits, so that the terminal is left in its initial state. But why do we need to do the restoring when we /(re)start/ the target? IOW, why is this change ... > @@ -5924,6 +5932,7 @@ DBGPerspective::restart_local_inferior () > // restart; in which case, we can't just simply call debugger > // ()->run (). > && debugger ()->get_target_path () == m_priv->prog_path) { > + restore_terminal_settings (); > going_to_run_target_signal ().emit (true); > debugger ()->re_run > (sigc::mem_fun ... useful? In any case, I believe that this whole tty attribute business would be more appropriately done at a lower level, namely in the GDBEngine which job is to deal with the low level GDB plumbing. I have thus updated your patch to do that, keeping your authorship, of course, and adding a GNU-style ChangeLog entry in the commit log, as recommended by the Nemiver commit log guidelines[1]. In the process, I have renamed the functions {save,restore}_terminal_settings into {get,set}_tty_attributes to avoid confusing these low level tty attributes fiddling with higher level terminal settings that might come later, like Fonts, background colors etc, which are relevant when you use the terminal widget provided by Nemiver. As the patch review request was done in bugzilla, I am attaching my updated patch to bugzilla as well. Could you please test it on your setup and see if I broke something? Thank you. [1]: http://git.gnome.org/browse/nemiver/tree/README.git-commit-messages.txt
That all seems to be working fine. The reason for resetting the attributes on a restart is that programs tend to assume that they will be started in cooked mode and if a program that has changed to raw mode is restarted (without having had a chance to cleanup) it may make incorrect assumptions about the initial state of the terminal.
Great, Thank you for testing, Tom. I have updated the patch accordingly and applied it to master. It should be available in Nemiver 0.9.3. Thank you for your time!