PATCH: some minor fixes for zle_hist.c (3.1.4)

PATCH: some minor fixes for zle_hist.c (3.1.4)

[Wayne Davison]

I've fixed a couple minor bugs in zle_hist.c in 3.1.4:

 + The up-line and down-line code was duplicated in multiple spots
   in zle_hist.c, and among the different versions there was one
   inconsistency which I believe is a bug.  The code in the
   {up,down}lineorsearch() functions had this test:

	    if (cs && invicmdmode())
		cs--;

   rather than this one found in the {up,down}lineorhistory()
   functions:

	    if (cs > findbol() && invicmdmode())
		cs--;

   I believe the former is a bug when moving through a multi-line
   history entry.

   Rather than just fix this inconsistency, I decided to integrate
   the up-line code into its own function, upline(), and the down-
   line code into its own function, downline().

 + In viuplineorhistory(), if zmult was negative it called
   downlineorhistory() rather than vidownlineorhistory().  Similarly,
   vidownlineorhistory() called the wrong up-line function.

   Rather than just fix this inconsistency, I decided to eliminate
   the duplicated code by having the vi versions of these functions
   call the normal versions and just tweak cs afterwards.  This
   results in identical functionality, as far as I could tell.

   My new code does ensure that the vi{up,down}lineorhistory
   functions don't change lastcol since the old functions left it
   alone, but I'm not sure that this is really necessary.

This patch can be applied before or after applying my previous
patch for historysearch{for,back}ward.

..wayne..

---8<------8<------8<------8<---cut here--->8------>8------>8------>8---
Index: zle_hist.c
@@ -96,16 +96,16 @@
 }
 
 /**/
-void
-uplineorhistory(void)
+int
+upline(void)
 {
-    int ocs = cs, n = zmult;
+    int n = zmult;
 
     if (n < 0) {
-	zmult = -n;
-	downlineorhistory();
-	zmult = n;
-	return;
+	zmult = -zmult;
+	n = downline();
+	zmult = -zmult;
+	return n;
     }
     if (lastcol == -1)
 	lastcol = cs - findbol();
@@ -117,18 +117,7 @@
 	cs = findbol();
 	n--;
     }
-    if (n) {
-	int m = zmult;
-
-	cs = ocs;
-	if (virangeflag || !histallowed) {
-	    feep();
-	    return;
-	}
-	zmult = n;
-	uphistory();
-	zmult = m;
-    } else {
+    if (!n) {
 	int x = findeol();
 
 	if ((cs += lastcol) >= x) {
@@ -137,28 +126,15 @@
 		cs--;
 	}
     }
+    return n;
 }
 
 /**/
 void
-viuplineorhistory(void)
+uplineorhistory(void)
 {
-    int ocs = cs, n = zmult;
-
-    if (n < 0) {
-	zmult = -n;
-	downlineorhistory();
-	zmult = n;
-	return;
-    }
-    cs = findbol();
-    while (n) {
-	if (!cs)
-	    break;
-	cs--;
-	cs = findbol();
-	n--;
-    }
+    int ocs = cs;
+    int n = upline();
     if (n) {
 	int m = zmult;
 
@@ -171,6 +147,15 @@
 	uphistory();
 	zmult = m;
     }
+}
+
+/**/
+void
+viuplineorhistory(void)
+{
+    int col = lastcol;
+    uplineorhistory();
+    lastcol = col;
     vifirstnonblank();
 }
 
@@ -179,24 +164,8 @@
 void
 uplineorsearch(void)
 {
-    int ocs = cs, n = zmult;
-
-    if (n < 0) {
-	zmult = -n;
-	downlineorsearch();
-	zmult = n;
-	return;
-    }
-    if (lastcol == -1)
-	lastcol = cs - findbol();
-    cs = findbol();
-    while (n) {
-	if (!cs)
-	    break;
-	cs--;
-	cs = findbol();
-	n--;
-    }
+    int ocs = cs;
+    int n = upline();
     if (n) {
 	int m = zmult;
 
@@ -208,28 +177,20 @@
 	zmult = n;
 	historysearchbackward();
 	zmult = m;
-    } else {
-	int x = findeol();
-
-	if ((cs += lastcol) >= x) {
-	    cs = x;
-	    if (cs && invicmdmode())
-		cs--;
-	}
     }
 }
 
 /**/
-void
-downlineorhistory(void)
+int
+downline(void)
 {
-    int ocs = cs, n = zmult;
+    int n = zmult;
 
     if (n < 0) {
-	zmult = -n;
-	uplineorhistory();
-	zmult = n;
-	return;
+	zmult = -zmult;
+	n = upline();
+	zmult = -zmult;
+	return n;
     }
     if (lastcol == -1)
 	lastcol = cs - findbol();
@@ -241,18 +202,7 @@
 	cs = x + 1;
 	n--;
     }
