zsh-workers
 help / color / mirror / code / Atom feed
* PATCH: pattern incremental search
@ 2008-04-26 19:41 Peter Stephenson
  2008-04-26 20:22 ` Peter Stephenson
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Peter Stephenson @ 2008-04-26 19:41 UTC (permalink / raw)
  To: Zsh hackers list

I think this is now good enough for some beta testing, though I'm sure
there must be glitches.  It adds
history-incremental-pattern-search-backward and
history-incremental-pattern-search-forkward which are ridiculously long
names but nothing shorter really fits in with the names we have already.

The limitation that only non-overlapping pattern matches on the same
line are found comes from the parameter substitution code I hijacked to
do this.  It's not needed in this case since there's no substitution.
If anyone can see a good reason I can alter it.

I've also finished the work of the previous patch that put history
searching back to using unmetafied strings: it was no longer necessary
to allocate additional space for each history line at all (unless it was
modified), so now it doesn't.  This should be a significant saving for
searching large histories.

Index: NEWS
===================================================================
RCS file: /cvsroot/zsh/zsh/NEWS,v
retrieving revision 1.15
diff -u -r1.15 NEWS
--- NEWS	17 Apr 2008 10:23:52 -0000	1.15
+++ NEWS	26 Apr 2008 19:40:38 -0000
@@ -16,6 +16,11 @@
 files using the system call fcntl().  On recent NFS implementations this
 may provide better reliability.
 
+Patterns can now be used in incremental searches with the new widgets
+history-incremental-pattern-search-backward and
+history-incremental-pattern-search-forkward.  These are not bound to
+keys by default.
+
 Highlighting of sections of the command line is now supported, controlled
 by the array parameter zle_highlight and the ZLE special parameter
 REGION_HIGHLIGHT.
Index: Doc/Zsh/zle.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/zle.yo,v
retrieving revision 1.62
diff -u -r1.62 zle.yo
--- Doc/Zsh/zle.yo	7 Apr 2008 09:44:34 -0000	1.62
+++ Doc/Zsh/zle.yo	26 Apr 2008 19:40:40 -0000
@@ -1152,6 +1152,27 @@
 search to the beginning of the line.  The functions available in the
 mini-buffer are the same as for tt(history-incremental-search-backward).
 )
+tindex(history-incremental-pattern-search-backward)
+tindex(history-incremental-pattern-search-forward)
+xitem(tt(history-incremental-pattern-search-backward))
+item(tt(history-incremental-pattern-search-forward))(
+These widgets behave similarly to the corresponding widgets with
+no tt(-pattern), but the search string typed by the user is treated
+as a pattern, respecting the current settings of the various options
+affecting pattern matching.  See
+ifzman(FILENAME GENERATION in zmanref(zshexpn))\
+ifnzman(noderef(Filename Generation)) for a description of patterns.
+If no numeric argument was given lowercase letters in the search
+string may match uppercase letters in the history.  The string may begin
+with `tt(^)' to anchor the search to the beginning of the line.
+
+The prompt changes to indicate an invalid pattern; this may simply
+indicate the pattern is not yet complete.
+
+Note that only non-overlapping matches are reported, so an expression
+with wildcards may return fewer matches on a line than are visible
+by inspection.
+)
 tindex(history-search-backward)
 item(tt(history-search-backward) (ESC-P ESC-p) (unbound) (unbound))(
 Search backward in the history for a line beginning with the first
Index: Src/glob.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/glob.c,v
retrieving revision 1.61
diff -u -r1.61 glob.c
--- Src/glob.c	1 Nov 2007 17:57:57 -0000	1.61
+++ Src/glob.c	26 Apr 2008 19:40:43 -0000
@@ -2050,12 +2050,6 @@
 /* do the ${foo%%bar}, ${foo#bar} stuff */
 /* please do not laugh at this code. */
 
-struct repldata {
-    int b, e;			/* beginning and end of chunk to replace */
-    char *replstr;		/* replacement string to use */
-};
-typedef struct repldata *Repldata;
-
 /* Having found a match in getmatch, decide what part of string
  * to return.  The matched part starts b characters into string s
  * and finishes e characters in: 0 <= b <= e <= strlen(s)
@@ -2073,19 +2067,23 @@
     char buf[80], *r, *p, *rr;
     int ll = 0, l = strlen(s), bl = 0, t = 0, i;
 
-    if (replstr) {
+    if (replstr || (fl & SUB_LIST)) {
 	if (fl & SUB_DOSUBST) {
 	    replstr = dupstring(replstr);
 	    singsub(&replstr);
 	    untokenize(replstr);
 	}
-	if ((fl & SUB_GLOBAL) && repllist) {
+	if ((fl & (SUB_GLOBAL|SUB_LIST)) && repllist) {
 	    /* We are replacing the chunk, just add this to the list */
-	    Repldata rd = (Repldata) zhalloc(sizeof(*rd));
+	    Repldata rd = (Repldata)
+		((fl & SUB_LIST) ? zalloc(sizeof(*rd)) : zhalloc(sizeof(*rd)));
 	    rd->b = b;
 	    rd->e = e;
 	    rd->replstr = replstr;
-	    addlinknode(repllist, rd);
+	    if (fl & SUB_LIST)
+		zaddlinknode(repllist, rd);
+	    else
+		addlinknode(repllist, rd);
 	    return s;
 	}
 	ll += strlen(replstr);
@@ -2214,9 +2212,14 @@
     if (!(p = compgetmatch(pat, &fl, &replstr)))
 	return 1;
 
-    return igetmatch(sp, p, fl, n, replstr);
+    return igetmatch(sp, p, fl, n, replstr, NULL);
 }
 
+/*
+ * This is the corresponding function for array variables.
+ * Matching is done with the same pattern on each element.
+ */
+
 /**/
 void
 getmatcharr(char ***ap, char *pat, int fl, int n, char *replstr)
@@ -2229,10 +2232,47 @@
 
     *ap = pp = hcalloc(sizeof(char *) * (arrlen(arr) + 1));
     while ((*pp = *arr++))
-	if (igetmatch(pp, p, fl, n, replstr))
+	if (igetmatch(pp, p, fl, n, replstr, NULL))
 	    pp++;
 }
 
+/*
+ * Match against str using pattern pp; return a list of
+ * Repldata matches in the linked list *replistp; this is
+ * in permanent storage and to be freed by freematchlist()
+ */
+
+/**/
+mod_export int
+getmatchlist(char *str, Patprog p, LinkList *repllistp)
+{
+    char **sp = &str;
+
+    /*
+     * We don't care if we have longest or shortest match, but SUB_LONG
+     * is cheaper since the pattern code does that by default.
+     * We need SUB_GLOBAL to get all matches.
+     * We need SUB_SUBSTR to scan through for substrings.
+     * We need SUB_LIST to activate the special handling of the list
+     * passed in.
+     */
+    return igetmatch(sp, p, SUB_LONG|SUB_GLOBAL|SUB_SUBSTR|SUB_LIST,
+		     0, NULL, repllistp);
+}
+
+static void
+freerepldata(void *ptr)
+{
+    zfree(ptr, sizeof(struct repldata));
+}
+
+/**/
+mod_export void
+freematchlist(LinkList repllist)
+{
+    freelinklist(repllist, freerepldata);
+}
+
 /**/
 static void
 set_pat_start(Patprog p, int offs)
@@ -2295,7 +2335,8 @@
 
 /**/
 static int
-igetmatch(char **sp, Patprog p, int fl, int n, char *replstr)
+igetmatch(char **sp, Patprog p, int fl, int n, char *replstr,
+	  LinkList *repllistp)
 {
     char *s = *sp, *t, *tmatch;
     /*
@@ -2341,7 +2382,7 @@
 
     if (fl & SUB_ALL) {
 	int i = matched && pattry(p, s);
-	*sp = get_match_ret(*sp, 0, i ? l : 0, fl, i ? replstr : 0, repllist);
+	*sp = get_match_ret(*sp, 0, i ? l : 0, fl, i ? replstr : 0, NULL);
 	if (! **sp && (((fl & SUB_MATCH) && !i) || ((fl & SUB_REST) && i)))
 	    return 0;
 	return 1;
@@ -2387,7 +2428,7 @@
 			umlen += iincchar(&t);
 		    }
 		}
-		*sp = get_match_ret(*sp, 0, mlen, fl, replstr, repllist);
+		*sp = get_match_ret(*sp, 0, mlen, fl, replstr, NULL);
 		return 1;
 	    }
 	    break;
@@ -2414,11 +2455,11 @@
 		umlen -= iincchar(&t);
 	    }
 	    if (tmatch) {
-		*sp = get_match_ret(*sp, tmatch - s, l, fl, replstr, repllist);
+		*sp = get_match_ret(*sp, tmatch - s, l, fl, replstr, NULL);
 		return 1;
 	    }
 	    if (!(fl & SUB_START) && pattrylen(p, s + l, 0, 0, ioff)) {
-		*sp = get_match_ret(*sp, l, l, fl, replstr, repllist);
+		*sp = get_match_ret(*sp, l, l, fl, replstr, NULL);
 		return 1;
 	    }
 	    break;
@@ -2431,7 +2472,7 @@
 	    for (ioff = 0, t = s, umlen = umltot; t < s + l; ioff++) {
 		set_pat_start(p, t-s);
 		if (pattrylen(p, t, s + l - t, umlen, ioff)) {
-		    *sp = get_match_ret(*sp, t-s, l, fl, replstr, repllist);
+		    *sp = get_match_ret(*sp, t-s, l, fl, replstr, NULL);
 		    return 1;
 		}
 		if (fl & SUB_START)
@@ -2439,7 +2480,7 @@
 		umlen -= iincchar(&t);
 	    }
 	    if (!(fl & SUB_START) && pattrylen(p, s + l, 0, 0, ioff)) {
-		*sp = get_match_ret(*sp, l, l, fl, replstr, repllist);
+		*sp = get_match_ret(*sp, l, l, fl, replstr, NULL);
 		return 1;
 	    }
 	    break;
@@ -2448,14 +2489,17 @@
 	    /* Smallest at start, but matching substrings. */
 	    set_pat_start(p, l);
 	    if (!(fl & SUB_GLOBAL) && pattry(p, s + l) && !--n) {
-		*sp = get_match_ret(*sp, 0, 0, fl, replstr, repllist);
+		*sp = get_match_ret(*sp, 0, 0, fl, replstr, NULL);
 		return 1;
 	    } /* fall through */
 	case (SUB_SUBSTR|SUB_LONG):
 	    /* longest or smallest at start with substrings */
 	    t = s;
-	    if (fl & SUB_GLOBAL)
-		repllist = newlinklist();
+	    if (fl & SUB_GLOBAL) {
+		repllist = (fl & SUB_LIST) ? znewlinklist() : newlinklist();
+		if (repllistp)
+		     *repllistp = repllist;
+	    }
 	    ioff = 0;		/* offset into string */
 	    umlen = umltot;
 	    mb_metacharinit();
@@ -2539,7 +2583,7 @@
 	    if (!(fl & SUB_LONG)) {
 		set_pat_start(p, l);
 		if (pattrylen(p, s + l, 0, 0, umltot) && !--n) {
-		    *sp = get_match_ret(*sp, l, l, fl, replstr, repllist);
+		    *sp = get_match_ret(*sp, l, l, fl, replstr, NULL);
 		    return 1;
 		}
 	    }
@@ -2595,12 +2639,12 @@
 		    }
 		}
 		*sp = get_match_ret(*sp, tmatch-s, mpos-s, fl,
-				    replstr, repllist);
+				    replstr, NULL);
 		return 1;
 	    }
 	    set_pat_start(p, l);
 	    if ((fl & SUB_LONG) && pattrylen(p, s + l, 0, 0, umltot) && !--n) {
-		*sp = get_match_ret(*sp, l, l, fl, replstr, repllist);
+		*sp = get_match_ret(*sp, l, l, fl, replstr, NULL);
 		return 1;
 	    }
 	    break;
