Re: fc -l doesn't match %h number?

[Peter Stephenson <pws@xxxxxxx>]

On Wed, 1 Oct 2008 11:54:45 -0400
"Rocky Bernstein" <rocky.bernstein@xxxxxxxxx> wrote:
> It looks to me like "fc -l" is not showing a command in the history
> that %h is counting when one enters a new subshell.

I had a moment to look at this when it seemed futile to try to start
something new...

I don't think the problem is the subshell, I think it's a question of
consistently looking for the latest line that's been added.  (As previously
mentioned, there is an inevitable issue with losing stuff on leaving
subshells which we're not worrying about, that's just how they work.)

If you do "fc -l" or "history" at the command line it deliberately
suppresses that command itself from the listing, even though it has by
then been added to the history list as that's done when the line has been
read and before execution (which, I think, makes perfect sense).  The same
suppression is happening here, but the line in question is the one that's
been added by "print -s", so you would normally expect to see it.  So it's
not that the line number to list isn't being incremented, it's just that
it's suppressing more than you expect.

Other glitches with the current way of doing things appeared when I added a
second "print -s", causing the last edited line to go further out of step
with the last entry in the history, and more entries to be missed from the
listing.

I think this patch has a chance of fixing it without doing anything
horrible --- it shouldn't change the usual interactive behaviour, when
there's no manipulation of the history behind the user's back.  There are
basically two points:

(i) When listing, we want to base the list on the last line added, not the
last line edited by the user, since these drift away from one another when
you add lines using "print -s" (that's my second discovery that's not
displayed by Rocky's original test).  We do not want to do this when
reexecuting instead of listing, since we could end up reexecuting a line
added with "print -s" that the user hasn't seen, so I've left that the way
it is.  (The presence of "print -s" stuff catches up with you eventually if
you do !-2 from the next command line, or whatever, but there's no escaping
that without a completely separate numbering scheme.)

(ii) If we find that that the last line added is not the same as the last
edited line, we want to show all lines added, since it means the user has
added a line non-interactively since the line edited and wants to see it.
Slight complication: although the last edited line was therefore not a line
directly asking for history to be listed, it might have been a line that
triggered a "print -s", then asked for history to be listed.  However, I
think that's OK: the fact there's that extra entry in between is enough to
show the user something's happened to warrant the extra history showing up.

It's vaguely possible there's some case I've missed that means that the
last edited line drifts away from the last added line in some other way that
might cause the last entry not to be the correct one.  Luckily, the
temporary history mechanism (the last line is always there for immediate
recall even if it won't be saved long term) seems to fix this up in the
cases I've tried.

The explanation and comments are considerably longer than the three
lines of code that have changed.

Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.210
diff -u -r1.210 builtin.c
--- Src/builtin.c	29 Sep 2008 08:46:31 -0000	1.210
+++ Src/builtin.c	6 Oct 2008 17:43:16 -0000
@@ -1416,7 +1416,19 @@
     /* default values of first and last, and range checking */
     if (last == -1) {
 	if (OPT_ISSET(ops,'l') && first < curhist) {
-	    last = addhistnum(curline.histnum,-1,0);
+	    /*
+	     * When listing base our calculations on curhist,
+	     * to show anything added since the edited history line.
+	     * Also, in that case curhist will have been modified
+	     * past the current history line; then we want to
+	     * show everything, because the user expects to
+	     * see the result of "print -s".  Otherwise, we subtract
+	     * -1 from the line, because the user doesn't usually expect
+	     * to see the command line that caused history to be
+	     * listed.
+	     */
+	    last = (curline.histnum == curhist) ? addhistnum(curhist,-1,0)
+		: curhist;
 	    if (last < firsthist())
 		last = firsthist();
 	}
@@ -1424,7 +1436,15 @@
 	    last = first;
     }
     if (first == -1) {
-	first = OPT_ISSET(ops,'l')? addhistnum(curline.histnum,-16,0)
+	/*
+	 * When listing, we want to see everything that's been
+	 * added to the history, including by print -s, so use
+	 * curhist.
+	 * When reexecuting, we want to restrict to the last edited
+	 * command line to avoid giving the user a nasty turn
+	 * if some helpful soul ran "print -s 'rm -rf /'".
+	 */
+	first = OPT_ISSET(ops,'l')? addhistnum(curhist,-16,0)
 			: addhistnum(curline.histnum,-1,0);
 	if (first < 1)
 	    first = 1;


-- 
Peter Stephenson <pws@xxxxxxx>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070