This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Use external editor in 'commands' command


2009/2/2 Tom Tromey <tromey@redhat.com>:
>>>>>> "Alfredo" == Alfredo Ortega <ortegaalfredo@gmail.com> writes:
>
> Alfredo> 1) Following the suggestion of Tom Tromey, i made "commands"
> Alfredo> a prefix command, and now it is "commands edit n".
>
> Your patch does this using strncmp -- but gdb already has built-in
> machinery for prefix commands.  See add_prefix_cmd.  So, the idea here
> would be to change _initialize_breakpoint to use add_prefix_cmd,
> instead of add_com, when creating the "commands" command.  Then, you'd
> have a separate function to implement "commands edit".  Maybe this
> means introducing a helper function to do some of the work, I don't
> know.
>
> Alfredo> This is a much better patch, but also is a much bigger one (I already
> Alfredo> sent the FSF form that Tom suggested), so surely there are plenty of
> Alfredo> errors. Corrections are welcomed.
>
> A few nits inline.
>
> Alfredo> +#define COMMANDS_EDCOMMAND "edit"
>
> With the prefix change, you won't need this.
>
> Alfredo> +  /* Edit commands with external editor */
>
> In the GNU style, comments are full sentences that start with a
> capital letter and end with a period and two spaces.  This one is
> missing the period, but others are incorrect in other ways.
>
> Alfredo> +      /* discard the "edit" command */
>
> E.g., this one...
>
> Alfredo> +      get_number (&p);
> Alfredo> +      bnum = get_number (&p);
>
> Two calls to get_number seems suspect.
>
> Alfredo> +      /* vitmp = tempnam(NULL,".gdb"); this is more secure according to man mkstemp, but gcc complains... */
>
> There's no need for this comment, IMO.
>
> Alfredo> +      if (!(vitmp = make_temp_file (NULL)))
>
> GNU style prohibits assignments in conditionals.
>
> Alfredo> +      {
> Alfredo> +        error (_("Can't create temporary file for editing."));
> Alfredo> +        return;
> Alfredo> +      }
>
> The "error" function never returns.  It calls longjmp.  So, this
> return is not needed.  This occurs in a few spots.
>
> Alfredo> +            l = b->commands;
> Alfredo> +            while (l)
> Alfredo> +              {
> Alfredo> +                fsize = 0;
> Alfredo> +                fsize += fwrite (l->line, 1, strlen (l->line), tmpstream);
>
> I think you should probably use "print_command_lines" to print the
> breakpoint commands to a file.
>
> Alfredo> +        sysret = system (cmdline);
> Alfredo> +        xfree (cmdline);
> Alfredo> +        if (sysret < 0)
>
> I think this should also check "sysret" when it is >= 0, and fail if
> the editor does not exit with status 0.
>
> Thanks for working on this,
> Tom
>

Sorry for the late answer.
I refactored the whole patch, is getting better...add_prefix_cmd()
made me sweat! :) and print_command_lines() is a little tricky to use,
so maybe i didn't get it right this time.
Also I send the FSF paperwork and tried to do all your corrections in
the following patch.
Thanks to you for teaching and the patience!

Regards,

Alfred
diff -upr OLD/src/gdb/breakpoint.c NEW/src/gdb/breakpoint.c
--- OLD/src/gdb/breakpoint.c	2009-01-14 03:10:29.000000000 -0200
+++ NEW/src/gdb/breakpoint.c	2009-02-19 06:12:26.000000000 -0200
@@ -45,6 +45,7 @@
 #include "completer.h"
 #include "gdb.h"
 #include "ui-out.h"
+#include "cli-out.h"
 #include "cli/cli-script.h"
 #include "gdb_assert.h"
 #include "block.h"
@@ -585,20 +586,116 @@ condition_command (char *arg, int from_t
   error (_("No breakpoint number %d."), bnum);
 }
 
+/* Dumps breakpoint commands to a file.  */
 static void
-commands_command (char *arg, int from_tty)
+commands_dump_to_file (char *filename, struct breakpoint *b)
+{
+  struct cleanup *cleanups;
+  struct ui_file *old, *outfile = gdb_fopen (filename, "w");
+  cleanups = make_cleanup_ui_file_delete (outfile);
+  old = cli_out_set_stream (uiout, outfile);
+  print_command_lines (uiout, b->commands, 4);
+  cli_out_set_stream (uiout, old);
+  do_cleanups (cleanups);
+
+}
+
+/* Launches the editor on the breakpoint command.  */
+char *
+commands_edit_command (int bnum)
 {
+  char *vitmp = NULL;
+  char *editor;
   struct breakpoint *b;
-  char *p;
-  int bnum;
-  struct command_line *l;
+  char *cmdline = NULL;
+  int sysret;
+  /* Generates the temporary file name.  */
+  vitmp = make_temp_file (NULL);
+  if (!vitmp)
+      error (_("Can't create temporary file for editing."));
+  editor = external_editor ();
+  if (!editor)
+      error (_("External editor not found."));
+  ALL_BREAKPOINTS (b) if (b->number == bnum)
+    {
+      if (&b->commands)
+	commands_dump_to_file (vitmp, b);
+      /* Edit the file.  */
+      cmdline = xmalloc (strlen (editor) + strlen (vitmp) + 50);
+      sprintf (cmdline, "%s \"%s\"", editor, vitmp);
+      sysret = system (cmdline);
+      xfree (cmdline);
+      if (sysret < 0)
+	  error (_("Can't execute external editor."));
+      if (sysret > 0)
+	  error (_("External editor returned non-zero status."));
+    }
+  return vitmp;
+}
+
 
   /* If we allowed this, we would have problems with when to
      free the storage, if we change the commands currently
      being read from.  */
 
