Latest hist-immediate-drop patch

Latest hist-immediate-drop patch

[Wayne Davison]

Let me know if zsh's release is not imminent, 'cuz I think that this
patch should be added to zsh.  It's very addictive -- I can't imagine
trying to use zsh without it now.

This is an updated patch that fixes one minor non-desirable side-effect
(the history number is now decremented when the previous temporary line
is discarded, thus avoiding unneeded gaps in the value) and to tweak my
use of the "save" variable slightly.

Enjoy,

..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 if (should_ignore_line(prog))
+	    save = -1;
     }
     if (flag & (HISTFLAG_DONE | HISTFLAG_RECALL)) {
 	char *ptr;
@@ -1058,14 +1063,23 @@
 	}
 	if (flag & HISTFLAG_RECALL) {
 	    zpushnode(bufstack, ptr);
-
 	    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) {
+	    if (he == hist_ring)
+		curline.histnum = curhist--;
+	    freehistnode((HashNode)he);
+	}
     }
-    if (save) {
+    if (save > 0 || (save < 0 && !isset(HISTIMMEDIATEDROP))) {
 	Histent he;
-	int keepflags;
+	int newflags;

 #ifdef DEBUG
 	/* debugging only */
@@ -1081,6 +1095,7 @@
 	    if (isset(HISTREDUCEBLANKS))
 		histreduceblanks();
 	}
+	newflags = save > 0? 0 : HIST_OLD | HIST_TMPSTORE;
 	if ((isset(HISTIGNOREDUPS) || isset(HISTIGNOREALLDUPS)) && hist_ring
 	 && histstrcmp(chline, hist_ring->text) == 0) {
 	    /* This history entry compares the same as the previous.
@@ -1089,18 +1104,16 @@
 	     * timestamp right.  Perhaps, preserve the HIST_OLD flag.
 	     */
 	    he = hist_ring;
-	    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));
@@ -1893,8 +1906,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);
@@ -1962,7 +1977,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---

Re: Latest hist-immediate-drop patch

[Bart Schaefer]

On May 14,  3:39pm, Wayne Davison wrote:
> Subject: Latest hist-immediate-drop patch
> Let me know if zsh's release is not imminent, 'cuz I think that this
> patch should be added to zsh.  It's very addictive -- I can't imagine
> trying to use zsh without it now.

I still think you should just put this in, without the option.  I can't
think of any reason why anyone would care, or even notice, the change
(especially given that you've closed the gaps in the numbering) unless
they were making use of the new functionality.

Re: Latest hist-immediate-drop patch

[Wayne Davison]

I've been rather ambivilent about the new option, but I think I'm ready
to side with you on the issue -- this delayed-drop is the way the
history code should have worked in the first place.  Both Zefram and
Sven voted for the inclusion of the option, though, which is the "safe"
choice (and everyone else has abstained).  I'd be quite happy to remove
the hist_immediate_drop option from my patch if we can reach some sort
of an agreement here.  Any changed votes?  Any new votes?  Or shall I
just take the bull by the horns and decide for us all?

..wayne..

Re: Latest hist-immediate-drop patch

[Zefram]

Wayne Davison wrote:
>                                           I'd be quite happy to remove
>the hist_immediate_drop option from my patch if we can reach some sort
>of an agreement here.  Any changed votes?

I don't want to actually change my vote -- it was a considered opinion,
which still stands -- but it is a close issue and my overall opinion in
favour of having the option is not a strong feeling.  I would not be
unhappy about the new behaviour being mandatory, with no way to reach
the old behaviour.

-zefram

Re: Latest hist-immediate-drop patch

[Sven Wischnowsky]

Wayne Davison wrote:

> [...]  Both Zefram and
> Sven voted for the inclusion of the option, though, which is the "safe"
> choice (and everyone else has abstained).  I'd be quite happy to remove
> the hist_immediate_drop option from my patch if we can reach some sort
> of an agreement here.  Any changed votes?  Any new votes?  Or shall I
> just take the bull by the horns and decide for us all?

Actually I meant that I liked the behaviour (and didn't care much about
whether we'd add a new option for it or not -- I said `feature', not
`option' ;-).


Bye
  Sven


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

Re: Latest hist-immediate-drop patch

[Adam Spiers]

Wayne Davison (wayned@users.sourceforge.net) wrote:
> I've been rather ambivilent about the new option, but I think I'm ready
> to side with you on the issue -- this delayed-drop is the way the
> history code should have worked in the first place.  Both Zefram and
> Sven voted for the inclusion of the option, though, which is the "safe"
> choice (and everyone else has abstained).  I'd be quite happy to remove
> the hist_immediate_drop option from my patch if we can reach some sort
> of an agreement here.  Any changed votes?  Any new votes?  Or shall I
> just take the bull by the horns and decide for us all?

FWIW, I like the idea a lot, and I'd be quite happy to see it included
with or without the option.  Like Bart, I can't imagine anyone being
seriously bothered by history hanging around for an extra line.