Finer control over what gets written to the history file

Finer control over what gets written to the history file

[Bart Schaefer]

I often find myself running a lot of similar but not quite identical
commands to test some obscure bug or otherwise experiment with a shell
construct.  This stuff tends to push useful history out of the history
file when I exit from the shell, if I don't remember to use my "die"
alias that disables history and other exit-time operations.

It'd be nice to be able to selectively exclude those from history after
the fact, also without having to remember to type a leading space on
every such line.

[[ ASIDE:  I'd always assumed that those useful history entries would be
salvaged by exiting from some other shell window later, but it turns out
that for some time now (I've lost track of exactly when) history saving
is optimized to only write the stuff that's new since the current shell
was started.  This means all the old stuff that I assumed was going to
be merged back into the history file is actually discarded instead, and
it means that if you change HISTFILE interactively you're likely to lose
a lot of your old history as well, unless you explicitly "fc -W". ]]

The following patch adds a HISTORY_IGNORE parameter, like CORRECT_IGNORE
except that it applies to writing lines into the history file.  I have
not written doc yet because I wanted to get this reviewed, particularly
for memory management -- I'm not sure whether using META_HEAPDUP here
means that a copy of the history is sometimes going to end up on the
top-level (and hence effectively "permanent") heap and thus be "leaked"
for practical purposes.  Maybe heap push/pop is needed?

Does anyone else think this is useful?  I considered instead applying it
at the point where HIST_IGNORE_DUPS is calculated, but that removes the
command from the runtime history which I usually don't want.

diff --git a/Src/hist.c b/Src/hist.c
index ed95609..46b488f 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -2583,12 +2583,25 @@ savehistfile(char *fn, int err, int writeflags)
 	}
     }
     if (out) {
+	char *history_ignore;
+	Patprog histpat = NULL;
+
+	if ((history_ignore = getsparam("HISTORY_IGNORE")) != NULL) {
+	    tokenize(history_ignore = dupstring(history_ignore));
+	    remnulargs(history_ignore);
+	    histpat = patcompile(history_ignore, 0, NULL);
+	}
+
 	ret = 0;
 	for (; he && he->histnum <= xcurhist; he = down_histent(he)) {
 	    if ((writeflags & HFILE_SKIPDUPS && he->node.flags & HIST_DUP)
 	     || (writeflags & HFILE_SKIPFOREIGN && he->node.flags & HIST_FOREIGN)
 	     || he->node.flags & HIST_TMPSTORE)
 		continue;
+	    if (histpat &&
+		pattry(histpat, metafy(he->node.nam, -1, META_HEAPDUP))) {
+		continue;
+	    }
 	    if (writeflags & HFILE_SKIPOLD) {
 		if (he->node.flags & HIST_OLD)
 		    continue;

Re: Finer control over what gets written to the history file

[Peter Stephenson]

On Tue, 15 Oct 2013 09:41:00 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> I often find myself running a lot of similar but not quite identical
> commands to test some obscure bug or otherwise experiment with a shell
> construct.  This stuff tends to push useful history out of the history
> file when I exit from the shell, if I don't remember to use my "die"
> alias that disables history and other exit-time operations.
> 
> It'd be nice to be able to selectively exclude those from history after
> the fact, also without having to remember to type a leading space on
> every such line.

You know about the zshaddhistory hook?  Excluding by pattern is a
fairly easy instance of this, and it should be readily extensible for
various different ways of checking.  You could make it configurable by
styles.

  zshistory_veto () {
    local -a line
    line=(${(Q)${(z)1}})
    # too lazy to do the array stuff...
    if [[ $line[1] = ${~HISTIGNORE} ]]
    then
    	return 1
    fi
  }
  autoload -Uz add-zsh-hook
  add-zsh-hook zshaddhistory zshaddhistory_veto

Just seen a typo.


zshaddhistory
       Executed  when  a  history line has been read interactively, but
       before it is executed.  The sole argument is the  complete  his‐
       tory  line  (so  that  any  terminating  newline  will  still be
       present).

       If any of the hook functions return a non-zero value the history
       line will not be saved, although it lingers in the history until
       the next line is executed allow you to reuse or edit it  immedi‐
       ately.

       A  hook function may call `fc -p ...' to switch the history con‐
       text so that the history is saved in a different file  from  the
       that  in  the  global  HISTFILE parameter.  This is handled spe‐
       cially: the history context is automatically restored after  the
       processing of the history line is finished.

       The  following  example  function first adds the history line to
       the normal history with the newline stripped,  which is  usually
       the  correct behaviour.  Then it switches the history context so
       that the line will be written to a history file in  the  current
       directory.

              zshaddhistory() {
                print -sr -- ${1%%$'\n'}
                fc -p .zsh_local_history
              }

pws

Re: Finer control over what gets written to the history file

[Bart Schaefer]

On Oct 15,  5:58pm, Peter Stephenson wrote:
} Subject: Re: Finer control over what gets written to the history file
}
} On Tue, 15 Oct 2013 09:41:00 -0700
} Bart Schaefer <schaefer@brasslantern.com> wrote:
} > It'd be nice to be able to selectively exclude those from history after
} > the fact, also without having to remember to type a leading space on
} > every such line.
} 
} You know about the zshaddhistory hook?  Excluding by pattern is a
} fairly easy instance of this, and it should be readily extensible for
} various different ways of checking.

Yes, but it happens at the wrong time.  I don't want to prevent stuff
from going into the history as I type it, I want to filter it out of
the runtime history just before saving it.

I.e., I want to realize 30 commands later that I really don't need the
last 30 iterations of "Src/zsh -ec '{ /bin/cp } 2>>(sleep 1; cat -n)'"
to be written to the history file.  By that time zshaddhistory is long
out of the picture.

I suppose I could have gone with a zshsavehistory hook instead, but it
seemed a lot more straightforward to copy the CORRECT_IGNORE code.  :-)

Re: Finer control over what gets written to the history file

[Bart Schaefer]

Incidentally ...

On Oct 15,  5:58pm, Peter Stephenson quoted the docs:
>
> zshaddhistory
>        The  following  example  function first adds the history line to
>        the normal history with the newline stripped,  which is  usually
>        the  correct behaviour.  Then it switches the history context so
>        that the line will be written to a history file in  the  current
>        directory.
> 
>               zshaddhistory() {
>                 print -sr -- ${1%%$'\n'}
>                 fc -p .zsh_local_history
>               }

This doesn't quite work, does it?  The missing bit being that you also
must have INC_APPEND_HISTORY set, otherwise the stack is popped before
anything is written.

Re: Finer control over what gets written to the history file

[Peter Stephenson]

On Tue, 15 Oct 2013 22:25:04 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:

> Incidentally ...
> 
> On Oct 15,  5:58pm, Peter Stephenson quoted the docs:
> >
> > zshaddhistory
> >        The  following  example  function first adds the history line to
> >        the normal history with the newline stripped,  which is  usually
> >        the  correct behaviour.  Then it switches the history context so
> >        that the line will be written to a history file in  the  current
> >        directory.
> > 
> >               zshaddhistory() {
> >                 print -sr -- ${1%%$'\n'}
> >                 fc -p .zsh_local_history
> >               }
> 
> This doesn't quite work, does it?  The missing bit being that you also
> must have INC_APPEND_HISTORY set, otherwise the stack is popped before
> anything is written.

Yes, that needs adding.

diff --git a/Doc/Zsh/func.yo b/Doc/Zsh/func.yo
index 78bdfc0..1f58df8 100644
--- a/Doc/Zsh/func.yo
+++ b/Doc/Zsh/func.yo
@@ -274,10 +274,13 @@ that in the global tt(HISTFILE) parameter.  This is handled specially:
 the history context is automatically restored after the processing
 of the history line is finished.
 
-The following example function first adds the history line to the normal
-history with the newline stripped,  which is usually the correct behaviour.
-Then it switches the history context so that the line will
-be written to a history file in the current directory.
+The following example function works with one of the options
+tt(INC_APPEND_HISTORY) or tt(SHARE_HISTORY) set, in order that the line
+is written out immediately after the history entry is added.  It first
+adds the history line to the normal history with the newline stripped,
+which is usually the correct behaviour.  Then it switches the history
+context so that the line will be written to a history file in the
+current directory.
 
 example(zshaddhistory+LPAR()RPAR() {
   print -sr -- ${1%%$'\n'}

pws

Re: Finer control over what gets written to the history file

[Peter Stephenson]

On Tue, 15 Oct 2013 17:34:53 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Oct 15,  5:58pm, Peter Stephenson wrote:
> } Subject: Re: Finer control over what gets written to the history file
>
> } You know about the zshaddhistory hook?  Excluding by pattern is a
> } fairly easy instance of this, and it should be readily extensible for
> } various different ways of checking.
> 
> Yes, but it happens at the wrong time.  I don't want to prevent stuff
> from going into the history as I type it, I want to filter it out of
> the runtime history just before saving it.
> 
> I.e., I want to realize 30 commands later that I really don't need the
> last 30 iterations of "Src/zsh -ec '{ /bin/cp } 2>>(sleep 1; cat -n)'"
> to be written to the history file.  By that time zshaddhistory is long
> out of the picture.

We could unify the approaches and add a flag for this to histent (we've
got plenty of spare bits at the moment).  So e.g. zshaddhistory could
return 2 to say "keep this internally but don't save it".  I haven't
looked, but it shouldn't be too hard to combine with the HIST_FOREIGN
and HIST_TMPSTORE logic.

pws

Re: Finer control over what gets written to the history file

[Peter Stephenson]

On Wed, 16 Oct 2013 09:55:24 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> On Tue, 15 Oct 2013 17:34:53 -0700
> Bart Schaefer <schaefer@brasslantern.com> wrote:
> > On Oct 15,  5:58pm, Peter Stephenson wrote:
> > } Subject: Re: Finer control over what gets written to the history file
> >
> > } You know about the zshaddhistory hook?  Excluding by pattern is a
> > } fairly easy instance of this, and it should be readily extensible for
> > } various different ways of checking.
> > 
> > Yes, but it happens at the wrong time.  I don't want to prevent stuff
> > from going into the history as I type it, I want to filter it out of
> > the runtime history just before saving it.
> > 
> > I.e., I want to realize 30 commands later that I really don't need the
> > last 30 iterations of "Src/zsh -ec '{ /bin/cp } 2>>(sleep 1; cat -n)'"
> > to be written to the history file.  By that time zshaddhistory is long
> > out of the picture.
> 
> We could unify the approaches and add a flag for this to histent (we've
> got plenty of spare bits at the moment).  So e.g. zshaddhistory could
> return 2 to say "keep this internally but don't save it".  I haven't
> looked, but it shouldn't be too hard to combine with the HIST_FOREIGN
> and HIST_TMPSTORE logic.

Oh, wait, you're saying you didn't even realise at the time you wanted
to omit the lines.  You want something you can activate at the point
you're about to run fc -W or exit the shell.   So that would have to be
a different hook, which isn't really appropriate for running on every
line of a potentially huge file.  So probably your way is as good as it
gets.

pws

Re: Finer control over what gets written to the history file

[Peter Stephenson]

On Wed, 16 Oct 2013 09:55:24 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> We could unify the approaches and add a flag for this to histent (we've
> got plenty of spare bits at the moment).  So e.g. zshaddhistory could
> return 2 to say "keep this internally but don't save it".  I haven't
> looked, but it shouldn't be too hard to combine with the HIST_FOREIGN
> and HIST_TMPSTORE logic.

It turns out this is indeed extremely easy, even if it doesn't solve
Bart's problem without an extra measure of clairvoyance.  So there might
be some merit in having it available.

I'm guessing that anyone already using this mechanism would naturally
return status 1, so making 2 special isn't a big incompatibility, but
I've noted it anyway.

diff --git a/Doc/Zsh/func.yo b/Doc/Zsh/func.yo
index 1f58df8..6e9cfee 100644
--- a/Doc/Zsh/func.yo
+++ b/Doc/Zsh/func.yo
@@ -264,9 +264,16 @@ Executed when a history line has been read interactively, but
 before it is executed.  The sole argument is the complete history
 line (so that any terminating newline will still be present).
 
-If any of the hook functions return a non-zero value the history
-line will not be saved, although it lingers in the history until the
-next line is executed, allowing you to reuse or edit it immediately.
+If any of the hook functions returns status 1 (or any non-zero value
+other than 2, though this is not guaranteed for future versions of the
+shell) the history line will not be saved, although it lingers in the
+history until the next line is executed, allowing you to reuse or edit
+it immediately.
+
+If any of the hook functions returns status 2 the history line
+will be saved on the internal history list, but not written to
+the history file.  In case of a conflict, the first non-zero status
+value is taken.
 
 A hook function may call `tt(fc -p) var(...)' to switch the history
 context so that the history is saved in a different file from the
diff --git a/README b/README
index 2a22f42..8ddad6e 100644
--- a/README
+++ b/README
@@ -28,6 +28,17 @@ Zsh is a shell with lots of features.  For a list of some of these, see the
 file FEATURES, and for the latest changes see NEWS.  For more
 details, see the documentation.
 
+Incompatibilities between 5.0.2 and 5.0.3
+-----------------------------------------
+
+The "zshaddhistory" hook mechanism documented in the zshmisc manual page
+has been upgraded so that a hook returning status 2 causes a history
+line to be saved on the internal history list but not written to the
+history file.  Previously any non-zero status return would cause
+the line not to be saved on the history at all.  It is recommended
+to use status 1 for this (indeed most shell users would naturally do
+so).
+
 Incompatibilities between 5.0.0 and 5.0.2
 -----------------------------------------
 
diff --git a/Src/hist.c b/Src/hist.c
index ed95609..fa5bdbb 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -1226,7 +1226,15 @@ mod_export int
 hend(Eprog prog)
 {
     LinkList hookargs = newlinklist();
-    int flag, save = 1, hookret, stack_pos = histsave_stack_pos;
+    int flag, hookret, stack_pos = histsave_stack_pos;
+    /*
+     * save:
+     * 0: don't save
+     * 1: save normally
+     * -1: save temporarily, delete after next line
+     * -2: save internally but mark for not writing
+     */
+    int save = 1;
     char *hf;
 
     DPUTS(stophist != 2 && !(inbufflags & INP_ALIAS) && !chline,
@@ -1279,7 +1287,11 @@ hend(Eprog prog)
 	}
 	if (chwordpos <= 2)
 	    save = 0;
-	else if (hookret || should_ignore_line(prog))
+	else if (should_ignore_line(prog))
+	    save = -1;
+	else if (hookret == 2)
+	    save = -2;
+	else if (hookret)
 	    save = -1;
     }
     if (flag & (HISTFLAG_DONE | HISTFLAG_RECALL)) {
@@ -1325,7 +1337,12 @@ hend(Eprog prog)
 	    if (isset(HISTREDUCEBLANKS))
 		histreduceblanks();
 	}
-	newflags = save > 0? 0 : HIST_TMPSTORE;
+	if (save == -1)
+	    newflags = HIST_TMPSTORE;
+	else if (save == -2)
+	    newflags = HIST_NOWRITE;
+	else
+	    newflags = 0;
 	if ((isset(HISTIGNOREDUPS) || isset(HISTIGNOREALLDUPS)) && save > 0
 	 && hist_ring && histstrcmp(chline, hist_ring->node.nam) == 0) {
 	    /* This history entry compares the same as the previous.
@@ -2590,7 +2607,7 @@ savehistfile(char *fn, int err, int writeflags)
 	     || he->node.flags & HIST_TMPSTORE)
 		continue;
 	    if (writeflags & HFILE_SKIPOLD) {
-		if (he->node.flags & HIST_OLD)
+		if (he->node.flags & (HIST_OLD|HIST_NOWRITE))
 		    continue;
 		he->node.flags |= HIST_OLD;
 		if (writeflags & HFILE_USE_OPTIONS)
diff --git a/Src/zsh.h b/Src/zsh.h
index a46898d..a935d23 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -1894,6 +1894,7 @@ struct histent {
 #define HIST_DUP	0x00000008	/* Command duplicates a later line */
 #define HIST_FOREIGN	0x00000010	/* Command came from another shell */
 #define HIST_TMPSTORE	0x00000020	/* Kill when user enters another cmd */
+#define HIST_NOWRITE	0x00000040	/* Keep internally but don't write */
 
 #define GETHIST_UPWARD  (-1)
 #define GETHIST_DOWNWARD  1

pws

Re: Finer control over what gets written to the history file

[Bart Schaefer]

On Oct 16, 10:20am, Peter Stephenson wrote:
}
} Oh, wait, you're saying you didn't even realise at the time you wanted
} to omit the lines.  You want something you can activate at the point
} you're about to run fc -W or exit the shell.   So that would have to be
} a different hook, which isn't really appropriate for running on every
} line of a potentially huge file.  So probably your way is as good as it
} gets.