-    if (n) {
-	int m = zmult;
-
-	cs = ocs;
-	if (virangeflag || !histallowed) {
-	    feep();
-	    return;
-	}
-	zmult = n;
-	downhistory();
-	zmult = m;
-    } else {
+    if (!n) {
 	int x = findeol();
 
 	if ((cs += lastcol) >= x) {
@@ -261,28 +211,15 @@
 		cs--;
 	}
     }
+    return n;
 }
 
 /**/
 void
-vidownlineorhistory(void)
+downlineorhistory(void)
 {
-    int ocs = cs, n = zmult;
-
-    if (n < 0) {
-	zmult = -n;
-	uplineorhistory();
-	zmult = n;
-	return;
-    }
-    while (n) {
-	int x = findeol();
-
-	if (x == ll)
-	    break;
-	cs = x + 1;
-	n--;
-    }
+    int ocs = cs;
+    int n = downline();
     if (n) {
 	int m = zmult;
 
@@ -295,6 +232,15 @@
 	downhistory();
 	zmult = m;
     }
+}
+
+/**/
+void
+vidownlineorhistory(void)
+{
+    int col = lastcol;
+    downlineorhistory();
+    lastcol = col;
     vifirstnonblank();
 }
 
@@ -302,24 +248,8 @@
 void
 downlineorsearch(void)
 {
-    int ocs = cs, n = zmult;
-
-    if (n < 0) {
-	zmult = -n;
-	uplineorsearch();
-	zmult = n;
-	return;
-    }
-    if (lastcol == -1)
-	lastcol = cs - findbol();
-    while (n) {
-	int x = findeol();
-
-	if (x == ll)
-	    break;
-	cs = x + 1;
-	n--;
-    }
+    int ocs = cs;
+    int n = downline();
     if (n) {
 	int m = zmult;
 
@@ -331,14 +261,6 @@
 	zmult = n;
 	historysearchforward();
 	zmult = m;
-    } else {
-	int x = findeol();
-
-	if ((cs += lastcol) >= x) {
-	    cs = x;
-	    if (cs && invicmdmode())
-		cs--;
-	}
     }
 }
 
---8<------8<------8<------8<---cut here--->8------>8------>8------>8---

Re: PATCH: some minor fixes for zle_hist.c (3.1.4)

[Bart Schaefer]

On Jun 10,  3:56am, Wayne Davison wrote:
} Subject: PATCH: some minor fixes for zle_hist.c (3.1.4)
}
} I've fixed a couple minor bugs in zle_hist.c in 3.1.4:
} 
}  + The up-line and down-line code was duplicated in multiple spots
}    in zle_hist.c, and among the different versions there was one
}    inconsistency which I believe is a bug.

I'm staring at this code and I've come to the conclusion that if it
works the same as the old code, then the old code had more bugs than
Wayne fixed.

I would have expected zmult to indicate how many previous occurrences
of the first word on the current line to search through. However,
historysearchbackward() ignores zmult entirely, and uplineorsearch()
attempts to move up zmult lines (and reduces zmult by the number of
lines it moved) -before- beginning the search ...

So a multiplier on up-line-or-search means "move up N lines in the edit
buffer, or if you can't move N lines, then search the history for the
first word on the first line in the edit buffer."

Does that seem as useless to anyone else as it does to me?

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

Re: PATCH: some minor fixes for zle_hist.c (3.1.4)

[Wayne Davison]

"Bart Schaefer" writes:
> I would have expected zmult to indicate how many previous occurrences
> of the first word on the current line to search through. However,
> historysearchbackward() ignores zmult entirely

I had meant to comment on this and ask why this was.  I think its
weird for zmult to be ignored like this, but it appears to be
consistently ignored throughout the various search functions (e.g.
history-beginning-search-* ignores it too, and history-incremental-
search-* uses it to determine if case is to be ignored in the
search).

If the code is not meant to work this way, the following patch will
make zmult now affect these commands:

	{vi-,}history-search-{for,back}ward
	history-beginning-search-{for,back}ward
	{vi-,}{up,down}-line-or-history
	vi-{rev-,}repeat-search

