Re: clumsy patch for h = history (n)

[John W. Eaton]

On 19-Dec-2012, Ben Abbott wrote:

| Perhaps the return cell-strings shouldn' t include the indices?

Oops.  I'm attaching an updated version of the changeset.

jwe

# HG changeset patch
# User John W. Eaton <address@hidden>
# Date 1355970302 18000
# Node ID 565a6f16ba79b5fb46464dbde3c2f08be2abb375
# Parent  1e9a6285acc46cf68168725b0b08e38b8475052e
allow history commands to take numeric arguments

* oct-hist.cc (do_history, mk_tmp_hist_file, do_edit_history,
do_runt_history): Accept args as octave_value_list instead of
string_vector.  Handle numeric and character string inputs.
(get_int_arg): New static function.
(Fhistory): Pass args and nargout to do_history instead of converting
args to a string_vector.
(Fedit_history): Pass args do_edit_history instead of converting
args to a string_vector.
(Frun_history): Likewise, for do_run_history.

diff --git a/libinterp/interpfcn/oct-hist.cc b/libinterp/interpfcn/oct-hist.cc
--- a/libinterp/interpfcn/oct-hist.cc
+++ b/libinterp/interpfcn/oct-hist.cc
@@ -129,26 +129,51 @@
 // means only display that many items.
 
 static string_vector
-do_history (int argc, const string_vector& argv, bool output = true)
+do_history (const octave_value_list& args, int nargout)
 {
-  int numbered_output = 1;
+  bool numbered_output = nargout == 0;
 
   unwind_protect frame;
+
   string_vector hlist;
 
   frame.add_fcn (command_history::set_file, command_history::file ());
 
+  int nargin = args.length ();
+
   // Number of history lines to show
-  std::string N;
-  for (int i = 1; i < argc; i++)
+  int limit = -1;
+
+  for (octave_idx_type i = 0; i < nargin; i++)
     {
-      std::string option = argv[i];
+      octave_value arg = args(i);
+
+      std::string option;
+
+      if (arg.is_string ())
+        option = arg.string_value ();
+      else if (arg.is_numeric_type ())
+        {
+          limit = arg.int_value ();
+          continue;
+        }
+      else
+        {
+          gripe_wrong_type_arg ("history", arg);
+          return hlist;
+        }
 
       if (option == "-r" || option == "-w" || option == "-a"
           || option == "-n")
         {
-          if (i < argc - 1)
-            command_history::set_file (argv[i+1]);
+          if (i < nargin - 1 && args(i+1).is_string ())
+            command_history::set_file (args(++i).string_value ());
+          else
+            {
+              error ("history: expecting file name for %s option",
+                     option.c_str ());
+              return hlist;
+            }
 
           if (option == "-a")
             // Append 'new' lines to file.
@@ -171,42 +196,45 @@
 
           return hlist;
         }
-      else if (argv[i] == "-q")
-        numbered_output = 0;
-      else if (argv[i] == "--")
+      else if (option == "-q")
+        numbered_output = false;
+      else if (option == "--")
         {
           i++;
           break;
         }
       else
-        // The last argument found in the command list that looks like
-        // an integer will be used
-        N = argv[i];
-    }
+        {
+          // The last argument found in the command list that looks like
+          // an integer will be used
+          int tmp;
 
-  int limit = -1;
+          if (sscanf (option.c_str (), "%d", &tmp) == 1)
+            limit = tmp;
+          else
+            {
+              if (option.length () > 0 && option[0] == '-')
+                error ("history: unrecognized option '%s'", option.c_str ());
+              else
+                error ("history: bad non-numeric arg '%s'", option.c_str ());
 
-  if (N != "" && sscanf (N.c_str (), "%d", &limit) != 1)
-    {
-      if (N[0] == '-')
-        error ("history: unrecognized option '%s'", N.c_str ());
-      else
-        error ("history: bad non-numeric arg '%s'", N.c_str ());
-
-      return hlist;
+              return  hlist;
+            }
+        }
     }
 
   if (limit < 0)
     limit = -limit;
 
-
   hlist = command_history::list (limit, numbered_output);
 
   int len = hlist.length ();
 
-  if (output)
-    for (int i = 0; i < len; i++)
-      octave_stdout << hlist[i] << "\n";
+  if (nargout == 0)
+    {
+      for (octave_idx_type i = 0; i < len; i++)
+        octave_stdout << hlist[i] << "\n";
+    }
 
   return hlist;
 }
