Re: Proposed history improvements

[Wayne Davison <wayned@xxxxxxxxxxxxxxxxxxxxx>]

Thanks for the feedback.  Since the idea for the lingering-history-drop
is fairly simple, let's separate that one out.  Here's a new patch that
just adds this functionality, and also adds a new option called
"hist_immediate_drop" that will restore the old behavior, should the
user decide to turn it on.

Comments?

After I test this for a while longer, I'd be comfortable committing this
to CVS.  If we're further along in the release cycle where that wouldn't
be a good idea, let me know.

..wayne..

---8<------8<------8<------8<---cut here--->8------>8------>8------>8---
Index: Doc/Zsh/options.yo
@@ -544,18 +544,39 @@
 pindex(HIST_IGNORE_SPACE)
 cindex(history, ignoring spaces)
 item(tt(HIST_IGNORE_SPACE) (tt(-g)))(
-Do not enter command lines into the history list
-if the first character on the line is a space, or if one of
-the expanded aliases contained a leading space.
+Remove command lines from the history list when the first character on
+the line is a space, or when one of the expanded aliases contains a
+leading space.
+If tt(HIST_IMMEDIATE_DROP) is set, the line is dropped from the
+history right away, otherwise the command lingers in the internal
+history until the next command is entered before being dropped
+(allowing you to briefly reuse and/or edit the line).
 )
