Re: Patch: session saving

[Syd Polk <spolk at redhat dot com>]

I will let others comment on this before I approve it, but it looks like a 
great feature. I see nothing holding it up, but I would like others to see it.

At 01:40 PM 11/28/00 -0700, Tom Tromey wrote:
>This patch implements simple session saving for Insight.
>
>A session is keyed to the executable name.  With this patch, the
>session is automatically saved whenever the executable is changed (the
>pre-change session is saved) and whenever gdbtk exits.
>
>Whatever sessions are available are listed on the File menu.
>
>Currently saved information is the executable name, the current
>working directory, the source file search list, and the command-line
>arguments.
>
>This patch also includes a simple (required) bug fix in prefs.tcl to
>allow for keys with multiple `/'s in them.
>
>Future work includes:
>
>* Make saving the information conditional in useful ways.
>   For instance we could have a "manual save" mode where nothing
>   is done automatically but the session can be saved via a button or
>   menu item
>
>* Save target and target settings
>
>* Save breakpoints and watchpoints (probably conditionally)
>
>* Longer term, saving window layouts and expression watches
>
>I don't plan to do all of this, but I'll definitely do some of it.
>
>
>(BTW why don't we have a unified gdbtk ChangeLog?  Having two is
>annoying.  For one thing you can't properly document a change that
>spans the Tcl and C code, like this one.)
>
>
>This patch also requires this trivial gdb patch (patch omitted as it
>is obvious from the ChangeLog entry):
>
>2000-11-28  Tom Tromey  <tromey@cygnus.com>
>
>         * infcmd.c (inferior_args): No longer static.
>
>I'm not sure if this is a smart thing to do or not.
>Is there a better way for gdbtk to get this information from gdb?
>Using the `show args' command is not better.  It is worse because you
>have to parse the output in an ugly way.
>
>
>I'd like to commit this now, before I start work on the next round of
>enhancements to session saving.  It is actually useful as it now
>stands and I don't think it hurts anything (you never have to choose a
>saved session).
>
>Ok?
>
>2000-11-28  Tom Tromey  <tromey@cygnus.com>
>
>         * tclIndex: Rebuilt.
>         * interface.tcl (set_exe_name): Save session.
>         (gdbtk_quit_check): Save session.
>         * prefs.tcl (pref_save): Added `session' to list of sections.
>         Allow keys with many `/'s.
>         * session.tcl: New file.
>         * srcbar.tcl (create_menu_items): Add menu items to recall old
>         sessions.
>
>2000-11-28  Tom Tromey  <tromey@cygnus.com>
>
>         * gdbtk-cmds.c (Gdbtk_Init): Create gdb_current_directory,
>         gdb_inferior_args, and gdb_source_path variables.
>
>Tom
>
>Index: generic/gdbtk-cmds.c
>===================================================================
>RCS file: /cvs/src/src/gdb/gdbtk/generic/gdbtk-cmds.c,v
>retrieving revision 1.12
>diff -u -r1.12 gdbtk-cmds.c
>--- gdbtk-cmds.c        2000/07/25 20:41:07     1.12
>+++ gdbtk-cmds.c        2000/11/28 19:40:15
>@@ -60,6 +60,10 @@
>  #include "annotate.h"
>  #include <sys/time.h>
>
>+/* Various globals we reference.  */
>+extern char *source_path;
>+extern char *inferior_args;
>+
>  static void setup_architecture_data (void);
>  static int tracepoint_exists (char *args);
>
>@@ -418,6 +422,28 @@
>                (char *) &gdb_context,
>                TCL_LINK_INT | TCL_LINK_READ_ONLY);
>
>+  /* Make gdb's notion of the pwd visible.  This is read-only because
>+     (1) it doesn't make sense to change it directly and (2) it is
>+     allocated using xmalloc and not Tcl_Alloc.  You might think we
>+     could just use the Tcl `pwd' command.  However, Tcl (erroneously,
>+     imho) maintains a cache of the current directory name, and
>+     doesn't provide a way for gdb to invalidate the cache.  */
>+  Tcl_LinkVar (interp, "gdb_current_directory",
>+              (char *) &current_directory,
>+              TCL_LINK_STRING | TCL_LINK_READ_ONLY);
>+
>+  /* Current gdb source file search path.  This is read-only for
>+     reasons similar to those for gdb_current_directory.  */
>+  Tcl_LinkVar (interp, "gdb_source_path",
>+              (char *) &source_path,
>+              TCL_LINK_STRING | TCL_LINK_READ_ONLY);
>+
>+  /* Current inferior command-line arguments.  This is read-only for
>+     reasons similar to those for gdb_current_directory.  */
>+  Tcl_LinkVar (interp, "gdb_inferior_args",
>+              (char *) &inferior_args,
>+              TCL_LINK_STRING | TCL_LINK_READ_ONLY);
>+
>    /* Init variable interface... */
>    if (gdb_variable_init (interp) != TCL_OK)
>      return TCL_ERROR;
>Index: library/interface.tcl
>===================================================================
>RCS file: /cvs/src/src/gdb/gdbtk/library/interface.tcl,v
>retrieving revision 1.6
>diff -u -r1.6 interface.tcl
>--- interface.tcl       2000/11/06 22:40:16     1.6
>+++ interface.tcl       2000/11/28 19:40:22
>@@ -206,7 +206,7 @@
>  #  PROCEDURE:  gdbtk_quit_check - Ask if the user really wants to quit.
>  # ------------------------------------------------------------------
>  proc gdbtk_quit_check {} {
>-  global gdb_downloading gdb_running
>+  global gdb_downloading gdb_running gdb_exe_name
>
>    if {$gdb_downloading} {
>      set msg "Downloading to target,\n really close the debugger?"
>@@ -214,14 +214,16 @@
>        return 0
>      }
>    } elseif {$gdb_running} {
>-    # While we are running the inferior, gdb_cmd is fenceposted and returns
>-    # immediately. Therefore, we need to ask here. Do we need to stop the 
>target,
>-    # too?
>+    # While we are running the inferior, gdb_cmd is fenceposted and
>+    # returns immediately. Therefore, we need to ask here. Do we need
>+    # to stop the target, too?
>      set msg "A debugging session is active.\n"
>      append msg "Do you still want to close the debugger?"
>      if {![gdbtk_tcl_query $msg no]} {
>        return 0
>      }
>+  } elseif {$gdb_exe_name != ""} {
>+    session_save
>    }
>    return 1
>  }
>@@ -736,6 +738,11 @@
>  proc set_exe_name {exe} {
>    global gdb_exe_name gdb_exe_changed
>    #debug "set_exe_name: exe=$exe  gdb_exe_name=$gdb_exe_name"
>+
>+  if {$gdb_exe_name != ""} then {
>+    session_save
>+  }
>+
>    set gdb_exe_name $exe
>    set gdb_exe_changed 1
>  }
>Index: library/prefs.tcl
>===================================================================
>RCS file: /cvs/src/src/gdb/gdbtk/library/prefs.tcl,v
>retrieving revision 1.3
>diff -u -r1.3 prefs.tcl
>--- prefs.tcl   2000/04/04 00:17:47     1.3
>+++ prefs.tcl   2000/11/28 19:40:22
>@@ -170,16 +170,18 @@
>        }
>      }
>
>-    #now loop through all sections writing out values
>+    # now loop through all sections writing out values
>+    # FIXME: this is broken.  We should discover the list
>+    # dynamically.
>      lappend secs load console src reg stack locals watch bp search \
>-      process geometry help browser kod window
>+      process geometry help browser kod window session
>
>      foreach section $secs {
>        puts $fd "\[$section\]"
>        foreach var $plist {
>         set t [split $var /]
>         if {[lindex $t 0] == "gdb" && [lindex $t 1] == $section} {
>-       set x [lindex $t 2]
>+         set x [join [lrange $t 2 end] /]
>           set v [escape_value [pref get $var]]
>           if {$x != "" && $v != ""} {
>             puts $fd "\t$x=$v"
>Index: library/srcbar.tcl
>===================================================================
>RCS file: /cvs/src/src/gdb/gdbtk/library/srcbar.tcl,v
>retrieving revision 1.2
>diff -u -r1.2 srcbar.tcl
>--- srcbar.tcl  2000/03/10 18:53:05     1.2
>+++ srcbar.tcl  2000/11/28 19:40:23
>@@ -123,6 +123,17 @@
>      add_menu_command Other "Source..." \
>        "source_file" -underline 0
>
>+    set sessions [session_list]
>+    if {[llength $sessions]} {
>+      add_menu_separator
>+      set i 1
>+      foreach item $sessions {
>+       add_menu_command Other "$i $item" \
>+         [list session_load $item] \
>+         -underline 0
>+      }
>+    }
>+
>      add_menu_separator
>
>      if {$tcl_platform(platform) == "windows"} {
>Index: library/session.tcl
>===================================================================
>RCS file: session.tcl
>diff -N session.tcl
>--- /dev/null   Tue May  5 13:32:27 1998
>+++ session.tcl Tue Nov 28 12:14:57 2000
>@@ -0,0 +1,127 @@
>+# Local preferences functions for GDBtk.
>+# Copyright 2000 Red Hat, Inc.
>+#
>+# This program is free software; you can redistribute it and/or modify it
>+# under the terms of the GNU General Public License (GPL) as published by
>+# the Free Software Foundation; either version 2 of the License, or (at
>+# your option) any later version.
>+#
>+# This program is distributed in the hope that it will be useful,
>+# but WITHOUT ANY WARRANTY; without even the implied warranty of
>+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+# GNU General Public License for more details.
>+
>+#
>+# This procedure decides what makes up a gdb `session'.  Roughly a
>+# session is whatever the user found useful when debugging a certain
>+# executable.
>+#
>+# Eventually we should expand this procedure to know how to save
>+# window placement and contents.  That requires more work.
>+#
>+proc session_save {} {
>+  global gdb_exe_name gdb_target_name
>+  global gdb_current_directory gdb_source_path gdb_inferior_args
>+
>+  # gdb sessions are named after the executable.  However, we have to
>+  # change `/' in the name.  We use a URL-like encoding here.
>+  regsub -all -- / $gdb_exe_name "%2f" name
>+  set key gdb/session/$name
>+
>+  # We fill a hash and then use that to set the actual preferences.
>+
>+  # Always set the exe. name in case we later decide to change the
>+  # interpretation of the session key.
>+  set values(executable) $gdb_exe_name
>+
>+  # Some simple state the user wants.  FIXME: these should have
>+  # dedicated commands instead of using `gdb_cmd'.
>+  set values(args) $gdb_inferior_args
>+  set values(dirs) $gdb_source_path
>+  set values(pwd) $gdb_current_directory
>+
>+  set values(target) $gdb_target_name
>+
>+  # Recompute list of recent sessions.  Trim to no more than 5 sessions.
>+  set recent [concat [list $name] \
>+               [lremove [pref getd gdb/recent-projects] $name]]
>+  if {[llength $recent] > 5} then {
>+    set recent [lreplace $recent 5 end]
>+  }
>+  pref setd gdb/recent-projects $recent
>+
>+  foreach k [array names values] {
>+    pref setd $key/$k $values($k)
>+  }
>+  pref setd $key/all-keys [array names values]
>+}
>+
>+#
>+# Load a session saved with session_save.  NAME is the pretty name of
>+# the session, as returned by session_list.
>+#
>+proc session_load {name} {
>+  # gdb sessions are named after the executable.  However, we have to
>+  # change `/' in the name.  We use a URL-like encoding here.
>+  regsub -all -- / $name "%2f" name
>+  set key gdb/session/$name
>+
>+  # Fetch all keys for this session into an array.
>+  foreach k [pref getd $key/all-keys] {
>+    set values($k) [pref getd $key/$k]
>+  }
>+
>+  if {[info exists values(dirs)]} {
>+    # FIXME: short-circuit confirmation.
>+    gdb_cmd "directory"
>+    gdb_cmd "directory $values(dirs)"
>+  }
>+
>+  if {[info exists values(pwd)]} {
>+    gdb_cmd "cd $values(pwd)"
>+  }
>+
>+  if {[info exists values(args)]} {
>+    gdb_cmd "set args $values(args)"
>+  }
>+
>+  if {[info exists values(executable)]} {
>+    gdb_clear_file
>+    set_exe_name $values(executable)
>+    set_exe
>+  }
>+
>+  # FIXME: handle target
>+}
>+
>+#
>+# Delete a session.  NAME is the internal name of the session.
>+#
>+proc session_delete {name} {
>+  # FIXME: we can't yet fully define this because the libgui
>+  # preference code doesn't supply a delete method.
>+  set recent [lremove [pref getd gdb/recent-projects] $name]
>+  pref setd gdb/recent-projects $recent
>+}
>+
>+#
>+# Return a list of all known sessions.  This returns the `pretty name'
>+# of the session -- something suitable for a menu.
>+#
>+proc session_list {} {
>+  set newlist {}
>+  set result {}
>+  foreach name [pref getd gdb/recent-projects] {
>+    set exe [pref getd gdb/session/$name/executable]
>+    # Take this opportunity to prune the list.
>+    if {[file exists $exe]} then {
>+      lappend newlist $name
>+      lappend result $exe
>+    } else {
>+      # FIXME: if we could delete keys we would delete all keys
>+      # associated with NAME now.
>+    }
>+  }
>+  pref setd gdb/recent-projects $newlist
>+  return $result
>+}

Syd Polk		spolk@redhat.com
Engineering Manager	+1 415 777 9810 x 241
Red Hat, Inc.