@@ -2611,34 +2655,39 @@
 	/* Put all the bits of a global search and replace together. */
 	LinkNode nd;
 	Repldata rd;
-	int lleft = 0;		/* size of returned string */
+	int lleft;
 	char *ptr, *start;
 	int i;
 
-	i = 0;			/* start of last chunk we got from *sp */
-	for (nd = firstnode(repllist); nd; incnode(nd)) {
-	    rd = (Repldata) getdata(nd);
-	    lleft += rd->b - i; /* previous chunk of *sp */
-	    lleft += strlen(rd->replstr);	/* the replaced bit */
-	    i = rd->e;		/* start of next chunk of *sp */
+	if (!(fl & SUB_LIST)) {
+	    lleft = 0;		/* size of returned string */
+	    i = 0;			/* start of last chunk we got from *sp */
+	    for (nd = firstnode(repllist); nd; incnode(nd)) {
+		rd = (Repldata) getdata(nd);
+		lleft += rd->b - i; /* previous chunk of *sp */
+		lleft += strlen(rd->replstr);	/* the replaced bit */
+		i = rd->e;		/* start of next chunk of *sp */
+	    }
+	    lleft += l - i;	/* final chunk from *sp */
+	    start = t = zhalloc(lleft+1);
+	    i = 0;
+	    for (nd = firstnode(repllist); nd; incnode(nd)) {
+		rd = (Repldata) getdata(nd);
+		memcpy(t, s + i, rd->b - i);
+		t += rd->b - i;
+		ptr = rd->replstr;
+		while (*ptr)
+		    *t++ = *ptr++;
+		i = rd->e;
+	    }
+	    memcpy(t, s + i, l - i);
+	    start[lleft] = '\0';
+	    *sp = (char *)start;
 	}
-	lleft += l - i;	/* final chunk from *sp */
-	start = t = zhalloc(lleft+1);
-	i = 0;
-	for (nd = firstnode(repllist); nd; incnode(nd)) {
-	    rd = (Repldata) getdata(nd);
-	    memcpy(t, s + i, rd->b - i);
-	    t += rd->b - i;
-	    ptr = rd->replstr;
-	    while (*ptr)
-		*t++ = *ptr++;
-	    i = rd->e;
-	}
-	memcpy(t, s + i, l - i);
-	start[lleft] = '\0';
-	*sp = (char *)start;
 	return 1;
     }
+    if (fl & SUB_LIST)		/* safety: don't think this can happen */
+	return 0;
 
     /* munge the whole string: no match, so no replstr */
     *sp = get_match_ret(*sp, 0, 0, fl, 0, 0);
@@ -2656,7 +2705,8 @@
 
 /**/
 static int
-igetmatch(char **sp, Patprog p, int fl, int n, char *replstr)
+igetmatch(char **sp, Patprog p, int fl, int n, char *replstr,
+	  LinkList *replistp)
 {
     char *s = *sp, *t;
     /*
@@ -2695,7 +2745,7 @@
 
     if (fl & SUB_ALL) {
 	int i = matched && pattry(p, s);
-	*sp = get_match_ret(*sp, 0, i ? l : 0, fl, i ? replstr : 0, repllist);
+	*sp = get_match_ret(*sp, 0, i ? l : 0, fl, i ? replstr : 0, NULL);
 	if (! **sp && (((fl & SUB_MATCH) && !i) || ((fl & SUB_REST) && i)))
 	    return 0;
 	return 1;
@@ -2724,7 +2774,7 @@
 			}
 		    }
 		}
-		*sp = get_match_ret(*sp, 0, mlen, fl, replstr, repllist);
+		*sp = get_match_ret(*sp, 0, mlen, fl, replstr, NULL);
 		return 1;
 	    }
 	    break;
@@ -2739,7 +2789,7 @@
 		    t--;
 		set_pat_start(p, t-s);
 		if (pattrylen(p, t, s + l - t, umlen, ioff)) {
-		    *sp = get_match_ret(*sp, t - s, l, fl, replstr, repllist);
+		    *sp = get_match_ret(*sp, t - s, l, fl, replstr, NULL);
 		    return 1;
 		}
 		if (t > s+1 && t[-2] == Meta)
@@ -2755,7 +2805,7 @@
 		 ioff++, METAINC(t), umlen--) {
 		set_pat_start(p, t-s);
 		if (pattrylen(p, t, s + l - t, umlen, ioff)) {
-		    *sp = get_match_ret(*sp, t-s, l, fl, replstr, repllist);
+		    *sp = get_match_ret(*sp, t-s, l, fl, replstr, NULL);
 		    return 1;
 		}
 		if (*t == Meta)
@@ -2767,14 +2817,17 @@
 	    /* Smallest at start, but matching substrings. */
 	    set_pat_start(p, l);
 	    if (!(fl & SUB_GLOBAL) && pattry(p, s + l) && !--n) {
-		*sp = get_match_ret(*sp, 0, 0, fl, replstr, repllist);
+		*sp = get_match_ret(*sp, 0, 0, fl, replstr, NULL);
 		return 1;
 	    } /* fall through */
 	case (SUB_SUBSTR|SUB_LONG):
 	    /* longest or smallest at start with substrings */
 	    t = s;
-	    if (fl & SUB_GLOBAL)
+	    if (fl & SUB_GLOBAL) {
 		repllist = newlinklist();
+		if (repllistp)
+		    *repllistp = repllist;
+	    }
 	    ioff = 0;		/* offset into string */
 	    umlen = uml;
 	    do {
@@ -2849,7 +2902,7 @@
 	    if (!(fl & SUB_LONG)) {
 		set_pat_start(p, l);
 		if (pattrylen(p, s + l, 0, 0, uml) && !--n) {
-		    *sp = get_match_ret(*sp, l, l, fl, replstr, repllist);
+		    *sp = get_match_ret(*sp, l, l, fl, replstr, NULL);
 		    return 1;
 		}
 	    }
@@ -2874,13 +2927,13 @@
 			}
 		    }
 		    *sp = get_match_ret(*sp, t-s, mpos-s, fl,
-					replstr, repllist);
+					replstr, NULL);
 		    return 1;
 		}
 	    }
 	    set_pat_start(p, l);
 	    if ((fl & SUB_LONG) && pattrylen(p, s + l, 0, 0, uml) && !--n) {
-		*sp = get_match_ret(*sp, l, l, fl, replstr, repllist);
+		*sp = get_match_ret(*sp, l, l, fl, replstr, NULL);
 		return 1;
 	    }
 	    break;
Index: Src/pattern.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/pattern.c,v
retrieving revision 1.43
diff -u -r1.43 pattern.c
--- Src/pattern.c	30 Mar 2008 22:14:24 -0000	1.43
+++ Src/pattern.c	26 Apr 2008 19:40:45 -0000
@@ -504,6 +504,8 @@
 	else
 	    patglobflags = 0;
     }
+    if (patflags & PAT_LCMATCHUC)
+	patglobflags |= GF_LCMATCHUC;
     /*
      * Have to be set now, since they get updated during compilation.
      */
Index: Src/zsh.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/zsh.h,v
retrieving revision 1.126
diff -u -r1.126 zsh.h
--- Src/zsh.h	23 Apr 2008 08:40:04 -0000	1.126
+++ Src/zsh.h	26 Apr 2008 19:40:46 -0000
@@ -437,6 +437,7 @@
 #define firstnode(X)        ((X)->list.first)
 #define lastnode(X)         ((X)->list.last)
 #define peekfirst(X)        (firstnode(X)->dat)
+#define peeklast(X)         (lastnode(X)->dat)
 #define addlinknode(X,Y)    insertlinknode(X,lastnode(X),Y)
 #define zaddlinknode(X,Y)   zinsertlinknode(X,lastnode(X),Y)
 #define uaddlinknode(X,Y)   uinsertlinknode(X,lastnode(X),Y)
@@ -450,6 +451,7 @@
 #define pushnode(X,Y)       insertlinknode(X,&(X)->node,Y)
 #define zpushnode(X,Y)      zinsertlinknode(X,&(X)->node,Y)
 #define incnode(X)          (X = nextnode(X))
+#define decnode(X)          (X = prevnode(X))
 #define firsthist()         (hist_ring? hist_ring->down->histnum : curhist)
 #define setsizednode(X,Y,Z) (firstnode(X)[(Y)].dat = (void *) (Z))
 
@@ -1292,6 +1294,7 @@
 #define PAT_NOTSTART	0x0200	/* Start of string is not real start */
 #define PAT_NOTEND	0x0400	/* End of string is not real end */
 #define PAT_HAS_EXCLUDP	0x0800	/* (internal): top-level path1~path2. */
+#define PAT_LCMATCHUC   0x1000  /* equivalent to setting (#l) */
 
 /* Globbing flags: lower 8 bits gives approx count */
 #define GF_LCMATCHUC	0x0100
@@ -1489,6 +1492,19 @@
 #define SUB_RETFAIL	0x0800  /* return status 0 if no match */
 #define SUB_START	0x1000  /* force match at start with SUB_END
 				 * and no SUB_SUBSTR */
+#define SUB_LIST	0x2000  /* no substitution, return list of matches */
+
+/*
+ * Structure recording multiple matches inside a test string.
+ * b and e are the beginning and end of the match.
+ * replstr is the replacement string, if any.
+ */
+struct repldata {
+    int b, e;			/* beginning and end of chunk to replace */
+    char *replstr;		/* replacement string to use */
+};
+typedef struct repldata *Repldata;
+
 
 /* Flags as the second argument to prefork */
 #define PF_TYPESET	0x01	/* argument handled like typeset foo=bar */
Index: Src/Zle/iwidgets.list
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/iwidgets.list,v
retrieving revision 1.10
diff -u -r1.10 iwidgets.list
--- Src/Zle/iwidgets.list	16 Oct 2006 01:09:22 -0000	1.10
+++ Src/Zle/iwidgets.list	26 Apr 2008 19:40:46 -0000
@@ -65,6 +65,8 @@
 "history-beginning-search-forward", historybeginningsearchforward, 0
 "history-incremental-search-backward", historyincrementalsearchbackward, 0
 "history-incremental-search-forward", historyincrementalsearchforward, 0
