Re: PATCH: History bug with "print -s"

Re: PATCH: History bug with "print -s"

[Sven Wischnowsky]

Wayne Davison wrote:

> Appended is a patch that should fix the funkiness associated with
> running "print -s" while zle is active.  I've done some very basic
> testing, and it appears to work fine.
> 
> However, if "print -s" is supposed to make the line immediately
> available for history browsing (before pressing return), that is not
> happening.  I can check into this next, but I don't have time to do
> that right now.

Yes, that's what Bart wanted. And it would be nice to have, wouldn't it?

> The following diff is based on an unpatched 3.1.7-pre-2.

I've committed it.

Bye
 Sven


--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: Re: History bug (Re: Completion debugging)

[Bart Schaefer]

On May 4,  5:34pm, Sven Wischnowsky wrote:
} Subject: Re: PATCH: Re: History bug (Re: Completion debugging)
}
} There is so much mucking around curhist (and histline) in zle that I
} wonder if there is a clean way (other than changing the meaning of
} curhist in the core and then adapting zle).

Cf. "The one icky bit" in my "Dear old literal history" message.

} Maybe we should just make `print -s' put the strings added while zle
} is active into some list and then add those strings only after zle is
} left? I.e. in a toplevel-loop() or somewhere around that.

That's approximately what "print -z" does.  At least for _complete_debug,
the whole point is to get them into the history while you can still scroll
around and look at them.

Unfortunately that means either putting them into the future history or
changing the event number of the current command.  Neither works, at
least not easily, because the history is now stored as a ring rather
than as a simple linked list.  Wayne, are you out there?

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

PATCH: History bug with "print -s"

[Wayne Davison]

Appended is a patch that should fix the funkiness associated with
running "print -s" while zle is active.  I've done some very basic
testing, and it appears to work fine.

However, if "print -s" is supposed to make the line immediately
available for history browsing (before pressing return), that is not
happening.  I can check into this next, but I don't have time to do
that right now.

The following diff is based on an unpatched 3.1.7-pre-2.

..wayne..

---8<------8<------8<------8<---cut here--->8------>8------>8------>8---
Index: Src/builtin.c
@@ -2777,7 +2777,7 @@
 	int nwords = 0, nlen, iwords;
 	char **pargs = args;
 
-	ent = prepnexthistent(++curhist);
+	ent = prepnexthistent();
 	while (*pargs++)
 	    nwords++;
 	if ((ent->nwords = nwords)) {
Index: Src/hist.c
@@ -704,6 +704,36 @@
 {
 }
 
+/* these functions handle adding/removing curline to/from the hist_ring */
+
+static void
+linkcurline(void)
+{
+    if (!hist_ring)
+	hist_ring = curline.up = curline.down = &curline;
+    else {
+	curline.up = hist_ring;
+	curline.down = hist_ring->down;
+	hist_ring->down = hist_ring->down->up = &curline;
+	hist_ring = &curline;
+    }
+    curline.histnum = ++curhist;
+}
+
+static void
+unlinkcurline(void)
+{
+    curline.up->down = curline.down;
+    curline.down->up = curline.up;
+    if (hist_ring == &curline) {
+	if (!histlinect)
+	    hist_ring = NULL;
+	else
+	    hist_ring = curline.up;
+    }
+    curhist--;
+}
+
 /* initialize the history mechanism */
 
 /**/
@@ -745,15 +775,7 @@
     if (interact && isset(SHINSTDIN) && !strin) {
 	histactive = HA_ACTIVE;
 	attachtty(mypgrp);
-	if (!hist_ring)
-	    hist_ring = curline.up = curline.down = &curline;
-	else {
-	    curline.up = hist_ring;
-	    curline.down = hist_ring->down;
-	    hist_ring->down = hist_ring->down->up = &curline;
-	    hist_ring = &curline;
-	}
-	curline.histnum = ++curhist;
+	linkcurline();
 	defev = addhistnum(curhist, -1, HIST_FOREIGN);
     } else
 	histactive = HA_ACTIVE | HA_NOINC;
@@ -883,9 +905,13 @@
 
 /**/
 Histent
-prepnexthistent(int histnum)
+prepnexthistent(void)
 {
     Histent he;
+    int curline_in_ring = hist_ring == &curline;
+
+    if (curline_in_ring)
+	unlinkcurline();
 
     if (histlinect < histsiz) {
 	he = (Histent)zcalloc(sizeof *he);
@@ -920,8 +946,10 @@
 	}
 	freehistdata(hist_ring = he, 0);
     }
-    hist_ring->histnum = histnum;
-    return hist_ring;
+    he->histnum = ++curhist;
+    if (curline_in_ring)
+	linkcurline();
+    return he;
 }
 
 /* say we're done using the history mechanism */
@@ -937,17 +965,8 @@
 	  "BUG: chline is NULL in hend()");
     if (histdone & HISTFLAG_SETTY)
 	settyinfo(&shttyinfo);
-    if (!(histactive & HA_NOINC)) {
-	curline.up->down = curline.down;
-	curline.down->up = curline.up;
-	if (hist_ring == &curline) {
-	    if (!histlinect)
-		hist_ring = NULL;
-	    else
-		hist_ring = curline.up;
-	}
-	curhist--;
-    }
+    if (!(histactive & HA_NOINC))
+	unlinkcurline();
     if (histactive & (HA_NOSTORE|HA_NOINC)) {
 	zfree(chline, hlinesz);
 	zfree(chwords, chwordlen*sizeof(short));
@@ -1023,7 +1042,7 @@
 	    freehistdata(he, 0);
 	} else {
 	    keepflags = 0;
-	    he = prepnexthistent(++curhist);
+	    he = prepnexthistent();
 	}
 
 	he->text = ztrdup(chline);
@@ -1777,7 +1796,7 @@
 		lasthist.stim = stim;
 	    }
 
-	    he = prepnexthistent(++curhist);
+	    he = prepnexthistent();
 	    he->text = ztrdup(pt);
 	    he->flags = newflags;
 	    if ((he->stim = stim) == 0)
---8<------8<------8<------8<---cut here--->8------>8------>8------>8---