Re: PATCH: removed remhist()

Re: PATCH: removed remhist()

[Sven Wischnowsky]

Wayne Davison wrote:

> The following patch removes the remhist() function.  [...]

Fine, but...

> +static int
> +shouldIgnoreLine(Eprog prog)

Oh, please, don't. We use that form of functionName nowhere else (and 
I, at least, hate it, hate, hate, hate).

Bye
 Sven


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

Re: PATCH: removed remhist()

[Wayne Davison]

On Thu, 20 Jul 2000, Sven Wischnowsky wrote:
> Oh, please, don't. We use that form of functionName nowhere else (and 
> I, at least, hate it, hate, hate, hate).

Well, if it had only been "hate, hate", I suppose that would have been
OK, but since it's "hate, hate, hate", I'll go ahead and change it.  :-)

Seriously, I tweaked the name to use underscores.  I like this much better
than crammingallthewordstogetherinahardtoreadandoftenambiguoussingleword.

..wayne..

Re: PATCH: removed remhist()

[Sven Wischnowsky]

Wayne Davison wrote:

> ...
> 
> +	    } while (isalpha(*b));

This gives me a `char used as subscript warning'.

Or is there a reason to use isalpha() instead of ialpha()?

Bye
 Sven

Index: Src/hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/hist.c,v
retrieving revision 1.17
diff -u -r1.17 hist.c
--- Src/hist.c	2000/07/20 17:00:19	1.17
+++ Src/hist.c	2000/07/31 08:10:54
@@ -980,7 +980,7 @@
 		    zsfree(t);
 		    return 1;
 		}
-	    } while (isalpha(*b));
+	    } while (ialpha(*b));
 	}
 	zsfree(t);
     }

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

PATCH: removed remhist()

[Wayne Davison]

The following patch removes the remhist() function.  We now use a new
function named shouldIgnoreLine() from inside hend() to determine in
advance if a line should be stored in the history or not.  This makes
the incremental updating of the history file work properly, since
remhist() could never retroactively remove the line from the history
file when it removed it from the internal history list.

The code also fixes a bug in HIST_IGNORE_SPACE so that it once again
ignores a line if an expanded alias starts with a space.

I am much more satisfied with this code than I was a few days ago, so
I went ahead and checked it into CVS.

..wayne..

---8<------8<------8<------8<---cut here--->8------>8------>8------>8---
Index: Src/builtin.c
@@ -1208,8 +1208,6 @@
 	savehistfile(*argv, 1, HFILE_APPEND | (ops['I'] ? HFILE_SKIPOLD : 0));
 	return 0;
     }
-    if (!(ops['l'] && unset(HISTNOSTORE)))
-	remhist();
     /* put foo=bar type arguments into the substitution list */
     while (*argv && equalsplit(*argv, &s)) {
 	Asgment a = (Asgment) zhalloc(sizeof *a);
Index: Src/exec.c
@@ -3120,8 +3120,6 @@
 	}
 	shfunctab->addnode(shfunctab, ztrdup(s), shf);
     }
-    if (isset(HISTNOFUNCTIONS))
-	remhist();
     state->pc = end;
     return 0;
 }
Index: Src/hist.c
@@ -126,8 +126,7 @@
 /* Bits of histactive variable */
 #define HA_ACTIVE	(1<<0)	/* History mechanism is active */
 #define HA_NOSTORE	(1<<1)	/* Don't store the line when finished */
-#define HA_JUNKED	(1<<2)	/* Last history line was already junked */
-#define HA_NOINC	(1<<3)	/* Don't store, curhist not incremented */
+#define HA_NOINC	(1<<2)	/* Don't store, curhist not incremented */
 
 /* Array of word beginnings and endings in current history line. */
 
@@ -679,7 +678,7 @@
 mod_export void
 strinend(void)
 {
-    hend();
+    hend(NULL);
     DPUTS(!strin, "BUG: strinend() called without strinbeg()");
     strin--;
     isfirstch = 1;
@@ -763,8 +762,6 @@
     }
     chwordpos = 0;
 
-    if (histactive & HA_JUNKED)
-	curhist--;
     if (hist_ring && !hist_ring->ftim)
 	hist_ring->ftim = time(NULL);
     if (interact && isset(SHINSTDIN) && !strin) {
@@ -945,11 +942,54 @@
     return he;
 }
 
+/* A helper function for hend() */
+
+static int
+shouldIgnoreLine(Eprog prog)
+{
+    if (!prog)
+	return 0;
+
+    if (isset(HISTIGNORESPACE)) {
+	if (*chline == ' ' || aliasspaceflag)
+	    return 1;
+    }
+
+    if (isset(HISTNOFUNCTIONS)) {
+	Wordcode pc = prog->prog;
+	wordcode code = *pc;
+	if (wc_code(code) == WC_LIST && WC_LIST_TYPE(code) & Z_SIMPLE
+	 && wc_code(pc[2]) == WC_FUNCDEF)
+	    return 1;
+    }
+
+    if (isset(HISTNOSTORE)) {
+	char *b = getpermtext(prog, NULL);
+	if (*b == 'h' && strncmp(b, "history", 7) == 0
+	 && (!b[7] || b[7] == ' ')) {
+	    zsfree(b);
+	    return 1;
+	}
+	if (*b == 'f' && b[1] == 'c' && b[2] == ' ' && b[3] == '-') {
+	    b += 3;
+	    do {
+		if (*++b == 'l') {
+		    zsfree(b);
+		    return 1;
+		}
+	    } while (isalpha(*b));
+	}
+	zsfree(b);
+    }
+
+    return 0;
+}
+
 /* say we're done using the history mechanism */
 
 /**/
 mod_export int
-hend(void)
+hend(Eprog prog)
 {
     int flag, save = 1;
     char *hf = getsparam("HISTFILE");
@@ -987,7 +1027,7 @@
 	    } else
 		save = 0;
 	}
-	if (chwordpos <= 2 || (isset(HISTIGNORESPACE) && *chline == ' '))
+	if (chwordpos <= 2 || shouldIgnoreLine(prog))
 	    save = 0;
     }
     if (flag & (HISTFLAG_DONE | HISTFLAG_RECALL)) {
@@ -1059,24 +1099,6 @@
 	savehistfile(hf, 0, HFILE_USE_OPTIONS | HFILE_FAST);
     unlockhistfile(hf); /* It's OK to call this even if we aren't locked */
     return !(flag & HISTFLAG_NOEXEC || errflag);
-}
-
-/* remove the current line from the history List */
-
-/**/
-void
-remhist(void)
-{
-    if (hist_ring == &curline)
-	return;
-    if (!(histactive & HA_ACTIVE)) {
-	if (!(histactive & HA_JUNKED) && curline.histnum == curhist) {
-	    freehistnode((HashNode)hist_ring);
-	    histactive |= HA_JUNKED;
-	    /* curhist-- is delayed until the next hbegin() */
-	}
-    } else
-	histactive |= HA_NOSTORE;
 }
 
 /* Gives current expansion word if not last word before chwordpos. */
Index: Src/init.c
@@ -119,14 +119,14 @@
 	intr();			/* interrupts on            */
 	lexinit();              /* initialize lexical state */
 	if (!(prog = parse_event())) {	/* if we couldn't parse a list */
-	    hend();
+	    hend(NULL);
 	    if ((tok == ENDINPUT && !errflag) ||
 		(tok == LEXERR && (!isset(SHINSTDIN) || !toplevel)) ||
 		justonce)
 		break;
 	    continue;
 	}
-	if (hend()) {
+	if (hend(prog)) {
 	    int toksav = tok;
 	    Eprog preprog;
 
Index: Src/lex.c
@@ -1561,9 +1561,8 @@
 	    if (an && !an->inuse && ((an->flags & ALIAS_GLOBAL) || incmdpos ||
 				     inalmore)) {
 		inpush(an->text, INP_ALIAS, an);
-		/* remove from history if it begins with space */
-		if (isset(HISTIGNORESPACE) && an->text[0] == ' ')
-		    remhist();
+		if (an->text[0] == ' ')
+		    aliasspaceflag = 1;
 		lexstop = 0;
 		if (yytext == copy)
 		    yytext = tokstr;
Index: Src/parse.c
@@ -34,7 +34,10 @@
  
 /**/
 mod_export int incmdpos;
- 
+
+/**/
+int aliasspaceflag;
+
 /* != 0 if we are in the middle of a [[ ... ]] */
  
 /**/
@@ -418,6 +421,7 @@
 {
     tok = ENDINPUT;
     incmdpos = 1;
+    aliasspaceflag = 0;
     yylex();
     init_parse();
     return ((par_event()) ? bld_eprog() : NULL);
Index: Src/Zle/zle_main.c
@@ -852,7 +852,7 @@
 	hbegin(1);
     t = (char *) zleread(p1, p2, ops['h'] ? ZLRF_HISTORY : 0);
     if (ops['h'])
-	hend();
+	hend(NULL);
     isfirstln = ifl;
     varedarg = ova;
     if (haso) {
---8<------8<------8<------8<---cut here--->8------>8------>8------>8---