+"history-incremental-pattern-search-backward", historyincrementalpatternsearchbackward, 0
+"history-incremental-pattern-search-forward", historyincrementalpatternsearchforward, 0
 "history-search-backward", historysearchbackward, 0
 "history-search-forward", historysearchforward, 0
 "infer-next-history", infernexthistory, 0
Index: Src/Zle/zle_hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_hist.c,v
retrieving revision 1.43
diff -u -r1.43 zle_hist.c
--- Src/Zle/zle_hist.c	22 Apr 2008 15:08:13 -0000	1.43
+++ Src/Zle/zle_hist.c	26 Apr 2008 19:40:46 -0000
@@ -52,47 +52,12 @@
 
 /*** History text manipulation utilities ***/
 
-
-struct zle_text {
-    /* Metafied, NULL-terminated string */
-    char *text;
-    /* 1 if we have allocated space for text */
-    int alloced;
-};
-
 /*
- * Fetch the text of a history line in internal ZLE format.
- * If the line has been edited, returns that, else allocates
- * a converted line.
- *
- * Each use of this must have a matching zletextfree() in order
- * to free up the allocated line, if any.  (N.B.: each use *of
- * the function*, not just each use of a struct zle_text.)
+ * Text for the line:  anything previously modified within zle since
+ * the last time the line editor was started, else what was originally
+ * put in the history.
  */
-
-static void
-zletext(Histent ent, struct zle_text *zt)
-{
-    if (ent->zle_text) {
-	zt->text = ent->zle_text;
-	zt->alloced = 0;
-	return;
-    }
-
-    zt->text = ztrdup(ent->node.nam);
-    zt->alloced = 1;
-}
-
-/* See above. */
-
-static void
-zletextfree(struct zle_text *zt)
-{
-    if (zt->alloced) {
-	free(zt->text);
-	zt->alloced = 0;
-    }
-}
+#define GETZLETEXT(ent)	((ent)->zle_text ? (ent)->zle_text : (ent)->node.nam)
 
 /**/
 void