+pindex(HIST_IMMEDIATE_DROP)
+cindex(history, dropping lines)
+item(tt(HIST_IMMEDIATE_DROP))(
+When this option is set, dropped history lines are immediately lost.
+By default, the to-be-dropped history line lingers until the next
+command is typed, allowing you to briefly reuse and/or edit it.
+See also tt(HIST_IGNORE_SPACE), tt(HIST_NO_FUNCTIONS), and
+tt(HIST_NO_STORE).
+)
 pindex(HIST_NO_FUNCTIONS)
 item(tt(HIST_NO_FUNCTIONS))(
-Do not store function definitions in the history list.
+Remove function definitions from the history list.
+If tt(HIST_IMMEDIATE_DROP) is set, the line is dropped from the
+history right away, otherwise the function lingers in the internal
+history until the next command is entered before being dropped
+(allowing you to briefly reuse and/or edit the line).
 )
 pindex(HIST_NO_STORE)
 item(tt(HIST_NO_STORE))(
-Remove the tt(history) (tt(fc -l)) command from
-the history when invoked.
+Remove the tt(history) (tt(fc -l)) command from the history list
+when invoked.
+If tt(HIST_IMMEDIATE_DROP) is set, the line is dropped from the
+history right away, otherwise the command lingers in the internal
+history until the next command is entered before being dropped
+(allowing you to briefly reuse and/or edit the line).
 )
 pindex(HIST_REDUCE_BLANKS)
 item(tt(HIST_REDUCE_BLANKS))(
Index: Src/hashtable.c
@@ -1480,10 +1480,11 @@
     HashNode oldnode = addhashnode2(ht, nam, nodeptr);
     Histent he = (Histent)nodeptr;
     if (oldnode && oldnode != (HashNode)nodeptr) {
-	if (he->flags & HIST_MAKEUNIQUE
+	if (he->flags & (HIST_MAKEUNIQUE | HIST_TMPSTORE)
 	 || (he->flags & HIST_FOREIGN && (Histent)oldnode == he->up)) {
+	    (void) addhashnode2(ht, oldnode->nam, oldnode); /* restore hash */
 	    he->flags |= HIST_DUP;
-	    addhashnode(ht, oldnode->nam, oldnode); /* Remove the new dup */
+	    he->flags &= ~HIST_MAKEUNIQUE;
 	}
 	else {
 	    oldnode->flags |= HIST_DUP;
Index: Src/hist.c
@@ -794,16 +794,19 @@
 void
 histreduceblanks(void)
 {
-    int i, len, pos, needblank;
+    int i, len, pos, needblank, spacecount = 0;

-    for (i = 0, len = 0; i < chwordpos; i += 2) {
+    if (isset(HISTIGNORESPACE))
+	while (chline[spacecount] == ' ') spacecount++;
+
+    for (i = 0, len = spacecount; i < chwordpos; i += 2) {
 	len += chwords[i+1] - chwords[i]
 	     + (i > 0 && chwords[i] > chwords[i-1]);
     }
     if (chline[len] == '\0')
 	return;

-    for (i = 0, pos = 0; i < chwordpos; i += 2) {
+    for (i = 0, pos = spacecount; i < chwordpos; i += 2) {
 	len = chwords[i+1] - chwords[i];
 	needblank = (i < chwordpos-2 && chwords[i+2] > chwords[i+1]);
 	if (pos != chwords[i]) {
@@ -1044,8 +1047,10 @@
 	    } else
 		save = 0;
 	}
-	if (chwordpos <= 2 || should_ignore_line(prog))
+	if (chwordpos <= 2)
 	    save = 0;
+	else
+	    save = should_ignore_line(prog)? 2 : 1;
     }
     if (flag & (HISTFLAG_DONE | HISTFLAG_RECALL)) {
 	char *ptr;
@@ -1062,10 +1067,17 @@
 	    save = 0;
 	} else
 	    zsfree(ptr);
+    }
+    if (isset(HISTIGNORESPACE) && (save || *chline == ' ')) {
+	Histent he;
+	for (he = hist_ring; he && he->flags & HIST_FOREIGN;
+	     he = up_histent(he)) ;
+	if (he && he->flags & HIST_TMPSTORE)
+	    freehistnode((HashNode)he);
     }
-    if (save) {
+    if (save == 1 || (save == 2 && !isset(HISTIMMEDIATEDROP))) {
 	Histent he;
-	int keepflags;
+	int newflags;

 	for (he = hist_ring; he && he->flags & hist_skip_flags;
 	     he = up_histent(he)) ;
@@ -1083,6 +1095,7 @@
 	    if (isset(HISTREDUCEBLANKS))
 		histreduceblanks();
 	}
+	newflags = save == 1? 0 : HIST_OLD | HIST_TMPSTORE;
 	if ((isset(HISTIGNOREDUPS) || isset(HISTIGNOREALLDUPS)) && he
 	 && histstrcmp(chline, he->text) == 0) {
 	    /* This history entry compares the same as the previous.
@@ -1090,18 +1103,16 @@
 	     * previous one with the current one.  This also gets the
 	     * timestamp right.  Perhaps, preserve the HIST_OLD flag.
 	     */
-	    keepflags = he->flags & HIST_OLD; /* Avoid re-saving */
+	    newflags |= he->flags & HIST_OLD; /* Avoid re-saving */
 	    freehistdata(he, 0);
 	    curline.histnum = curhist;
-	} else {
-	    keepflags = 0;
+	} else
 	    he = prepnexthistent();
-	}

 	he->text = ztrdup(chline);
 	he->stim = time(NULL);
 	he->ftim = 0L;
-	he->flags = keepflags;
+	he->flags = newflags;

 	if ((he->nwords = chwordpos/2)) {
 	    he->words = (short *)zalloc(chwordpos * sizeof(short));
@@ -1894,8 +1905,10 @@
 	    } else
 		he->words = (short *)NULL;
 	    addhistnode(histtab, he->text, he);
-	    if (hist_ring != he)
-		curhist--; /* We discarded a foreign duplicate */
+	    if (he->flags & HIST_DUP) {
+		freehistnode((HashNode)he);
+		curhist--;
+	    }
 	}
 	if (start && readflags & HFILE_USE_OPTIONS) {
 	    zsfree(lasthist.text);
@@ -1963,7 +1976,8 @@
     if (out) {
 	for (; he && he->histnum <= xcurhist; he = down_histent(he)) {
 	    if ((writeflags & HFILE_SKIPDUPS && he->flags & HIST_DUP)
-	     || (writeflags & HFILE_SKIPFOREIGN && he->flags & HIST_FOREIGN))
+	     || (writeflags & HFILE_SKIPFOREIGN && he->flags & HIST_FOREIGN)
+	     || he->flags & HIST_TMPSTORE)
 		continue;
 	    if (writeflags & HFILE_SKIPOLD) {
 		if (he->flags & HIST_OLD)
Index: Src/options.c
@@ -130,6 +130,7 @@
 {NULL, "histignorealldups",   0,			 HISTIGNOREALLDUPS},
 {NULL, "histignoredups",      0,			 HISTIGNOREDUPS},
 {NULL, "histignorespace",     0,			 HISTIGNORESPACE},
+{NULL, "histimmediatedrop",   0,			 HISTIMMEDIATEDROP},
 {NULL, "histnofunctions",     0,			 HISTNOFUNCTIONS},
 {NULL, "histnostore",	      0,			 HISTNOSTORE},
 {NULL, "histreduceblanks",    0,			 HISTREDUCEBLANKS},
Index: Src/zsh.h
@@ -1252,6 +1252,7 @@
 #define HIST_READ	0x00000004	/* Command was read back from disk*/
 #define HIST_DUP	0x00000008	/* Command duplicates a later line */
 #define HIST_FOREIGN	0x00000010	/* Command came from another shell */
+#define HIST_TMPSTORE	0x00000020	/* Kill when user enters another cmd */

 #define GETHIST_UPWARD  (-1)
 #define GETHIST_DOWNWARD  1
@@ -1367,6 +1368,7 @@
     HISTIGNOREALLDUPS,
     HISTIGNOREDUPS,
     HISTIGNORESPACE,
+    HISTIMMEDIATEDROP,
     HISTNOFUNCTIONS,
     HISTNOSTORE,
     HISTREDUCEBLANKS,
---8<------8<------8<------8<---cut here--->8------>8------>8------>8---