There's also one unrelated bug-fix in this patch in the function
historybeginningsearchforward():  the command would not go forward
to the last line in the history because of the "if" comparing
"histline" instead of the current value of "hl".  My previous
changes to history-search-forward had fixed this there (though I
don't think I mentioned it), but I hadn't noticed that it was also
broken elsewhere.

This patch assumes you've applied my prior changes for history-
search-{for,back}ward.

..wayne..

---8<------8<------8<------8<---cut here--->8------>8------>8------>8---
--- old/zle_hist.c	Wed Jun 10 03:31:06 1998
+++ zle_hist.c	Fri Jun 12 10:06:27 1998
@@ -299,8 +299,15 @@
 historysearchbackward(void)
 {
     int hl = histline;
+    int n = zmult;
     char *s;
 
+    if (zmult < 0) {
+	zmult = -n;
+	historysearchforward();
+	zmult = n;
+	return;
+    }
     if (hl == curhist || hl != srch_hl || cs != srch_cs || mark != 0
      || memcmp(srch_str, line, histpos) != 0) {
 	zfree(srch_str, histpos);
@@ -317,8 +324,10 @@
 	    return;
 	}
 	if (metadiffer(s, srch_str, histpos) < 0 &&
-	    metadiffer(s, srch_str, ll))
-	    break;
+	    metadiffer(s, srch_str, ll)) {
+	    if (--n <= 0)
+		break;
+	}
     }
     zle_goto_hist(hl);
     srch_hl = hl;
@@ -330,8 +339,15 @@
 historysearchforward(void)
 {
     int hl = histline;
+    int n = zmult;
     char *s;
 
+    if (zmult < 0) {
+	zmult = -n;
+	historysearchbackward();
+	zmult = n;
+	return;
+    }
     if (hl == curhist || hl != srch_hl || cs != srch_cs || mark != 0
      || memcmp(srch_str, line, histpos) != 0) {
 	zfree(srch_str, histpos);
@@ -349,7 +365,8 @@
 	}
 	if (metadiffer(s, srch_str, histpos) < (hl == curhist) &&
 	    metadiffer(s, srch_str, ll))
-	    break;
+	    if (--n <= 0)
+		break;
     }
     zle_goto_hist(hl);
     srch_hl = hl;
@@ -1019,12 +1036,17 @@
 virepeatsearch(void)
 {
     int hl = histline, t0;
+    int n = zmult;
     char *s;
 
     if (!visrchstr) {
 	feep();
 	return;
     }
+    if (zmult < 0) {
+	n = -n;
+	visrchsense = -visrchsense;
+    }
     t0 = strlen(visrchstr);
     for (;;) {
 	hl += visrchsense;
@@ -1035,9 +1057,11 @@
 	if (!metadiffer(s, (char *) line, ll))
 	    continue;
 	if (*visrchstr == '^') {
-	    if (!strncmp(s, visrchstr + 1, t0 - 1))
-		break;
-	} else if (hstrnstr(s, 0, visrchstr, t0, 1, 1))
+	    if (strncmp(s, visrchstr + 1, t0 - 1) != 0)
+		continue;
+	} else if (!hstrnstr(s, 0, visrchstr, t0, 1, 1))
+	    continue;
+	if (--n <= 0)
 	    break;
     }
     zle_goto_hist(hl);
@@ -1062,8 +1086,15 @@
 {
     int cpos = cs;		/* save cursor position */
     int hl = histline;
+    int n = zmult;
     char *s;
 
+    if (zmult < 0) {
+	zmult = -n;
+	historybeginningsearchforward();
+	zmult = n;
+	return;
+    }
     for (;;) {
 	hl--;
 	if (!(s = zle_get_event(hl))) {
@@ -1072,7 +1103,8 @@
 	}
 	if (metadiffer(s, (char *)line, cs) < 0 &&
 	    metadiffer(s, (char *)line, ll))
-	    break;
+	    if (--n <= 0)
+		break;
     }
 
     zle_goto_hist(hl);
@@ -1088,17 +1120,25 @@
 {
     int cpos = cs;		/* save cursor position */
     int hl = histline;
+    int n = zmult;
     char *s;
 
+    if (zmult < 0) {
+	zmult = -n;
+	historybeginningsearchbackward();
+	zmult = n;
+	return;
+    }
     for (;;) {
 	hl++;
 	if (!(s = zle_get_event(hl))) {
 	    feep();
 	    return;
 	}
-	if (metadiffer(s, (char *)line, cs) < (histline == curhist) &&
+	if (metadiffer(s, (char *)line, cs) < (hl == curhist) &&
 	    metadiffer(s, (char *)line, ll))
-	    break;
+	    if (--n <= 0)
+		break;
     }
 
     zle_goto_hist(hl);
---8<------8<------8<------8<---cut here--->8------>8------>8------>8---