Re: [PATCH] local history support, take 2

[Wayne Davison <wayned@xxxxxxxxxxxxxxxxxxxxx>]

On Wed, May 19, 2004 at 10:37:50PM +0100, Peter Stephenson wrote:
> Probably allowing it with both would be better, since `history' is a more
> natural command.

I agree that the command name reads better, but since it has always
implied listing of history entries, it seems a little weird to change
that.  I went ahead and added the -p & -P options to the history
builtin, but I didn't document it yet.

> Looks essentially OK to me as it is, it can be enhanced later if we
> feel like it.

OK, I checked it in.  I also started to work on an auto-pop feature
from a function.  Attached is a patch that almost works right, but has a
couple areas that need to be improved.  Perhaps someone can chime in
here with some advice on how to either do this the right way or fix the
problems.

I chose to add my pop-checking hook into level() -- the routine that
decrements locallevel.  One problem I encountered is that this routine
is called on function exit _before_ the traps are run.  This means that
if someone puts a manual "fc -P" call into a trap, it will try to do
an extra pop after the auto-pop for leaving the function.  I suppose we
can just tell people not to do that...

The other problem I noticed is how the saving of the parameters
interacts with local variables.  Take these two functions for instance:

function foo {
    local HISTSIZE=2221
    fc -p ~/.newhist 2222 2222
    echo $HISTFILE $HISTSIZE
}

function bar {
    fc -p
    local HISTFILE=~/.newhist HISTSIZE=2223
    echo $HISTFILE $HISTSIZE
}

In function foo the history-list push saves off the 2221 from the local
variable, so we have to be sure to call the history-list pop function
before removing the local variables (which is what I currently do).

However, in function bar the history-list push saves off the global
value of the HISTFILE and HISTSIZE variables, which are then made local.
When the function exits, the history-list pop first restores the global
values of the variables to the environment, and then the local-variable
restoration unsets HISTFILE and makes HISTSIZE=30 (the default value).

Is this something we can easily fix?  Perhaps with a different way of
saving the environment values?  Or is this something where we tell users
that they can't both use "fc -p" and make the variables local?

..wayne..

--- Functions/Misc/zcalc	20 May 2004 22:23:11 -0000	1.11
+++ Functions/Misc/zcalc	21 May 2004 01:22:44 -0000
@@ -89,13 +89,6 @@ setopt extendedglob
 # push to our own history file
 fc -p ~/.zcalc_history
 
-zcalc_restore() {
-    unfunction zcalc_restore
-    # pop back to original history
-    fc -P
-}
-trap zcalc_restore HUP INT QUIT EXIT
-
 local line ans base defbase forms match mbegin mend psvar optlist opt arg
 local compcontext="-math-"
 integer num outdigits outform=1
--- Src/builtin.c	20 May 2004 22:22:43 -0000	1.119
+++ Src/builtin.c	21 May 2004 01:22:45 -0000
@@ -4110,7 +4110,7 @@ zexit(int val, int from_where)
     }
     if (isset(RCS) && interact) {
 	if (!nohistsave) {
-	    saveandpophiststack(0);
+	    saveandpophiststack(1);
 	    savehistfile(NULL, 1, HFILE_USE_OPTIONS);
 	}
 	if (islogin && !subsh) {
--- Src/hist.c	20 May 2004 22:23:02 -0000	1.50
+++ Src/hist.c	21 May 2004 01:36:08 -0000
@@ -1807,6 +1807,7 @@ static struct histsave {
     int histlinect;
     int histsiz;
     int savehistsiz;
+    int locallevel;
 } *histsave_stack;
 static int histsave_stack_size = 0;
 static int histsave_stack_pos = 0;
@@ -2374,6 +2375,7 @@ pushhiststack(char *hf, int hs, int shs)
     h->histlinect = histlinect;
     h->histsiz = histsiz;
     h->savehistsiz = savehistsiz;
+    h->locallevel = locallevel;
 
     memset(&lasthist, 0, sizeof lasthist);
     if (hf) {
@@ -2433,14 +2435,26 @@ pophiststack(void)
     return histsave_stack_pos + 1;
 }
 
+/* If down_through > 0, pop all array items >= the 1-relative index value.
+ * If down_through < 0, pop (-1)*down_through levels off the stack.
+ * If down_through == 0, pop any stack items from a higher locallevel.
+ */
+
 /**/
 int
 saveandpophiststack(int down_through)
 {
-    if (down_through < 0)
+    if (!down_through) {
+	down_through = histsave_stack_pos;
+	while (down_through > 0
+	 && histsave_stack[down_through-1].locallevel > locallevel)
+	    down_through--;
+	down_through++;
+    } else if (down_through < 0) {
 	down_through += histsave_stack_pos + 1;
-    if (down_through <= 0)
-	down_through = 1;
+	if (down_through <= 0)
+	    down_through = 1;
+    }
     if (histsave_stack_pos < down_through)
 	return 0;
     do {
--- Src/params.c	6 Apr 2004 13:01:10 -0000	1.81
+++ Src/params.c	21 May 2004 01:22:49 -0000
@@ -3583,6 +3583,7 @@ mod_export void
 endparamscope(void)
 {
     locallevel--;
+    saveandpophiststack(0); /* Pops anything from a higher locallevel */
     scanhashtable(paramtab, 0, 0, 0, scanendscope, 0);
 }