@@ -319,9 +347,28 @@
     }
 }
 
+static bool
+get_int_arg (const octave_value& arg, int& val)
+{
+  bool ok = true;
+
+  if (arg.is_string ())
+    {
+      std::string tmp = arg.string_value ();
+
+      ok = sscanf (tmp.c_str (), "%d", &val) == 1;
+    }
+  else if (arg.is_numeric_type ())
+    val = arg.int_value ();
+  else
+    ok = false;
+
+  return ok;
+}
+
 static std::string
-mk_tmp_hist_file (int argc, const string_vector& argv,
-                  int insert_curr, const char *warn_for)
+mk_tmp_hist_file (const octave_value_list& args,
+                  bool insert_curr, const char *warn_for)
 {
   std::string retval;
 
@@ -344,31 +391,34 @@
 
   int hist_end = hist_count;
   int hist_beg = hist_count;
-  int reverse = 0;
+
+  bool reverse = false;
 
   // Process options.
 
-  int usage_error = 0;
-  if (argc == 3)
+  int nargin = args.length ();
+
+  bool usage_error = false;
+  if (nargin == 2)
     {
-      if (sscanf (argv[1].c_str (), "%d", &hist_beg) != 1
-          || sscanf (argv[2].c_str (), "%d", &hist_end) != 1)
-        usage_error = 1;
-      else
+      if (get_int_arg (args(0), hist_beg)
+          && get_int_arg (args(1), hist_end))
         {
           hist_beg--;
           hist_end--;
         }
+      else
+        usage_error = true;
     }
-  else if (argc == 2)
+  else if (nargin == 1)
     {
-      if (sscanf (argv[1].c_str (), "%d", &hist_beg) != 1)
-        usage_error = 1;
-      else
+      if (get_int_arg (args(0), hist_beg))
         {
           hist_beg--;
           hist_end = hist_beg;
         }
+      else
+        usage_error = true;
     }
 
   if (hist_beg < 0 || hist_end < 0 || hist_beg > hist_count
@@ -389,7 +439,7 @@
       int t = hist_end;
       hist_end = hist_beg;
       hist_beg = t;
-      reverse = 1;
+      reverse = true;
     }
 
   std::string name = octave_tempnam ("", "oct-");
@@ -426,9 +476,9 @@
 }
 
 static void
-do_edit_history (int argc, const string_vector& argv)
+do_edit_history (const octave_value_list& args)
 {
-  std::string name = mk_tmp_hist_file (argc, argv, 0, "edit_history");
+  std::string name = mk_tmp_hist_file (args, false, "edit_history");
 
   if (name.empty ())
     return;
@@ -494,9 +544,9 @@
 }
 
 static void
-do_run_history (int argc, const string_vector& argv)
+do_run_history (const octave_value_list& args)
 {
-  std::string name = mk_tmp_hist_file (argc, argv, 1, "run_history");
+  std::string name = mk_tmp_hist_file (args, true, "run_history");
 
   if (name.empty ())
     return;
@@ -575,14 +625,7 @@
 {
   octave_value_list retval;
 
-  int argc = args.length () + 1;
-
-  string_vector argv = args.make_argv ("edit_history");
-
-  if (error_state)
-    return retval;
-
-  do_edit_history (argc, argv);
+  do_edit_history (args);
 
   return retval;
 }
@@ -620,25 +663,12 @@
 argument as a cell string and will not be output to screen.\n\
 @end deftypefn")
 {
-  octave_value_list retval;
+  octave_value retval;
 
-  int argc = args.length () + 1;
+  string_vector hlist = do_history (args, nargout);
 
-  string_vector argv = args.make_argv ("history");
-
-  if (error_state)
-    return retval;
-
-  string_vector hlist;
   if (nargout > 0)
-    {
-      argv.append (std::string ("-q"));
-      argc++;
-      hlist = do_history (argc, argv, false);
-      retval(0) = Cell (hlist);
-    }
-  else
-    do_history (argc, argv, true);
+    retval = Cell (hlist);
 
   return retval;
 }
@@ -653,14 +683,7 @@
 {
   octave_value_list retval;
 
-  int argc = args.length () + 1;
-
-  string_vector argv = args.make_argv ("run_history");
-
-  if (error_state)
-    return retval;
-
-  do_run_history (argc, argv);
+  do_run_history (args);
 
   return retval;
 }