Re: Octave-maintainers Digest, Vol 84, Issue 21

[Daniel J Sebald]

On 03/09/2013 04:54 PM, Rik wrote:

> > On 03/08/2013 09:48 PM, Daniel J Sebald wrote:
[snip]
> > Oh, run_history, with an underscore.  Yes, I see there is a problem
> > there.  I'm inclined to say recursive implementation, but could that run
> > into stack issues thereby requiring some error checking also or leave it
> > open to potentially crash?  Have to think this one over.
> >
> > I point out that the help documentation for "run_history" could be
> > improved as well.  Anyway, there may be a bug in the run_history whereby
> > the most recent history entry can't be accessed by number:
> >
> > octave:24>  history -5
> >    1048 1
> >    1049 2
> >    1050 3
> >    1051 4
> >    1052 history -5
> > octave:25>  run_history 1052
> > error: run_history: history specification out of range
> >
> > The second most recent can be accessed though:
> >
> > octave:29>  history -5
> >    1054 6
> >    1055 7
> >    1056 8
> >    1057 9
> >    1058 history -5
> > octave:30>  run_history 1057
> > ans =  9
> >
> > Lastly, the non-argument version of "history" doesn't seem of much use.
> >    Is the following what it is supposed to do?
> >
> > octave:34>  history
> >    1063 history
> > octave:35>  history
> >    1064 history
> >
> > I would think that a full listing of history would be a more useful
> > thing to display, or at least default the list to, say, 25 entries.
> >
> > Dan
> 3/9/13
>
> Dan,
>
> Which Mercurial changeset are you running?  I improved both the
> documentation and fixed the off-by-1 error, and it was while coding there
> that I noticed this issue.
>
> history with no arguments used to return all history starting from command
> #1.  I think that is a regression although I didn't touch that part of the
> code.  I'll take a look though.

Rik,

I've updated to the latest changesets.  I see that the most recent 
history list entry can be accessed by "run_history".  I see that instead 
of 1024 as being the file-saved history length, it now starts up with 
1000.  However, the default displayed list length of 1 is still evident.

I looked at oct-hist.cc.  The list length is pretty clear in the 
"do_history" command:

  // Number of history lines to show
  int limit = -1;

  [handle arguments]

  if (limit < 0)
    limit = -limit;

So, if no arguments, the default length is 1.  There is a conflict here 
because a -1 means the same as 1, -2 means the same as 2, so on.  After 
going through the for loop, there is no way to tell that the number of 
arguments related to list length is zero.  It would have been nicer to 
have another degree of freedom by introducing a variable like 
"limit_set" to condition on once past the for-loop.

I'd have written a changeset but I don't know what we want to do.  We 
could make the easy change

  // Number of history lines to show
  int limit = -50;

or similar and see if

  hlist = command_history::list (limit, numbered_output);

does the proper boundary checking (i.e., if limit is actually bigger 
than the list, there is no segmentation fault).

Or we could go for the whole list with

  if (limit_set)
    hlist = command_history::list (limit, numbered_output);
  else
    hlist = command_history::list (command_history.length (), 
numbered_output);

approach.

Dan