@@ -100,7 +65,7 @@
 {
     Histent ent = quietgethist(histline);
     if (ent) {
-	char *line = 
+	char *line =
 	    zlemetaline ? zlemetaline :
 	    zlelineasstring(zleline, zlell, 0, NULL, NULL, 0);
 	if (!ent->zle_text || strcmp(line, ent->zle_text) != 0) {
@@ -464,7 +429,7 @@
     Histent he;
     int n = zmult;
     char *str;
-    struct zle_text zt;
+    char *zt;
 
     if (zmult < 0) {
 	int ret;
@@ -499,18 +464,16 @@
     while ((he = movehistent(he, -1, hist_skip_flags))) {
 	if (isset(HISTFINDNODUPS) && he->node.flags & HIST_DUP)
 	    continue;
-	zletext(he, &zt);
-	if (zlinecmp(zt.text, str) < 0 &&
-	    (*args || strcmp(zt.text, str) != 0)) {
+	zt = GETZLETEXT(he);
+	if (zlinecmp(zt, str) < 0 &&
+	    (*args || strcmp(zt, str) != 0)) {
 	    if (--n <= 0) {
 		zle_setline(he);
 		srch_hl = histline;
 		srch_cs = zlecs;
-		zletextfree(&zt);
 		return 0;
 	    }
 	}
-	zletextfree(&zt);
     }
     return 1;
 }
@@ -522,7 +485,7 @@
     Histent he;
     int n = zmult;
     char *str;
-    struct zle_text zt;
+    char *zt;
 
     if (zmult < 0) {
 	int ret;
@@ -555,18 +518,16 @@
     while ((he = movehistent(he, 1, hist_skip_flags))) {
 	if (isset(HISTFINDNODUPS) && he->node.flags & HIST_DUP)
 	    continue;
-	zletext(he, &zt);
-	if (zlinecmp(zt.text, str) < (he->histnum == curhist) &&
-	    (*args || strcmp(zt.text, str) != 0)) {
+	zt = GETZLETEXT(he);
+	if (zlinecmp(zt, str) < (he->histnum == curhist) &&
+	    (*args || strcmp(zt, str) != 0)) {
 	    if (--n <= 0) {
 		zle_setline(he);
 		srch_hl = histline;
 		srch_cs = zlecs;
-		zletextfree(&zt);
 		return 0;
 	    }
 	}
-	zletextfree(&zt);
     }
     return 1;
 }
@@ -780,7 +741,7 @@
     mkundoent();
     histline = he->histnum;
 
-    setline(he->zle_text ? he->zle_text : he->node.nam, ZSL_COPY|ZSL_TOEND);
+    setline(GETZLETEXT(he), ZSL_COPY|ZSL_TOEND);
     setlastline();
     clearlist = 1;
     if (remetafy)
@@ -809,15 +770,11 @@
     if (!he || !(he = movehistent(he, n, hist_skip_flags)))
 	return 1;
     if (skipdups && n) {
-	struct zle_text zt;
-
 	n = n < 0? -1 : 1;
 	while (he) {
 	    int ret;
 
-	    zletext(he, &zt);
-	    ret = zlinecmp(zt.text, line);
-	    zletextfree(&zt);
+	    ret = zlinecmp(GETZLETEXT(he), line);
 	    if (ret)
 		break;
 	    he = movehistent(he, n, hist_skip_flags);
@@ -917,7 +874,7 @@
 int
 historyincrementalsearchbackward(char **args)
 {
-    doisearch(args, -1);
+    doisearch(args, -1, 0);
     return 0;
 }
 
@@ -925,18 +882,36 @@
 int
 historyincrementalsearchforward(char **args)
 {
-    doisearch(args, 1);
+    doisearch(args, 1, 0);
+    return 0;
+}
+
+/**/
+int
+historyincrementalpatternsearchbackward(char **args)
+{
+    doisearch(args, -1, 1);
+    return 0;
+}
+
+/**/
+int
+historyincrementalpatternsearchforward(char **args)
+{
+    doisearch(args, 1, 1);
     return 0;
 }
 
 static struct isrch_spot {
     int hl;			/* This spot's histline */
+    int pat_hl;			/* histline where pattern search started */
     unsigned short pos;		/* The search position in our metafied str */
+    unsigned short pat_pos;     /* pos where pattern search started */
     unsigned short cs;		/* The visible search position to the user */
     unsigned short len;		/* The search string's length */
     unsigned short flags;	/* This spot's flags */
-#define ISS_FAILING	1
-#define ISS_FORWARD	2
+#define ISS_FORWARD	1
+#define ISS_NOMATCH_SHIFT 1
 } *isrch_spots;
 
 static int max_spot = 0;
@@ -952,7 +927,8 @@
 
 /**/
 static void
-set_isrch_spot(int num, int hl, int pos, int cs, int len, int dir, int nomatch)
+set_isrch_spot(int num, int hl, int pos, int pat_hl, int pat_pos,
+	       int cs, int len, int dir, int nomatch)
 {
     if (num >= max_spot) {
 	if (!isrch_spots) {
@@ -966,43 +942,161 @@
 
     isrch_spots[num].hl = hl;
     isrch_spots[num].pos = (unsigned short)pos;
+    isrch_spots[num].pat_hl = pat_hl;
+    isrch_spots[num].pat_pos = (unsigned short)pat_pos;
     isrch_spots[num].cs = (unsigned short)cs;
     isrch_spots[num].len = (unsigned short)len;
     isrch_spots[num].flags = (dir > 0? ISS_FORWARD : 0)
-			   + (nomatch? ISS_FAILING : 0);
+			   + (nomatch << ISS_NOMATCH_SHIFT);
 }
 
 /**/
 static void
-get_isrch_spot(int num, int *hlp, int *posp, int *csp, int *lenp, int *dirp, int *nomatch)
+get_isrch_spot(int num, int *hlp, int *posp, int *pat_hlp, int *pat_posp,
+	       int *csp, int *lenp, int *dirp, int *nomatch)
 {
     *hlp = isrch_spots[num].hl;
     *posp = (int)isrch_spots[num].pos;
+    *pat_hlp = isrch_spots[num].pat_hl;
+    *pat_posp = (int)isrch_spots[num].pat_pos;
     *csp = (int)isrch_spots[num].cs;
     *lenp = (int)isrch_spots[num].len;
     *dirp = (isrch_spots[num].flags & ISS_FORWARD)? 1 : -1;
-    *nomatch = (isrch_spots[num].flags & ISS_FAILING);
+    *nomatch = (int)(isrch_spots[num].flags >> ISS_NOMATCH_SHIFT);
+}
+
+/*
+ * In pattern search mode, look through the list for a match at, or
+ * before or after the given position, according to the direction.
+ * Return new position or -1.
+ *
+ * Note this handles curpos out of range correctly, i.e. curpos < 0
+ * never matches when searching backwards and curpos > length of string
+ * never matches when searching forwards.
+ */
+static int
+isearch_newpos(LinkList matchlist, int curpos, int dir)
+{
+    LinkNode node;
+
+    if (dir < 0) {
+	for (node = lastnode(matchlist);
+	     node != (LinkNode)matchlist; decnode(node)) {
+	    Repldata rdata = (Repldata)getdata(node);
+	    if (rdata->b <= curpos)
+		return rdata->b;
+	}
+    } else {
+	for (node = firstnode(matchlist);
+	     node; incnode(node)) {
+	    Repldata rdata = (Repldata)getdata(node);
+	    if (rdata->b >= curpos)
+		return rdata->b;
+	}
+    }
+
+    return -1;
 }
 
-#define ISEARCH_PROMPT		"failing XXX-i-search: "
-#define NORM_PROMPT_POS		8
+#define ISEARCH_PROMPT		"XXXXXXX XXX-i-search: "
+#define FAILING_TEXT		"failing"
+#define INVALID_TEXT		"invalid"
+#define BAD_TEXT_LEN		7
+#define NORM_PROMPT_POS		(BAD_TEXT_LEN+1)
 #define FIRST_SEARCH_CHAR	(NORM_PROMPT_POS + 14)
 
 /**/
 static void
-doisearch(char **args, int dir)
+doisearch(char **args, int dir, int pattern)
 {
+    /* The full search buffer, including space for all prompts */
     char *ibuf = zhalloc(80);
+    /* The part of the search buffer with the search string */
     char *sbuf = ibuf + FIRST_SEARCH_CHAR;
+    /* The previous line shown to the user */
     char *last_line = NULL;
-    struct zle_text zt;
-    int sbptr = 0, top_spot = 0, pos, sibuf = 80;
+    /* Text of the history line being examined */
+    char *zt;
+    /*
+     * sbptr: index into sbuf.
+     * top_spot: stack index into the "isrch_spot" stack.
+     * sibuf: allocation size for ibuf
+     */
+    int sbptr = 0, top_spot = 0, sibuf = 80;
+    /*
+     * nomatch = 1: failing isearch
+     * nomatch = 2: invalid pattern
+     * skip_line: finished with current line, skip to next
+     * skip_pos: keep current line but try before/after current position.
+     */
     int nomatch = 0, skip_line = 0, skip_pos = 0;
+    /*
+     * odir: original search direction
+     * sens: limit for zlinecmp to allow (3) or disallow (1) lower case
+     *       matching upper case.
+     */
     int odir = dir, sens = zmult == 1 ? 3 : 1;
-    int hl = histline, savekeys = -1, feep = 0;
+    /*
+     * The number of the history line we are looking at and the
+     * character position into it, essentially the cursor position
+     * except we don't update that as frequently.
+     */
+    int hl = histline, pos;
+    /*
+     * The value of hl and pos at which the last pattern match
+     * search started.  We need to record these because there's
+     * a pathology with pattern matching.  Here's an example.  Suppose
+     * the history consists of:
+     *  echo '*OH NO*'
+     *  echo '\n'
+     *  echo "*WHAT?*"
+     *  <...backward pattern search starts here...>
+     * The user types "\".  As there's nothing after it it's treated
+     * literally (and I certainly don't want to change that).  This
+     * goes to the second line.  Then the user types "*".  This
+     * ought to match the "*" in the line immediately before where the
+     * search started.  However, unless we return to that line for the
+     * new search it will instead carry on to the first line.  This is
+     * different from straight string matching where we never have
+     * to backtrack.
+     *
+     * I think these need resetting to the current hl and pos when
+     * we start a new search or repeat a search.  It seems to work,
+     * anyway.
+     *
+     * We could optimize this more, but I don't think there's a lot
+     * of point.  (Translation:  it's difficult.)
+     */
+    int pat_hl = hl, pat_pos;
+    /*
+     * This is the flag that we need to revert the positions to
+     * the above for the next pattern search.
+     */
+    int revert_patpos = 0;
+    /*
+     * savekeys records the unget buffer, so that if we have arguments
+     * they don't pollute the input.
+     * feep indicates we should feep.  This is a well-known word
+     * meaning "to indicate an error in the zsh line editor".
+     */
+    int savekeys = -1, feep = 0;
+    /* Flag that we are at an old position, no need to search again */
+    int nosearch = 0;
+    /* Command read as input:  we don't read characters directly. */
     Thingy cmd;
+    /* Save the keymap if necessary */
     char *okeymap;
+    /* The current history entry, corresponding to hl */
     Histent he;
+    /* When pattern matching, the compiled pattern */
+    Patprog patprog = NULL;
+    /* When pattern matching, the list of match positions */
+    LinkList matchlist = NULL;
+    /*
+     * When we exit isearching this may be a zle command to
+     * execute.  We save it and execute it after unmetafying the
+     * command line.
+     */
     ZleIntFunc exitfn = (ZleIntFunc)0;
 
     if (!(he = quietgethist(hl)))
@@ -1026,67 +1120,179 @@
 
     metafy_line();
     remember_edits();
-    zletext(he, &zt);
-    pos = zlemetacs;
+    zt = GETZLETEXT(he);
+    pat_pos = pos = zlemetacs;
     for (;;) {
 	/* Remember the current values in case search fails (doesn't push). */
-	set_isrch_spot(top_spot, hl, pos, zlemetacs, sbptr, dir, nomatch);
+	set_isrch_spot(top_spot, hl, pos, pat_hl, pat_pos,
+		       zlemetacs, sbptr, dir, nomatch);
 	if (sbptr == 1 && sbuf[0] == '^') {
 	    zlemetacs = 0;
     	    nomatch = 0;
 	    statusline = ibuf + NORM_PROMPT_POS;
 	} else if (sbptr > 0) {
-	    /*
-	     * As we may free zt.text as soon as we switch to a new
-	     * line, we can't keep the pointer to it.  This is a bit
-	     * ghastly.
-	     */
-	    if (last_line)
-		free(last_line);
-	    last_line = ztrdup(zt.text);
+	    char *t = NULL;
+	    last_line = zt;
 
 	    sbuf[sbptr] = '\0';
-	    for (;;) {
-		char *t;
-
+	    if (pattern && !patprog && !nosearch) {
+		/* avoid too much heap use, can get heavy round here... */
+		char *patbuf = ztrdup(sbuf);
+		char *patstring;
 		/*
-		 * If instructed, move past a match position:
-		 * backwards if searching backwards (skipping
-		 * the line if we're at the start), forwards
-		 * if searching forwards (skipping a line if we're
-		 * at the end).
+		 * Use static pattern buffer since we don't need
+		 * to maintain it and won't call other pattern functions
+		 * meanwhile.
+		 * Use PAT_NOANCH because we don't need the match
+		 * anchored to the end, even if it is at the start.
 		 */
-		if (skip_pos) {
-		    if (dir < 0) {
-			if (pos == 0)
-			    skip_line = 1;
-			else
-			    pos = backwardmetafiedchar(zlemetaline,
-						       zlemetaline + pos,
-						       NULL) - zlemetaline;
-		    } else if (sbuf[0] != '^') {
-			if (pos >= strlen(zt.text) - 1)
-			    skip_line = 1;
-			else
-			    pos += 1;
-		    } else
-			skip_line = 1;
+		int patflags = PAT_STATIC|PAT_NOANCH;
+		if (sbuf[0] == '^') {
+		    /*
+		     * We'll handle the anchor later when
+		     * we call into the globbing code.
+		     */
+		    patstring = patbuf + 1;
+		} else {
+		    /* Scanning for multiple matches per line */
+		    patflags |= PAT_SCAN;
+		    patstring = patbuf;
+		}
+		if (sens == 3)
+		    patflags |= PAT_LCMATCHUC;
+		tokenize(patstring);
+		remnulargs(patstring);
+		patprog = patcompile(patstring, patflags, NULL);
+		free(patbuf);
+		if (matchlist) {
+		    freematchlist(matchlist);
+		    matchlist = NULL;
+		}
+		if (patprog) {
+		    revert_patpos = 1;
+		} else {
+		    handlefeep(zlenoargs);
+		    nomatch = 2;
+		    /* indicate "invalid" in status line */
+		    memcpy(ibuf, INVALID_TEXT, BAD_TEXT_LEN);
+		    statusline = ibuf;
+		}
+	    }
+	    /*
+	     * skip search if pattern compilation failed, or
+	     * if we back somewhere we already searched.
+	     */
+	    while ((!pattern || patprog) && !nosearch) {
+		if (patprog) {
+		    /*
+		     * We are pattern matching against the current
+		     * line.  If anchored at the start, this is
+		     * easy; a single test suffices.
+		     *
+		     * Otherwise, our strategy is to retrieve a linked
+		     * list of all matches within the current line and
+		     * scan through it as appropriate.  This isn't
+		     * actually significantly more efficient, but
+		     * it is algorithmically easier since we just
+		     * need a single one-off line-matching interface
+		     * to the pattern code.  We use a variant of
+		     * the code used for replacing within parameters
+		     * which for historical reasons is in glob.c rather
+		     * than pattern.c.
+		     *
+		     * The code for deciding whether to skip something
+		     * is a bit icky but that sort of code always is.
+		     */
+		    if (!skip_line) {
+			if (sbuf[0] == '^') {
+			    /*
+			     * skip_pos applies to the whole line in
+			     * this mode.
+			     */
+			    if (!skip_pos && pattry(patprog, zt))
+				t = zt;
+			} else {
+			    if (!matchlist && !skip_pos) {
+				if (revert_patpos) {
+				    /*
+				     * Search from where the previous
+				     * search started; see note above.
+				     */
+				    revert_patpos = 0;
+				    he = quietgethist(hl = pat_hl);
+				    zt = GETZLETEXT(he);
+				    pos = pat_pos;
+				}
+				if (!getmatchlist(zt, patprog, &matchlist) ||
+				    !firstnode(matchlist)) {
+				    if (matchlist) {
+					freematchlist(matchlist);
+					matchlist = NULL;
+				    }
+				}
+			    }
+			    if (matchlist) {
+				int newpos;
+				if (!skip_pos) {
+				    /* OK to match at current pos */
+				    newpos = pos;
+				} else {
+				    if (dir < 0)
+					newpos = pos - 1;
+				    else
+					newpos = pos + 1;
+				}
+				newpos = isearch_newpos(matchlist, newpos,
+							dir);
+				/* need a new list next time if off the end */
+				if (newpos < 0) {
+				    freematchlist(matchlist);
+				    matchlist = NULL;
+				} else
+				    t = zt + newpos;
+			    }
+			}
+		    }
 		    skip_pos = 0;
+		} else {
+		    /*
+		     * If instructed, move past a match position:
+		     * backwards if searching backwards (skipping
+		     * the line if we're at the start), forwards
+		     * if searching forwards (skipping a line if we're
+		     * at the end).
+		     */
+		    if (skip_pos) {
+			if (dir < 0) {
+			    if (pos == 0)
+				skip_line = 1;
+			    else
+				pos = backwardmetafiedchar(zlemetaline,
+							   zlemetaline + pos,
+							   NULL) - zlemetaline;
+			} else if (sbuf[0] != '^') {
+			    if (pos >= strlen(zt) - 1)
+				skip_line = 1;
+			    else
+				pos += 1;
+			} else
+			    skip_line = 1;
+			skip_pos = 0;
+		    }
+		    /*
+		     * First search for a(nother) match within the
+		     * current line, unless we've been told to skip it.
+		     */
+		    if (!skip_line) {
+			if (sbuf[0] == '^') {
+			    if (zlinecmp(zt, sbuf + 1) < sens)
+				t = zt;
+			} else
+			    t = zlinefind(zt, pos, sbuf, dir, sens);
+		    }
 		}
-		/*
-		 * First search for a(nother) match within the
-		 * current line, unless we've been told to skip it.
-		 */
-		if (!skip_line && ((sbuf[0] == '^') ?
-				   (t = (zlinecmp(zt.text, sbuf + 1) < sens
-					 ? zt.text : NULL)) :
-		    (t = zlinefind(zt.text, pos, sbuf, dir, sens)))) {
-		    zle_setline(he);
-		    pos = t - zt.text;
-		    zlemetacs = pos +
-			(dir == 1 ? sbptr - (sbuf[0] == '^') : 0);
-	    	    nomatch = 0;
-		    statusline = ibuf + NORM_PROMPT_POS;
+		if (t) {
+		    pos = t - zt;
 		    break;
 		}
 		/*
@@ -1096,45 +1302,59 @@
 		if (!(zlereadflags & ZLRF_HISTORY)
 		 || !(he = movehistent(he, dir, hist_skip_flags))) {
 		    if (sbptr == (int)isrch_spots[top_spot-1].len
-		     && (isrch_spots[top_spot-1].flags & ISS_FAILING))
+		     && (isrch_spots[top_spot-1].flags >> ISS_NOMATCH_SHIFT))
 			top_spot--;
-		    get_isrch_spot(top_spot, &hl, &pos, &zlemetacs, &sbptr,
-				   &dir, &nomatch);
+		    get_isrch_spot(top_spot, &hl, &pos, &pat_hl, &pat_pos,
+				   &zlemetacs, &sbptr, &dir, &nomatch);
 		    if (!nomatch) {
 			feep = 1;
 			nomatch = 1;
 		    }
 		    he = quietgethist(hl);
-		    zletextfree(&zt);
-		    zletext(he, &zt);
+		    zt = GETZLETEXT(he);
 		    skip_line = 0;
+		    /* indicate "failing" in status line */
+		    memcpy(ibuf, nomatch == 2 ? INVALID_TEXT :FAILING_TEXT,
+			   BAD_TEXT_LEN);
 		    statusline = ibuf;
 		    break;
 		}
 		hl = he->histnum;
-		zletextfree(&zt);
-		zletext(he, &zt);
-		pos = (dir == 1) ? 0 : strlen(zt.text);
+		zt = GETZLETEXT(he);
+		pos = (dir == 1) ? 0 : strlen(zt);
 		skip_line = isset(HISTFINDNODUPS)
 		    ? !!(he->node.flags & HIST_DUP)
-		    : !strcmp(zt.text, last_line);
+		    : !strcmp(zt, last_line);
+	    }
+	    /*
+	     * If we matched above (t set), set the new line.
+	     * If we didn't, but are here because we are on a previous
+	     * match (nosearch set and nomatch not, set the line again).
+	     */
+	    if (t || (nosearch && !nomatch)) {
+		zle_setline(he);
+		zlemetacs = pos +
+		    (dir == 1 ? sbptr - (sbuf[0] == '^') : 0);
+		statusline = ibuf + NORM_PROMPT_POS;
+		nomatch = 0;
 	    }
 	} else {
 	    top_spot = 0;
     	    nomatch = 0;
 	    statusline = ibuf + NORM_PROMPT_POS;
 	}
+	nosearch = 0;
 	sbuf[sbptr] = '_';
 	sbuf[sbptr+1] = '\0';
     ref:
 	zrefresh();
 	if (!(cmd = getkeycmd()) || cmd == Th(z_sendbreak)) {
 	    int i;
-	    get_isrch_spot(0, &hl, &pos, &i, &sbptr, &dir, &nomatch);
+	    get_isrch_spot(0, &hl, &pos, &pat_hl, &pat_pos,
+			   &i, &sbptr, &dir, &nomatch);
 	    he = quietgethist(hl);
 	    zle_setline(he);
-	    zletextfree(&zt);
-	    zletext(he, &zt);
+	    zt = GETZLETEXT(he);
 	    zlemetacs = i;
 	    break;
 	}
@@ -1150,18 +1370,26 @@
 	    goto ref;
 	} else if(cmd == Th(z_vibackwarddeletechar) ||
 	    	cmd == Th(z_backwarddeletechar)) {
-	    if (top_spot)
-		get_isrch_spot(--top_spot, &hl, &pos, &zlemetacs, &sbptr,
-			       &dir, &nomatch);
-	    else
+	    if (top_spot) {
+		get_isrch_spot(--top_spot, &hl, &pos, &pat_hl, &pat_pos,
+			       &zlemetacs, &sbptr, &dir, &nomatch);
+		patprog = NULL;
+		nosearch = 1;
+	    } else
 		feep = 1;
 	    if (nomatch) {
+		memcpy(ibuf, nomatch == 2 ? INVALID_TEXT : FAILING_TEXT,
+		       BAD_TEXT_LEN);
 		statusline = ibuf;
 		skip_pos = 1;
 	    }
 	    he = quietgethist(hl);
-	    zletextfree(&zt);
-	    zletext(he, &zt);
+	    zt = GETZLETEXT(he);
+	    /*
+	     * Set the line for the cases where we won't go passed
+	     * the usual line-setting logic:  if we're not on a match,
+	     * or if we don't have enough to search for.
+	     */
 	    if (nomatch || !sbptr || (sbptr == 1 && sbuf[0] == '^')) {
 		int i = zlemetacs;
 		zle_setline(he);
@@ -1182,27 +1410,41 @@
 	} else if(cmd == Th(z_acceptline)) {
 	    exitfn = acceptline;
 	    break;
-	} else if(cmd == Th(z_historyincrementalsearchbackward)) {
-	    set_isrch_spot(top_spot++, hl, pos, zlemetacs, sbptr, dir, nomatch);
+	} else if(cmd == Th(z_historyincrementalsearchbackward) ||
+		  cmd == Th(z_historyincrementalpatternsearchbackward)) {
+	    pat_hl = hl;
+	    pat_pos = pos;
+	    set_isrch_spot(top_spot++, hl, pos, pat_hl, pat_pos,
+			   zlemetacs, sbptr, dir, nomatch);
 	    if (dir != -1)
 		dir = -1;
 	    else
 		skip_pos = 1;
 	    goto rpt;
-	} else if(cmd == Th(z_historyincrementalsearchforward)) {
-	    set_isrch_spot(top_spot++, hl, pos, zlemetacs, sbptr, dir, nomatch);
+	} else if(cmd == Th(z_historyincrementalsearchforward) ||
+		  cmd == Th(z_historyincrementalpatternsearchforward)) {
+	    pat_hl = hl;
+	    pat_pos = pos;
+	    set_isrch_spot(top_spot++, hl, pos, pat_hl, pat_pos,
+			   zlemetacs, sbptr, dir, nomatch);
 	    if (dir != 1)
 		dir = 1;
 	    else
 		skip_pos = 1;
 	    goto rpt;
 	} else if(cmd == Th(z_virevrepeatsearch)) {
-	    set_isrch_spot(top_spot++, hl, pos, zlemetacs, sbptr, dir, nomatch);
+	    pat_hl = hl;
+	    pat_pos = pos;
+	    set_isrch_spot(top_spot++, hl, pos, pat_hl, pat_pos,
+			   zlemetacs, sbptr, dir, nomatch);
 	    dir = -odir;
 	    skip_pos = 1;
 	    goto rpt;
 	} else if(cmd == Th(z_virepeatsearch)) {
-	    set_isrch_spot(top_spot++, hl, pos, zlemetacs, sbptr, dir, nomatch);
+	    pat_hl = hl;
+	    pat_pos = pos;
+	    set_isrch_spot(top_spot++, hl, pos, pat_hl, pat_pos,
+			   zlemetacs, sbptr, dir, nomatch);
 	    dir = odir;
 	    skip_pos = 1;
 	rpt:
@@ -1254,7 +1496,8 @@
 		feep = 1;
 		continue;
 	    }
-	    set_isrch_spot(top_spot++, hl, pos, zlemetacs, sbptr, dir, nomatch);
+	    set_isrch_spot(top_spot++, hl, pos, pat_hl, pat_pos,
+			   zlemetacs, sbptr, dir, nomatch);
 	    if (sbptr >= sibuf - FIRST_SEARCH_CHAR - 2 
 #ifdef MULTIBYTE_SUPPORT
 		- 2 * MB_CUR_MAX
@@ -1269,6 +1512,7 @@
 	     * always valid at this point.
 	     */
 	    sbptr += zlecharasstring(LASTFULLCHAR, sbuf + sbptr);
+	    patprog = NULL;
 	}
 	if (feep)
 	    handlefeep(zlenoargs);
@@ -1285,15 +1529,14 @@
 	exitfn(zlenoargs);
     selectkeymap(okeymap, 1);
     zsfree(okeymap);
+    if (matchlist)
+	freematchlist(matchlist);
     /*
      * Don't allow unused characters provided as a string to the
      * widget to overflow and be used as separated commands.
      */
     if (savekeys >= 0 && kungetct > savekeys)
 	kungetct = savekeys;
-    if (last_line)
-	free(last_line);
-    zletextfree(&zt);
 }
 
 static Histent
@@ -1302,15 +1545,10 @@
     metafy_line();
     for (he = movehistent(he, -2, HIST_FOREIGN);
 	 he; he = movehistent(he, -1, HIST_FOREIGN)) {
-	struct zle_text zt;
-	zletext(he, &zt);
-
-	if (!zlinecmp(zt.text, zlemetaline)) {
+	if (!zlinecmp(GETZLETEXT(he), zlemetaline)) {
 	    unmetafy_line();
-	    zletextfree(&zt);
 	    return movehistent(he, 1, HIST_FOREIGN);
 	}
-	zletextfree(&zt);
     }
     unmetafy_line();
     return NULL;
@@ -1541,7 +1779,7 @@
 {
     Histent he;
     int n = zmult;
-    struct zle_text zt;
+    char *zt;
 
     if (!visrchstr)
 	return 1;
@@ -1555,18 +1793,16 @@
     while ((he = movehistent(he, visrchsense, hist_skip_flags))) {
 	if (isset(HISTFINDNODUPS) && he->node.flags & HIST_DUP)
 	    continue;
-	zletext(he, &zt);
-	if (zlinecmp(zt.text, zlemetaline) &&
-	    (*visrchstr == '^' ? strpfx(zt.text, visrchstr + 1) :
-	     zlinefind(zt.text, 0, visrchstr, 1, 1) != 0)) {
+	zt = GETZLETEXT(he);
+	if (zlinecmp(zt, zlemetaline) &&
+	    (*visrchstr == '^' ? strpfx(zt, visrchstr + 1) :
+	     zlinefind(zt, 0, visrchstr, 1, 1) != 0)) {
 	    if (--n <= 0) {
 		unmetafy_line();
-		zletextfree(&zt);
 		zle_setline(he);
 		return 0;
 	    }
 	}
-	zletextfree(&zt);
     }
     unmetafy_line();
     return 1;
@@ -1594,7 +1830,7 @@
     Histent he;
     int cpos = zlecs;		/* save cursor position */
     int n = zmult;
-    struct zle_text zt;
+    char *zt;
 
     if (zmult < 0) {
 	int ret;
@@ -1611,22 +1847,20 @@
 	char sav;
 	if (isset(HISTFINDNODUPS) && he->node.flags & HIST_DUP)
 	    continue;
-	zletext(he, &zt);
+	zt = GETZLETEXT(he);
 	sav = zlemetaline[zlemetacs];
 	zlemetaline[zlemetacs] = '\0';
-	tst = zlinecmp(zt.text, zlemetaline);
+	tst = zlinecmp(zt, zlemetaline);
 	zlemetaline[zlemetacs] = sav;
-	if (tst < 0 && zlinecmp(zt.text, zlemetaline)) {
+	if (tst < 0 && zlinecmp(zt, zlemetaline)) {
 	    if (--n <= 0) {
 		unmetafy_line();
-		zletextfree(&zt);
 		zle_setline(he);
 		zlecs = cpos;
 		CCRIGHT();
 		return 0;
 	    }
 	}
-	zletextfree(&zt);
     }
     unmetafy_line();
     return 1;
@@ -1642,7 +1876,7 @@
     Histent he;
     int cpos = zlecs;		/* save cursor position */
     int n = zmult;
-    struct zle_text zt;
+    char *zt;
 
     if (zmult < 0) {
 	int ret;
@@ -1659,22 +1893,20 @@
 	int tst;
 	if (isset(HISTFINDNODUPS) && he->node.flags & HIST_DUP)
 	    continue;
-	zletext(he, &zt);
+	zt = GETZLETEXT(he);
 	sav = zlemetaline[zlemetacs];
 	zlemetaline[zlemetacs] = '\0';
-	tst = zlinecmp(zt.text, zlemetaline) < (he->histnum == curhist);
+	tst = zlinecmp(zt, zlemetaline) < (he->histnum == curhist);
 	zlemetaline[zlemetacs] = sav;
-	if (tst && zlinecmp(zt.text, zlemetaline)) {
+	if (tst && zlinecmp(zt, zlemetaline)) {
 	    if (--n <= 0) {
 		unmetafy_line();
-		zletextfree(&zt);
 		zle_setline(he);
 		zlecs = cpos;
 		CCRIGHT();
 		return 0;
 	    }
 	}
-	zletextfree(&zt);
     }
     unmetafy_line();
     return 1;

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PATCH: pattern incremental search
  2008-04-26 19:41 PATCH: pattern incremental search Peter Stephenson
@ 2008-04-26 20:22 ` Peter Stephenson
  2008-04-26 20:35   ` Peter Stephenson
  2008-04-26 23:52 ` Bart Schaefer
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Peter Stephenson @ 2008-04-26 20:22 UTC (permalink / raw)
  To: Zsh hackers list

Peter Stephenson wrote:
> I think this is now good enough for some beta testing, though I'm sure
> there must be glitches.

Just discovered this.  I hope this is the right fix.

Index: Src/Zle/zle_hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_hist.c,v
retrieving revision 1.44
diff -u -r1.44 zle_hist.c
--- Src/Zle/zle_hist.c	26 Apr 2008 19:51:09 -0000	1.44
+++ Src/Zle/zle_hist.c	26 Apr 2008 20:21:50 -0000
@@ -1074,6 +1074,14 @@
      */
     int revert_patpos = 0;
     /*
+     * Another nasty feature related to the above.  When
+     * we revert the position, we might advance the search to
+     * the same line again.  When we do this the test for ignoring
+     * duplicates may trigger.  This flag indicates that in this
+     * case it's OK.
+     */
+    int dup_ok = 0;
+    /*
      * savekeys records the unget buffer, so that if we have arguments
      * they don't pollute the input.
      * feep indicates we should feep.  This is a well-known word
@@ -1219,6 +1227,7 @@
 				     * search started; see note above.
 				     */
 				    revert_patpos = 0;
+				    dup_ok = 1;
 				    he = quietgethist(hl = pat_hl);
 				    zt = GETZLETEXT(he);
 				    pos = pat_pos;
@@ -1322,10 +1331,14 @@
 		hl = he->histnum;
 		zt = GETZLETEXT(he);
 		pos = (dir == 1) ? 0 : strlen(zt);
-		skip_line = isset(HISTFINDNODUPS)
-		    ? !!(he->node.flags & HIST_DUP)
-		    : !strcmp(zt, last_line);
+		if (dup_ok)
+		    skip_line = 0;
+		else
+		    skip_line = isset(HISTFINDNODUPS)
+			? !!(he->node.flags & HIST_DUP)
+			: !strcmp(zt, last_line);
 	    }
+	    dup_ok = 0;
 	    /*
 	     * If we matched above (t set), set the new line.
 	     * If we didn't, but are here because we are on a previous


-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PATCH: pattern incremental search
  2008-04-26 20:22 ` Peter Stephenson
@ 2008-04-26 20:35   ` Peter Stephenson
  2008-04-26 20:51     ` Peter Stephenson
  0 siblings, 1 reply; 13+ messages in thread
From: Peter Stephenson @ 2008-04-26 20:35 UTC (permalink / raw)
  To: Zsh hackers list

And this one... if a search fails, nothing happens, then if you
backtrack and type another characer, when a search again succeeds you
get a feep.  Am i missing something, or isn't this too feeping late?

Index: Src/Zle/zle_hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_hist.c,v
retrieving revision 1.45
diff -u -r1.45 zle_hist.c
--- Src/Zle/zle_hist.c	26 Apr 2008 20:27:39 -0000	1.45
+++ Src/Zle/zle_hist.c	26 Apr 2008 20:32:12 -0000
@@ -1357,6 +1357,10 @@
 	    statusline = ibuf + NORM_PROMPT_POS;
 	}
 	nosearch = 0;
+	if (feep) {
+	    handlefeep(zlenoargs);
+	    feep = 0;
+	}
 	sbuf[sbptr] = '_';
 	sbuf[sbptr+1] = '\0';
     ref:


-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PATCH: pattern incremental search
  2008-04-26 20:35   ` Peter Stephenson
@ 2008-04-26 20:51     ` Peter Stephenson
  0 siblings, 0 replies; 13+ messages in thread
From: Peter Stephenson @ 2008-04-26 20:51 UTC (permalink / raw)
  To: Zsh hackers list

Feeping awful, part 2...  I'll just commit any more I find on this
scale without posting.

There might be argument for even less feeping on invalid patterns.  You
do get a prompt, after all, and many complicated patterns go through
this stage.

Index: Src/Zle/zle_hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_hist.c,v
retrieving revision 1.46
diff -u -r1.46 zle_hist.c
--- Src/Zle/zle_hist.c	26 Apr 2008 20:40:18 -0000	1.46
+++ Src/Zle/zle_hist.c	26 Apr 2008 20:49:30 -0000
@@ -1179,8 +1179,10 @@
 		if (patprog) {
 		    revert_patpos = 1;
 		} else {
-		    handlefeep(zlenoargs);
-		    nomatch = 2;
+		    if (nomatch != 2) {
+			handlefeep(zlenoargs);
+			nomatch = 2;
+		    }
 		    /* indicate "invalid" in status line */
 		    memcpy(ibuf, INVALID_TEXT, BAD_TEXT_LEN);
 		    statusline = ibuf;


-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PATCH: pattern incremental search
  2008-04-26 19:41 PATCH: pattern incremental search Peter Stephenson
  2008-04-26 20:22 ` Peter Stephenson
@ 2008-04-26 23:52 ` Bart Schaefer
  2008-04-28  1:35 ` Geoff Wing
       [not found] ` <CAHYJk3Q5x=5Zn5kURXDDgLLoSEsro45_G=SZB-P+qX_qk7dn-Q@mail.gmail.com>
  3 siblings, 0 replies; 13+ messages in thread
From: Bart Schaefer @ 2008-04-26 23:52 UTC (permalink / raw)
  To: Zsh hackers list

On Apr 26,  8:41pm, Peter Stephenson wrote:
}
} I think this is now good enough for some beta testing, though I'm sure
} there must be glitches.  It adds
} history-incremental-pattern-search-backward and
} history-incremental-pattern-search-forkward which are ridiculously long

And rather amusingly spelt, too.  (I wouldn't even mention it, but you
made the same typo in NEWS.)

} but nothing shorter really fits in with the names we have already.

Maybe the word "history" doesn't need to be there.  It's either obvious
or wrong (depending on, say, whether you're (not) inside zed).


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PATCH: pattern incremental search
  2008-04-26 19:41 PATCH: pattern incremental search Peter Stephenson
  2008-04-26 20:22 ` Peter Stephenson
  2008-04-26 23:52 ` Bart Schaefer
@ 2008-04-28  1:35 ` Geoff Wing
       [not found] ` <CAHYJk3Q5x=5Zn5kURXDDgLLoSEsro45_G=SZB-P+qX_qk7dn-Q@mail.gmail.com>
  3 siblings, 0 replies; 13+ messages in thread
From: Geoff Wing @ 2008-04-28  1:35 UTC (permalink / raw)
  To: Zsh Hackers

glob.c:
[...]
:+ * Repldata matches in the linked list *replistp; this is
[...]
:+getmatchlist(char *str, Patprog p, LinkList *repllistp)
[...]

s/replistp/repllistp/ (or the other way) please.

Regards,
Geoff


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PATCH: pattern incremental search
       [not found] ` <CAHYJk3Q5x=5Zn5kURXDDgLLoSEsro45_G=SZB-P+qX_qk7dn-Q@mail.gmail.com>
@ 2021-12-20  3:40   ` Mikael Magnusson
  2021-12-20 12:10     ` Peter Stephenson
  0 siblings, 1 reply; 13+ messages in thread
From: Mikael Magnusson @ 2021-12-20  3:40 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

[forgot to change the ml address from sunsite.dk]

On 12/20/21, Mikael Magnusson <mikachu@gmail.com> wrote:
> On 4/26/08, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
>> I think this is now good enough for some beta testing, though I'm sure
>> there must be glitches.  It adds
>> history-incremental-pattern-search-backward and
>> history-incremental-pattern-search-forkward which are ridiculously long
>> names but nothing shorter really fits in with the names we have already.
>>
>> The limitation that only non-overlapping pattern matches on the same
>> line are found comes from the parameter substitution code I hijacked to
>> do this.  It's not needed in this case since there's no substitution.
>> If anyone can see a good reason I can alter it.
>>
>> I've also finished the work of the previous patch that put history
>> searching back to using unmetafied strings: it was no longer necessary
>> to allocate additional space for each history line at all (unless it was
>> modified), so now it doesn't.  This should be a significant saving for
>> searching large histories.
>>
>> Index: Src/glob.c
>> ===================================================================
>> RCS file: /cvsroot/zsh/zsh/Src/glob.c,v
>> retrieving revision 1.61
>> diff -u -r1.61 glob.c
>> --- Src/glob.c	1 Nov 2007 17:57:57 -0000	1.61
>> +++ Src/glob.c	26 Apr 2008 19:40:43 -0000
>> @@ -2050,12 +2050,6 @@
>>  /* do the ${foo%%bar}, ${foo#bar} stuff */
>>  /* please do not laugh at this code. */
>>
>> -struct repldata {
>> -    int b, e;			/* beginning and end of chunk to replace */
>> -    char *replstr;		/* replacement string to use */
>> -};
>> -typedef struct repldata *Repldata;
>> -
>>  /* Having found a match in getmatch, decide what part of string
>>   * to return.  The matched part starts b characters into string s
>>   * and finishes e characters in: 0 <= b <= e <= strlen(s)
>> @@ -2073,19 +2067,23 @@
>>      char buf[80], *r, *p, *rr;
>>      int ll = 0, l = strlen(s), bl = 0, t = 0, i;
>>
>> -    if (replstr) {
>> +    if (replstr || (fl & SUB_LIST)) {
>>  	if (fl & SUB_DOSUBST) {
>>  	    replstr = dupstring(replstr);
>>  	    singsub(&replstr);
>>  	    untokenize(replstr);
>>  	}
>> -	if ((fl & SUB_GLOBAL) && repllist) {
>> +	if ((fl & (SUB_GLOBAL|SUB_LIST)) && repllist) {
>>  	    /* We are replacing the chunk, just add this to the list */
>> -	    Repldata rd = (Repldata) zhalloc(sizeof(*rd));
>> +	    Repldata rd = (Repldata)
>> +		((fl & SUB_LIST) ? zalloc(sizeof(*rd)) : zhalloc(sizeof(*rd)));
>>  	    rd->b = b;
>>  	    rd->e = e;
>>  	    rd->replstr = replstr;
>> -	    addlinknode(repllist, rd);
>> +	    if (fl & SUB_LIST)
>> +		zaddlinknode(repllist, rd);
>> +	    else
>> +		addlinknode(repllist, rd);
>>  	    return s;
>>  	}
>>  	ll += strlen(replstr);
>
> Someone in the irc channel reported a crash on this strlen when doing
> history-incremental-pattern-search-backward with any search, and they
> can reproduce it with the latest git version too, they posted this
> backtrace:
>
> Program received signal SIGSEGV, Segmentation fault.
> __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:65
> 65              VPCMPEQ (%rdi), %ymm0, %ymm1
> (gdb) back
> #0  __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:65
> #1  0x0000000000436eaa in get_match_ret (imd=imd@entry=0x7fffffffd460,
>     b=<optimized out>, e=0) at glob.c:2571
> #2  0x000000000043742f in igetmatch (sp=sp@entry=0x7fffffffd4d8,
>     p=<optimized out>, fl=fl@entry=8710, n=<optimized out>, n@entry=0,
>     replstr=replstr@entry=0x0, repllistp=<optimized out>) at glob.c:3309
> #3  0x000000000043d7b0 in getmatchlist (str=<optimized out>,
>     p=<optimized out>, repllistp=<optimized out>) at glob.c:2743
> #4  0x00007ffff774d4f6 in ?? () from /usr/lib64/zsh/5.8.0.2-dev/zsh/zle.so
> #5  0x0000000000000001 in ?? ()
> #6  0x0000000000000000 in ?? ()
>
> I'm not very familiar with this code, but it looks like this patch
> allows going into this path with a NULL replstr on purpose, but it
> also looks like it shouldn't work... I never had any issues with it
> before though, and I do use pattern search on ^R.
>
> --
> Mikael Magnusson


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PATCH: pattern incremental search
  2021-12-20  3:40   ` Mikael Magnusson
@ 2021-12-20 12:10     ` Peter Stephenson
  2022-02-28 16:54       ` Madhu
  0 siblings, 1 reply; 13+ messages in thread
From: Peter Stephenson @ 2021-12-20 12:10 UTC (permalink / raw)
  To: Zsh hackers list

On 12/20/21, Mikael Magnusson <mikachu@gmail.com> wrote:
> On 4/26/08, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
>> -    if (replstr) {
>> +    if (replstr || (fl & SUB_LIST)) {
> Someone in the irc channel reported a crash on this strlen when doing
> history-incremental-pattern-search-backward with any search, and they
> can reproduce it with the latest git version too, they posted this
> backtrace:

That extra test doesn't look like it makes any sense --- I think it
may just be in completely the wrong place and shouldn't be in
get_match_ret() at all since it's similar to some checks in other
places where we allow zero-length (but not NULL) strings for some
edge cases in some variants of matching.  We should probably
just remove it and see what happens.

pws

diff --git a/Src/glob.c b/Src/glob.c
index bee890caf..375671cea 100644
--- a/Src/glob.c
+++ b/Src/glob.c
@@ -2549,7 +2549,7 @@ get_match_ret(Imatchdata imd, int b, int e)
     e += add;
 
     /* Everything now refers to metafied lengths. */
-    if (replstr || (fl & SUB_LIST)) {
+    if (replstr) {
 	if (fl & SUB_DOSUBST) {
 	    replstr = dupstring(replstr);
 	    singsub(&replstr);


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PATCH: pattern incremental search
  2021-12-20 12:10     ` Peter Stephenson
@ 2022-02-28 16:54       ` Madhu
  2022-03-31 22:44         ` Lawrence Velázquez
  0 siblings, 1 reply; 13+ messages in thread
From: Madhu @ 2022-02-28 16:54 UTC (permalink / raw)
  To: zsh-workers; +Cc: p.w.stephenson

[-- Attachment #1: Type: Text/Plain, Size: 1916 bytes --]

* Peter Stephenson <1514882387.357077.1640002234148@mail2.virginmedia.com> :
Wrote on Mon, 20 Dec 2021 12:10:34 +0000 (GMT):

> On 12/20/21, Mikael Magnusson <mikachu@gmail.com> wrote:
>> On 4/26/08, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
>>> - if (replstr) { + if (replstr || (fl & SUB_LIST)) {
>> Someone in the irc channel reported a crash on this strlen when
>> doing history-incremental-pattern-search-backward with any search,
>> and they can reproduce it with the latest git version too, they
>> posted this backtrace:
>
> That extra test doesn't look like it makes any sense --- I think it
> may just be in completely the wrong place and shouldn't be in
> get_match_ret() at all since it's similar to some checks in other
> places where we allow zero-length (but not NULL) strings for some edge
> cases in some variants of matching.  We should probably just remove it
> and see what happens.
>
> pws
>
> diff --git a/Src/glob.c b/Src/glob.c index bee890caf..375671cea 100644
> --- a/Src/glob.c +++ b/Src/glob.c @@ -2549,7 +2549,7 @@
> get_match_ret(Imatchdata imd, int b, int e) e += add;
>      /* Everything now refers to metafied lengths. */ - if (replstr ||
> (fl & SUB_LIST)) { + if (replstr) { if (fl & SUB_DOSUBST) { replstr =
> dupstring(replstr); singsub(&replstr);

This doesn't fix the segfault: which just gets postponed.  Besides
this breaks incremental-pattern-search, which just stops working and
doesn't match anything in the history. Also, the segfault only occurs
when zsh is built without multibyte.

To hit the segfault, in a --disable-multibyte build
$ zsh -f
$ bindkey ^R history-incremental-pattern-search-backward
C-r .

Please consider the attached patch which 1) reverts the above fix, and
2) modifies the non-multibyte version of igetmatch to match the
multibyte version at some points. (Disclaimer. this is submitted with
no understanding of what the code does :)


[-- Attachment #2: glob.c-fix-segfault-on-non-multibyte-history-inc.patch --]
[-- Type: Text/X-Patch, Size: 2210 bytes --]

From 81e59a30fa593a8bfd10ad8a28a7475803d5f839 Mon Sep 17 00:00:00 2001
From: Madhu <enometh@net.meer>
Date: Mon, 28 Feb 2022 22:04:09 +0530
Subject: [PATCH] Src/glob.c: fix segfault on non-multibyte
 history-incremental-pattern-search-backward

* Src/glob.c: (get_match_ret): revert the fix in
7f240e6aa9f5596a129474ba6294875dfe7ae264. With this patch ^R stops
working altogether.  (igetmatch): cargo cult port code from the ifdef
MULTIBYTE_SUPPORT version to the ifndef MULTIBYTE_VERSION. This seems
to fix the segfault.
---
 Src/glob.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/Src/glob.c b/Src/glob.c
index 375671c..7e2d810 100644
--- a/Src/glob.c
+++ b/Src/glob.c
@@ -2549,7 +2549,7 @@ get_match_ret(Imatchdata imd, int b, int e)
     e += add;
 
     /* Everything now refers to metafied lengths. */
-    if (replstr) {
+    if (replstr || (fl & SUB_LIST)) {
 	if (fl & SUB_DOSUBST) {
 	    replstr = dupstring(replstr);
 	    singsub(&replstr);
@@ -3351,7 +3351,7 @@ igetmatch(char **sp, Patprog p, int fl, int n, char *replstr,
 	    /* longest or smallest at start with substrings */
 	    t = s;
 	    if (fl & SUB_GLOBAL) {
-		imd.repllist = newlinklist();
+		imd.repllist = (fl & SUB_LIST) ? znewlinklist() : newlinklist();
 		if (repllistp)
 		    *repllistp = imd.repllist;
 	    }
@@ -3481,6 +3481,7 @@ igetmatch(char **sp, Patprog p, int fl, int n, char *replstr,
 	 * Results from get_match_ret in repllist are all metafied.
 	 */
 	s = *sp;
+	if (!(fl & SUB_LIST)) {
 	i = 0;			/* start of last chunk we got from *sp */
 	for (nd = firstnode(imd.repllist); nd; incnode(nd)) {
 	    rd = (Repldata) getdata(nd);
@@ -3503,16 +3504,22 @@ igetmatch(char **sp, Patprog p, int fl, int n, char *replstr,
 	memcpy(t, s + i, l - i);
 	start[lleft] = '\0';
 	*sp = (char *)start;
+	}
 	return 1;
     }
 
+    if (fl & SUB_LIST) {	/* safety: don't think this can happen */
+	return 0;
+    }
+
     /* munge the whole string: no match, so no replstr */
     imd.replstr = NULL;
     imd.repllist = NULL;
     *sp = get_match_ret(&imd, 0, 0);
-    return 1;
+    return (fl & SUB_RETFAIL) ? 0 : 1;
 }
 
+
 /**/
 #endif /* MULTIBYTE_SUPPORT */
 
-- 
2.35.1.dirty


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PATCH: pattern incremental search
  2022-02-28 16:54       ` Madhu
@ 2022-03-31 22:44         ` Lawrence Velázquez
  2022-04-01  2:25           ` Madhu
  0 siblings, 1 reply; 13+ messages in thread
From: Lawrence Velázquez @ 2022-03-31 22:44 UTC (permalink / raw)
  To: zsh-workers; +Cc: Madhu

On Mon, Feb 28, 2022, at 11:54 AM, Madhu wrote:
> This doesn't fix the segfault: which just gets postponed.  Besides
> this breaks incremental-pattern-search, which just stops working and
> doesn't match anything in the history. Also, the segfault only occurs
> when zsh is built without multibyte.
>
> To hit the segfault, in a --disable-multibyte build
> $ zsh -f
> $ bindkey ^R history-incremental-pattern-search-backward
> C-r .
>
> Please consider the attached patch which 1) reverts the above fix, and
> 2) modifies the non-multibyte version of igetmatch to match the
> multibyte version at some points. (Disclaimer. this is submitted with
> no understanding of what the code does :)
>
>
> Attachments:
> * glob.c-fix-segfault-on-non-multibyte-history-inc.patch

Was this fully obviated by workers/49870 (git 3bf95b9)?

-- 
vq


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PATCH: pattern incremental search
  2022-03-31 22:44         ` Lawrence Velázquez
@ 2022-04-01  2:25           ` Madhu
  2022-04-01  3:49             ` Bart Schaefer
  0 siblings, 1 reply; 13+ messages in thread
From: Madhu @ 2022-04-01  2:25 UTC (permalink / raw)
  To: larryv; +Cc: zsh-workers

*  Lawrence Velázquez <larryv@zsh.org> <cc93474f-4b5e-4d69-8b7a-de148913b27e@www.fastmail.com>
Wrote on Thu, 31 Mar 2022 18:44:46 -0400

> 
> On Mon, Feb 28, 2022, at 11:54 AM, Madhu wrote:
>> This doesn't fix the segfault: which just gets postponed.  Besides
>> this breaks incremental-pattern-search, which just stops working and
>> doesn't match anything in the history. Also, the segfault only occurs
>> when zsh is built without multibyte.
>>
>> To hit the segfault, in a --disable-multibyte build
>> $ zsh -f
>> $ bindkey ^R history-incremental-pattern-search-backward
>> C-r .
>>
>> Please consider the attached patch which 1) reverts the above fix, and
>> 2) modifies the non-multibyte version of igetmatch to match the
>> multibyte version at some points. (Disclaimer. this is submitted with
>> no understanding of what the code does :)
>>
>>
>> Attachments:
>> * glob.c-fix-segfault-on-non-multibyte-history-inc.patch
>
> Was this fully obviated by workers/49870 (git
>  3bf95b9)?

No. I scanned the git commit, it probably fixes the bug introduced in
"49658: Fix NULL reference in match code." but since it doesn't touch
the code path for the case where zsh is built without multibyte, it
don't believe it addresses the problem I raised at all.

I haven't tried master yet, The steps I gave for reproducing the
problem are very basic produce a segfault everytime. If you can try it
out with a non-multibyte build I'm sure you will hit it.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PATCH: pattern incremental search
  2022-04-01  2:25           ` Madhu
@ 2022-04-01  3:49             ` Bart Schaefer
  2022-04-01 18:23               ` Bart Schaefer
  0 siblings, 1 reply; 13+ messages in thread
From: Bart Schaefer @ 2022-04-01  3:49 UTC (permalink / raw)
  To: Madhu; +Cc: Lawrence Velázquez, Zsh hackers list

On Thu, Mar 31, 2022 at 7:26 PM Madhu <enometh@meer.net> wrote:
>
> No. I scanned the git commit, it probably fixes the bug introduced in
> "49658: Fix NULL reference in match code." but since it doesn't touch
> the code path for the case where zsh is built without multibyte, it
> don't believe it addresses the problem I raised at all.

The following appears to fix the crash problem.  I don't know what
else the rest of the patch in workers/49781 accomplishes.

diff --git a/Src/Zle/zle_refresh.c b/Src/Zle/zle_refresh.c
index d9d9503e2..c8c6f78c6 100644
--- a/Src/Zle/zle_refresh.c
+++ b/Src/Zle/zle_refresh.c
@@ -255,7 +255,9 @@ int cost;
 #endif

 static const REFRESH_ELEMENT zr_cr = { ZWC('\r'), 0 };
+#ifdef MULTIBYTE_support
 static const REFRESH_ELEMENT zr_dt = { ZWC('.'), 0 };
+#endif
 static const REFRESH_ELEMENT zr_nl = { ZWC('\n'), 0 };
 static const REFRESH_ELEMENT zr_sp = { ZWC(' '), 0 };
 static const REFRESH_ELEMENT zr_zr = { ZWC('\0'), 0 };
diff --git a/Src/glob.c b/Src/glob.c
index 349862531..63939fea8 100644
--- a/Src/glob.c
+++ b/Src/glob.c
@@ -3355,7 +3355,7 @@ igetmatch(char **sp, Patprog p, int fl, int n,
char *replstr,
            /* longest or smallest at start with substrings */
            t = s;
            if (fl & SUB_GLOBAL) {
-               imd.repllist = newlinklist();
+               imd.repllist = (fl & SUB_LIST) ? znewlinklist() : newlinklist();
                if (repllistp)
                    *repllistp = imd.repllist;
            }


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PATCH: pattern incremental search
  2022-04-01  3:49             ` Bart Schaefer
@ 2022-04-01 18:23               ` Bart Schaefer
  0 siblings, 0 replies; 13+ messages in thread
From: Bart Schaefer @ 2022-04-01 18:23 UTC (permalink / raw)
  To: Madhu; +Cc: Lawrence Velázquez, Zsh hackers list

[-- Attachment #1: Type: text/plain, Size: 2199 bytes --]

                /* patmatchlen returns metafied length, as we need */
                On Thu, Mar 31, 2022 at 8:49 PM Bart Schaefer
<schaefer@brasslantern.com> wrote:
>
> The following appears to fix the crash problem.  I don't know what
> else the rest of the patch in workers/49781 accomplishes.

Hmm, sorry ... that fixes the crash when there is no match on the
search, but there is still a crash when the search does begin to match
something.

I note this commentary difference from the MULTIBYTE to not-so case,
which I hope is correct:

-                /* patmatchlen returns unmetafied length in this case */
+                /* patmatchlen returns metafied length, as we need */

Also in the MULTIBYTE branch, we have declared two "char *mpos" in
nested scopes, and I can't immediately tell whether that's a bug
waiting to be surfaced or if the not-MULTIBYTE branch just needs to
declare the outer mpos.  This has a potential effect because there's a
call to get_match_ret(&imd, t-s, mpos-s); that is in the outer (loop)
scope in the MULTIBYTE branch but the inner ("if (pattrylen(...))")
scope in the not-MULTIBYTE branch.  I would expect one of those
placements must be wrong?

Those differences aside, the hunk from workers/49781 that prevents the
crash in the successful-match case is this one:

@@ -3481,6 +3481,7 @@ igetmatch(char **sp, Patprog p, int fl, int n,
char *replstr,
      * Results from get_match_ret in repllist are all metafied.
      */
     s = *sp;
+    if (!(fl & SUB_LIST)) {
     i = 0;            /* start of last chunk we got from *sp */
     for (nd = firstnode(imd.repllist); nd; incnode(nd)) {
         rd = (Repldata) getdata(nd);

(plus the matching close-brace, of course).  SUB_LIST is explained as
/* no substitution, return list of matches */
so I'm somewhat concerned that there remains a different way to enter
this code that is still going to crash it, but I don't know how to
force that path.

Anyway, a shorter but equivalent (requires no re-indentation) fix is
included in the patch below.  I have not attempted to resolve the
"mpos" question.  I did pull in a couple of what appear to be
optimizations (early loop break) from the MULTIBYTE code.

[-- Attachment #2: glob-pattern-search.txt --]
[-- Type: text/plain, Size: 2399 bytes --]

diff --git a/Src/Zle/zle_refresh.c b/Src/Zle/zle_refresh.c
index d9d9503e2..c8c6f78c6 100644
--- a/Src/Zle/zle_refresh.c
+++ b/Src/Zle/zle_refresh.c
@@ -255,7 +255,9 @@ int cost;
 #endif
 
 static const REFRESH_ELEMENT zr_cr = { ZWC('\r'), 0 };
+#ifdef MULTIBYTE_support
 static const REFRESH_ELEMENT zr_dt = { ZWC('.'), 0 };
+#endif
 static const REFRESH_ELEMENT zr_nl = { ZWC('\n'), 0 };
 static const REFRESH_ELEMENT zr_sp = { ZWC(' '), 0 };
 static const REFRESH_ELEMENT zr_zr = { ZWC('\0'), 0 };
diff --git a/Src/glob.c b/Src/glob.c
index 349862531..d4ffc2274 100644
--- a/Src/glob.c
+++ b/Src/glob.c
@@ -3286,6 +3286,7 @@ igetmatch(char **sp, Patprog p, int fl, int n, char *replstr,
 	return 1;
     }
     if (matched) {
+        /* Default is to match at the start; see comment in MULTIBYTE above */
 	switch (fl & (SUB_END|SUB_LONG|SUB_SUBSTR)) {
 	case 0:
 	case SUB_LONG:
@@ -3355,7 +3356,7 @@ igetmatch(char **sp, Patprog p, int fl, int n, char *replstr,
 	    /* longest or smallest at start with substrings */
 	    t = s;
 	    if (fl & SUB_GLOBAL) {
-		imd.repllist = newlinklist();
+		imd.repllist = (fl & SUB_LIST) ? znewlinklist() : newlinklist();
 		if (repllistp)
 		    *repllistp = imd.repllist;
 	    }
@@ -3405,6 +3406,8 @@ igetmatch(char **sp, Patprog p, int fl, int n, char *replstr,
 			 * which is already marked for replacement.
 			 */
 			matched = 1;
+			if (t == send)
+			    break;
 			while (t < mpos) {
 			    ioff++;
 			    umlen--;
@@ -3412,8 +3415,10 @@ igetmatch(char **sp, Patprog p, int fl, int n, char *replstr,
 			}
 			break;
 		    }
+		    if (t == send)
+			break;
 		}
-	    } while (matched);
+	    } while (matched && t < send);
 	    /*
 	     * check if we can match a blank string, if so do it
 	     * at the start.  Goodness knows if this is a good idea
@@ -3485,6 +3490,8 @@ igetmatch(char **sp, Patprog p, int fl, int n, char *replstr,
 	 * Results from get_match_ret in repllist are all metafied.
 	 */
 	s = *sp;
+	if (fl & SUB_LIST)
+	    return 1;
 	i = 0;			/* start of last chunk we got from *sp */
 	for (nd = firstnode(imd.repllist); nd; incnode(nd)) {
 	    rd = (Repldata) getdata(nd);
@@ -3514,7 +3521,7 @@ igetmatch(char **sp, Patprog p, int fl, int n, char *replstr,
     imd.replstr = NULL;
     imd.repllist = NULL;
     *sp = get_match_ret(&imd, 0, 0);
-    return 1;
+    return (fl & SUB_RETFAIL) ? 0 : 1;
 }
 
 /**/

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-04-01 18:24 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-04-26 19:41 PATCH: pattern incremental search Peter Stephenson
2008-04-26 20:22 ` Peter Stephenson
2008-04-26 20:35   ` Peter Stephenson
2008-04-26 20:51     ` Peter Stephenson
2008-04-26 23:52 ` Bart Schaefer
2008-04-28  1:35 ` Geoff Wing
     [not found] ` <CAHYJk3Q5x=5Zn5kURXDDgLLoSEsro45_G=SZB-P+qX_qk7dn-Q@mail.gmail.com>
2021-12-20  3:40   ` Mikael Magnusson
2021-12-20 12:10     ` Peter Stephenson
2022-02-28 16:54       ` Madhu
2022-03-31 22:44         ` Lawrence Velázquez
2022-04-01  2:25           ` Madhu
2022-04-01  3:49             ` Bart Schaefer
2022-04-01 18:23               ` Bart Schaefer

Code repositories for project(s) associated with this inbox:

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).