history problems

history problems

[Vin Shelton]

I've been off on vacation for a week and haven't yet had a chance to
catch up on my zsh email, so let me apologize in advance if this has
already been discussed.

With a current zsh built from CVS, I notice the following problem:

zsh -f
zion% ls

... output omitted ...

zion% history
    1  ls
zion% history -100
fc: bad history number: 0

It used to be that 'history -100' would return the same results as
'history' in this case.

I ran into this problem because .zlogout records my shell histories to
separate files upon logout.  The following command only works if at
least $HISTSIZE commands have been run:

fc -nldf -$HISTSIZE >> $outfile

This used to save up to $HISTSIZE commands, but now it generates:

fc: bad history number: 0

if fewer than $HISTSIZE commands have been executed in the shell

Thanks in advance,
  - vin

Re: history problems

[Bart Schaefer]

On Sep 16,  2:16pm, Vin Shelton wrote:
}
} zion% history
}     1  ls
} zion% history -100
} fc: bad history number: 0

This is an unintended side-effect of Wayne's patch in 15806.  One
possible fix is appended; Wayne may have a better idea.

Even with this patch, `curline.histnum' appears occasionally to be wrong:

schaefer<507> fc -l -1 506
  506  h
schaefer<508> fc -l -1 508
fc: bad history number: 508
schaefer<509> fc -l -1 508
fc: bad history number: 508

The second of those should have succeeded, I think.

Index: Src/builtin.c
===================================================================
diff -c -r1.7 builtin.c
--- Src/builtin.c	2001/09/15 19:16:25	1.7
+++ Src/builtin.c	2001/09/16 21:27:59
@@ -1268,7 +1268,7 @@
     /* interpret and check first history line specifier */
     if (*argv) {
 	minflag = **argv == '-';
-	first = fcgetcomm(*argv);
+	first = fcgetcomm(*argv, ops);
 	if (first == -1) {
 	    unqueue_signals();
 	    return 1;
@@ -1277,7 +1277,7 @@
     }
     /* interpret and check second history line specifier */
     if (*argv) {
-	last = fcgetcomm(*argv);
+	last = fcgetcomm(*argv, ops);
 	if (last == -1) {
 	    unqueue_signals();
 	    return 1;
@@ -1359,7 +1359,7 @@
 
 /**/
 static int
-fcgetcomm(char *s)
+fcgetcomm(char *s, char *ops)
 {
     int cmd;
 
@@ -1368,7 +1368,7 @@
     if ((cmd = atoi(s))) {
 	if (cmd < 0)
 	    cmd = addhistnum(curline.histnum,cmd,HIST_FOREIGN);
-	if (cmd < firsthist()) {
+	if (cmd >= curline.histnum || (!ops['l'] && cmd < firsthist())) {
 	    zwarnnam("fc", "bad history number: %d", 0, cmd);
 	    return -1;
 	}


-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: history problems

[Wayne Davison]

On Sun, 16 Sep 2001, Bart Schaefer wrote:
> This is an unintended side-effect of Wayne's patch in 15806.  One
> possible fix is appended; Wayne may have a better idea.

Here's my suggested fix.

Since the fcgetcomm() function is only called when doing a history list,
I don't see why the function is complaining if the numeric values are
out of bounds -- the list function fixes these up to be valid already.
So, I changed the function to just limit the lower range so that it
couldn't return a -1.  I also allow the user to be able to type a
history number of 0.

Also, I can't find any reason for the "minflag" code to exist.  Way back
before I started changing things, it looks to me like the code that used
the minflag value could never get executed.  So, I've removed minflag.

The end result is that the user can now type invalid values and have
them get rounded off.  For instance, "history 0 99999" will output the
entire history buffer, as will "history -99999 99999".  (Older zsh
versions would reject this as invalid because of the too-high end
value.)  Someone wanting to output a single line would use something
like this: "history -20 1" (since the ending value will get rounded up
to the first value).

Anyone believe that this should work differently?

..wayne..

---8<------8<------8<------8<---cut here--->8------>8------>8------>8---
Index: Src/builtin.c
--- Src/builtin.c	2001/09/13 18:19:08	1.52
+++ Src/builtin.c	2001/09/17 17:54:45
@@ -1208,7 +1208,7 @@
 int
 bin_fc(char *nam, char **argv, char *ops, int func)
 {
-    int first = -1, last = -1, retval, minflag = 0;
+    int first = -1, last = -1, retval;
     char *s;
     struct asgment *asgf = NULL, *asgl = NULL;
     Patprog pprog = NULL;
@@ -1267,7 +1267,6 @@
     }
     /* interpret and check first history line specifier */
     if (*argv) {
-	minflag = **argv == '-';
 	first = fcgetcomm(*argv);
 	if (first == -1) {
 	    unqueue_signals();
@@ -1299,9 +1298,9 @@
     if (last == -1)
 	last = ops['l']? addhistnum(curline.histnum,-1,0) : first;
     if (first < firsthist())
-	first = firsthist() - (first == last);
+	first = firsthist() - (last < firsthist());
     if (last > curhist)
-	last = (minflag) ? curhist : first;
+	last = curhist;
     else if (last < first)
 	last = first;
     if (ops['l']) {
@@ -1365,13 +1364,11 @@

     /* First try to match a history number.  Negative *
      * numbers indicate reversed numbering.           */
-    if ((cmd = atoi(s))) {
+    if ((cmd = atoi(s)) != 0 || *s == '0') {
 	if (cmd < 0)
 	    cmd = addhistnum(curline.histnum,cmd,HIST_FOREIGN);
-	if (cmd < firsthist()) {
-	    zwarnnam("fc", "bad history number: %d", 0, cmd);
-	    return -1;
-	}
+	if (cmd < 0)
+	    cmd = 0;
 	return cmd;
     }
     /* not a number, so search by string */
---8<------8<------8<------8<---cut here--->8------>8------>8------>8---

Re: history problems

[Vin Shelton]

This works fine for me, and I think the proposed semantic change is
fine.

  - vin

Wayne Davison <wayned@users.sourceforge.net> writes:

> On Sun, 16 Sep 2001, Bart Schaefer wrote:
> > This is an unintended side-effect of Wayne's patch in 15806.  One
> > possible fix is appended; Wayne may have a better idea.
> 
> Here's my suggested fix.
> 
> Since the fcgetcomm() function is only called when doing a history list,
> I don't see why the function is complaining if the numeric values are
> out of bounds -- the list function fixes these up to be valid already.
> So, I changed the function to just limit the lower range so that it
> couldn't return a -1.  I also allow the user to be able to type a
> history number of 0.
> 
> Also, I can't find any reason for the "minflag" code to exist.  Way back
> before I started changing things, it looks to me like the code that used
> the minflag value could never get executed.  So, I've removed minflag.
> 
> The end result is that the user can now type invalid values and have
> them get rounded off.  For instance, "history 0 99999" will output the
> entire history buffer, as will "history -99999 99999".  (Older zsh
> versions would reject this as invalid because of the too-high end
> value.)  Someone wanting to output a single line would use something
> like this: "history -20 1" (since the ending value will get rounded up
> to the first value).
> 
> Anyone believe that this should work differently?
> 
> ..wayne..