OK, here's a final patch including doc.  Nobody commented on the memory
management question and a push/pop heap is harmless if the heap is never
used, so ...

Hmm, this fails silently if the pattern can't be compiled.  I'll leave
any warnings to the next patch.


diff --git a/Doc/Zsh/params.yo b/Doc/Zsh/params.yo
index 8c37c2d..97087a1 100644
--- a/Doc/Zsh/params.yo
+++ b/Doc/Zsh/params.yo
@@ -967,6 +967,16 @@ item(tt(HISTFILE))(
 The file to save the history in when an interactive shell exits.
 If unset, the history is not saved.
 )
+vindex(HISTORY_IGNORE)
+item(tt(HISTORY_IGNORE))(
+If set, is treated as a pattern at the time history files are written.
+Any potential history entry that matches the pattern is skipped.  For
+example, if the value is `tt(fc *)' then commands that invoke the
+interactive history editor are never written to the history file (compare
+the tt(HIST_NO_STORE) option or the tt(zshaddhistory) hook, either of
+which would prevent such commands from being added to the interactive
+history at all).
+)
 vindex(HISTSIZE)
 item(tt(HISTSIZE) <S>)(
 The maximum number of events stored in the internal history list.
diff --git a/Src/hist.c b/Src/hist.c
index fa5bdbb..1845bd8 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -2600,12 +2600,27 @@ savehistfile(char *fn, int err, int writeflags)
 	}
     }
     if (out) {
+	char *history_ignore;
+	Patprog histpat = NULL;
+
+	pushheap();
+
+	if ((history_ignore = getsparam("HISTORY_IGNORE")) != NULL) {
+	    tokenize(history_ignore = dupstring(history_ignore));
+	    remnulargs(history_ignore);
+	    histpat = patcompile(history_ignore, 0, NULL);
+	}
+
 	ret = 0;
 	for (; he && he->histnum <= xcurhist; he = down_histent(he)) {
 	    if ((writeflags & HFILE_SKIPDUPS && he->node.flags & HIST_DUP)
 	     || (writeflags & HFILE_SKIPFOREIGN && he->node.flags & HIST_FOREIGN)
 	     || he->node.flags & HIST_TMPSTORE)
 		continue;
+	    if (histpat &&
+		pattry(histpat, metafy(he->node.nam, -1, META_HEAPDUP))) {
+		continue;
+	    }
 	    if (writeflags & HFILE_SKIPOLD) {
 		if (he->node.flags & (HIST_OLD|HIST_NOWRITE))
 		    continue;
@@ -2685,6 +2700,8 @@ savehistfile(char *fn, int err, int writeflags)
 		histactive = remember_histactive;
 	    }
 	}
+
+	popheap();
     } else
 	ret = -1;
 

Re: Finer control over what gets written to the history file

[Peter Stephenson]

On Thu, 17 Oct 2013 07:20:22 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> OK, here's a final patch including doc.  Nobody commented on the memory
> management question and a push/pop heap is harmless if the heap is never
> used, so ...

I didn't investigate, but putting it around a significant chunk of I/O
activity like this seems pretty uncontroversial.

pws