+static void
+check_executing_commands ()
+{
   if (executing_breakpoint_commands)
-    error (_("Can't use the \"commands\" command among a breakpoint's commands."));
+    error (_
+	   ("Can't use the \"commands\" command among a breakpoint's commands."));
+}
+
+/* Like commands_command but using an external editor.  */
+static void
+edit_command (char *args, int from_tty)
+{
+  int bnum;
+  char *p;
+  char *vitmp;
+  struct breakpoint *b;
+  struct command_line *l;
+  FILE *tmpstream = NULL;
+
+  check_executing_commands ();
+  p = args;
+  if (!p)
+    error (_("No breakpoint number."));
+  bnum = get_number (&p);
+  if (p && *p)
+    error (_("Unexpected extra arguments following breakpoint number."));
+  /* Launches the editor.  */
+  vitmp = commands_edit_command (bnum);
+  if (!vitmp)
+    return;
+
+  ALL_BREAKPOINTS (b) if (b->number == bnum)
+    {
+      /* Redirect instream to the commands temporal file.  */
+      tmpstream = instream;
+      instream = fopen (vitmp, "r");
+      l = read_command_lines (NULL, from_tty, 1);
+      free_command_lines (&b->commands);
+      b->commands = l;
+      breakpoints_changed ();
+      observer_notify_breakpoint_modified (b->number);
+      /* Restore instream and erase temporal file.  */
+      instream = tmpstream;
+      unlink (vitmp);
+      return;
+    }
+  error (_("No breakpoint number %d."), bnum);
+}
+
+static void
+commands_command (char *arg, int from_tty)
+{
+  struct breakpoint *b;
+  char *p;
+  int bnum;
+  struct command_line *l;
+
+  check_executing_commands();
 
   p = arg;
   bnum = get_number (&p);
@@ -8052,6 +8149,12 @@ Multiple breakpoints at one place are pe
 \n\
 Do \"help breakpoints\" for info on other commands dealing with breakpoints."
 
+/* List of subcommands for "commands".  */
+static struct cmd_list_element *commands_command_list;
+
+/* List of subcommands for "edit".  */
+static struct cmd_list_element *edit_command_list;
+
 /* List of subcommands for "catch".  */
 static struct cmd_list_element *catch_cmdlist;
 
@@ -8100,14 +8203,20 @@ Usage is `ignore N COUNT'."));
   if (xdb_commands)
     add_com_alias ("bc", "ignore", class_breakpoint, 1);
 
-  add_com ("commands", class_breakpoint, commands_command, _("\
+  add_prefix_cmd ("commands", class_breakpoint, commands_command, _("\
 Set commands to be executed when a breakpoint is hit.\n\
 Give breakpoint number as argument after \"commands\".\n\
 With no argument, the targeted breakpoint is the last one set.\n\
 The commands themselves follow starting on the next line.\n\
 Type a line containing \"end\" to indicate the end of them.\n\
 Give \"silent\" as the first line to make the breakpoint silent;\n\
-then no output is printed when it is hit, except what the commands print."));
+then no output is printed when it is hit, except what the commands print."),
+		      &commands_command_list, "commands ", 1, &cmdlist);
+
+  add_prefix_cmd ("edit", class_breakpoint, edit_command,_("\
+Edit the command using the external-editor variable, EDITOR environment\n\
+variable or /bin/ex, in that precedence."),
+		      &edit_command_list, "command edit ", 1, &commands_command_list);
 
   add_com ("condition", class_breakpoint, condition_command, _("\
 Specify breakpoint number N to break only if COND is true.\n\
diff -upr OLD/src/gdb/defs.h NEW/src/gdb/defs.h
--- OLD/src/gdb/defs.h	2009-01-14 03:10:28.000000000 -0200
+++ NEW/src/gdb/defs.h	2009-01-19 08:57:04.000000000 -0200
@@ -330,6 +330,8 @@ extern int subset_compare (char *, char 
 
 extern char *safe_strerror (int);
 
+extern char *external_editor( void );
+
 #define	ALL_CLEANUPS	((struct cleanup *)0)
 
 extern void do_cleanups (struct cleanup *);
diff -upr OLD/src/gdb/doc/gdb.texinfo NEW/src/gdb/doc/gdb.texinfo
--- OLD/src/gdb/doc/gdb.texinfo	2009-01-14 03:10:27.000000000 -0200
+++ NEW/src/gdb/doc/gdb.texinfo	2009-01-19 09:26:37.000000000 -0200
@@ -3976,6 +3976,8 @@ follow it immediately with @code{end}; t
 With no @var{bnum} argument, @code{commands} refers to the last
 breakpoint, watchpoint, or catchpoint set (not to the breakpoint most
 recently encountered).
+@item commands edit @r{[}@var{bnum}@r{]}
+This spawns an external editor for adding or editing commands.  The final  @code{end} is not necessary in this case.  @xref{Choosing your Editor}.
 @end table
 
 Pressing @key{RET} as a means of repeating the last @value{GDBN} command is
@@ -5384,6 +5386,7 @@ prefer to use Emacs facilities to view s
 * List::                        Printing source lines
 * Specify Location::            How to specify code locations
 * Edit::                        Editing source files
+* Choosing your Editor::        Specifying your text editor
 * Search::                      Searching source files
 * Source Path::                 Specifying source directories
 * Machine Code::                Source and machine code
@@ -5587,6 +5590,7 @@ Edit the file containing @var{function} 
 
 @end table
 
+@node Choosing your Editor
 @subsection Choosing your Editor
 You can customize @value{GDBN} to use any editor you want
 @footnote{
@@ -5611,6 +5615,19 @@ or in the @code{csh} shell,
 setenv EDITOR /usr/bin/vi
 gdb @dots{}
 @end smallexample
+Another option is to use the @code{set external-editor} command:
+
+@table @code
+@item set external-editor
+@kindex set external-editor
+This command controls the external text editor that internal gdb commands use. The external-editor variable has precedence over the @code{EDITOR} enviroment variable.
+@end table
+
+@table @code
+@item show external-editor
+@kindex show external-editor
+This command shows the external text editor that internal gdb commands use. 
+@end table
 
 @node Search
 @section Searching Source Files
diff -upr OLD/src/gdb/doc/refcard.tex NEW/src/gdb/doc/refcard.tex
--- OLD/src/gdb/doc/refcard.tex	2009-01-14 03:10:27.000000000 -0200
+++ NEW/src/gdb/doc/refcard.tex	2009-01-19 08:57:19.000000000 -0200
@@ -355,10 +355,9 @@ delete when reached
 ignore {\it n} {\it count}&ignore breakpoint {\it n}, {\it count}
 times\cr
 \cr
-commands {\it n}\par
+commands \opt{{\it edit}} {\it n}\par
 \qquad \opt{\tt silent}\par
-\qquad {\it command-list}&execute GDB {\it command-list} every time breakpoint {\it n} is reached. \opt{{\tt silent} suppresses default
-display}\cr
+\qquad {\it command-list}&execute GDB {\it command-list} every time breakpoint {\it n} is reached  \opt{{\tt edit} using external editor}. \opt{{\tt silent} suppresses default display} \cr
 end&end of {\it command-list}\cr
 \endsec
 
diff -upr OLD/src/gdb/utils.c NEW/src/gdb/utils.c
--- OLD/src/gdb/utils.c	2009-01-14 03:10:27.000000000 -0200
+++ NEW/src/gdb/utils.c	2009-01-19 09:25:19.000000000 -0200
@@ -96,6 +96,10 @@ static void prompt_for_continue (void);
 static void set_screen_size (void);
 static void set_width (void);
 
+/* External text editor */
+
+static char *external_editor_command = NULL;
+
 /* A flag indicating whether to timestamp debugging messages.  */
 
 static int debug_timestamp = 0;
@@ -2694,6 +2698,12 @@ When set, debugging messages will be mar
 			   NULL,
 			   show_debug_timestamp,
 			   &setdebuglist, &showdebuglist);
+  add_setshow_filename_cmd ("external-editor", no_class, &external_editor_command, _("\
+Set the external text editor that gdb uses."),
+			    _("\
+Show the external text editor."), NULL,
+			    NULL, NULL,
+			    &setlist, &showlist);
 }
 
 /* Machine specific function to handle SIGWINCH signal. */
@@ -3443,3 +3453,16 @@ gdb_buildargv (const char *s)
     nomem (0);
   return argv;
 }
+
+/* Returns the external editor */
+char *
+external_editor (void)
+{
+  char *editor;
+  if (external_editor_command)
+    return external_editor_command;
+  if ((editor = (char *) getenv ("EDITOR")) == NULL)
+    editor = "/bin/ex";
+  return editor;
+
+}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]