Re: Bug#749969: history no longer syncs immediately, INC_APPEND_HISTORY broken

[Han Pingtian <hanpt@xxxxxxxxxxxxxxxxxx>]

I have just a quick idea for it :)

How about in this case, write the command into history first in the
hend(), then rewrite the history in hbegin() after the commnad finished?

Thanks.

On Tue, Jun 03, 2014 at 08:56:07PM +0100, Peter Stephenson wrote:
> On Sat, 31 May 2014 20:22:11 +0100
> Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx> wrote:
> > On Sat, 31 May 2014 11:21:59 +0200
> > Frank Terbeck <ft@xxxxxxxxxxxxxxxxxxx> wrote:
> > > I think to enable both ways to be possible, there should be two options:
> > > INC_APPEND_HISTORY that enables the original behaviour and (if the name
> > > is agreeable) DELAY_INC_APPEND_HISTORY, that amends the behaviour of the
> > > original option in the way that is reflected by the current behaviour.
> > 
> > The issue is that, without rewriting the history, either the time taken
> > for the command is wrong (that was the old case) or the command isn't
> > written out to the history until it's finished (that's the current
> > position).  Writing the history to get the best of both worlds probably
> > isn't beyond the bounds of 21st century science, but it's beyond my
> > understanding of the history code at the moment.  So at the moment
> > having two options looks like the only way to allow you to achieve any
> > one of the effects.  I think the new behaviour could be
> > INC_APPEND_HISTORY_TIME which puts it next to the original one and
> > indicates why it exists.
> > 
> > I haven't looked, but I don't think this ought to be too difficult ---
> > there are still some reasons why you might have to deal with history at
> > the original place anyway, so it should be possible to add an option
> > test at that point as well as one at the later point.
> 
> I think this works, but it's getting pretty silly.
> 
> diff --git a/Doc/Zsh/options.yo b/Doc/Zsh/options.yo
> index 349946d..7e46048 100644
> --- a/Doc/Zsh/options.yo
> +++ b/Doc/Zsh/options.yo
> @@ -971,6 +971,19 @@ The file will still be periodically re-written to trim it when the
>  number of lines grows 20% beyond the value specified by
>  tt($SAVEHIST) (see also the HIST_SAVE_BY_COPY option).
>  )
> +pindex(INC_APPEND_HISTORY_TIME)
> +pindex(NO_INC_APPEND_HISTORY_TIME)
> +pindex(INCAPPENDHISTORYTIME)
> +pindex(NOINCAPPENDHISTORYTIME)
> +cindex(history, incremental appending to a file with time)
> +item(tt(INC_APPEND_HISTORY_TIME))(
> +This option is a variant of tt(INC_APPEND_HISTORY) in which, where
> +possible, the history entry is written out to the file after the
> +command is finished, so that the time taken by the command is recorded
> +correctly in the history file in tt(EXTENDED_HISTORY) format.  This
> +means that the history entry will not be available immediately from
> +other instances of the shell that are using the same history file.
> +)
>  pindex(SHARE_HISTORY)
>  pindex(NO_SHARE_HISTORY)
>  pindex(SHAREHISTORY)
> diff --git a/Src/hist.c b/Src/hist.c
> index 1182994..64f88f5 100644
> --- a/Src/hist.c
> +++ b/Src/hist.c
> @@ -935,9 +935,11 @@ hbegin(int dohist)
> 
>      hf = getsparam("HISTFILE");
>      /*
> -     * For INCAPPENDHISTORY, when interactive, save the history here
> +     * For INCAPPENDHISTORYTIME, when interactive, save the history here
>       * as it gives a better estimate of the times of commands.
>       *
> +     * If INCAPPENDHISTORY is also set we've already done it.
> +     *
>       * If SHAREHISTORY is also set continue to do so in the
>       * standard place, because that's safer about reading and
>       * rewriting history atomically.
> @@ -950,7 +952,8 @@ hbegin(int dohist)
>       * so that (correctly) nothing happens here.  But it shows
>       * I thought about it.
>       */
> -    if (isset(INCAPPENDHISTORY) && !isset(SHAREHISTORY) &&
> +    if (isset(INCAPPENDHISTORYTIME) && !isset(SHAREHISTORY) &&
> +	!isset(INCAPPENDHISTORY) &&
>  	!(histactive & HA_NOINC) && !strin && histsave_stack_pos == 0)
>  	savehistfile(hf, 0, HFILE_USE_OPTIONS | HFILE_FAST);
>  }
> @@ -1378,7 +1381,8 @@ hend(Eprog prog)
>       * For normal INCAPPENDHISTORY case and reasoning, see hbegin().
>       */
>      if (isset(SHAREHISTORY) ? histfileIsLocked() :
> -	(isset(INCAPPENDHISTORY) && histsave_stack_pos != 0))
> +	(isset(INCAPPENDHISTORY) || (isset(INCAPPENDHISTORYTIME) &&
> +				     histsave_stack_pos != 0)))
>  	savehistfile(hf, 0, HFILE_USE_OPTIONS | HFILE_FAST);
>      unlockhistfile(hf); /* It's OK to call this even if we aren't locked */
>      /*
> @@ -2542,7 +2546,7 @@ savehistfile(char *fn, int err, int writeflags)
>      }
>      if (writeflags & HFILE_USE_OPTIONS) {
>  	if (isset(APPENDHISTORY) || isset(INCAPPENDHISTORY)
> -	 || isset(SHAREHISTORY))
> +	 || isset(INCAPPENDHISTORYTIME) || isset(SHAREHISTORY))
>  	    writeflags |= HFILE_APPEND | HFILE_SKIPOLD;
>  	else
>  	    histfile_linect = 0;
> @@ -2578,7 +2582,7 @@ savehistfile(char *fn, int err, int writeflags)
>  		tmpfile = NULL;
>  		if (err) {
>  		    if (isset(APPENDHISTORY) || isset(INCAPPENDHISTORY)
> -		     || isset(SHAREHISTORY))
> +		     || isset(INCAPPENDHISTORYTIME) || isset(SHAREHISTORY))
>  			zerr("rewriting %s would change its ownership -- skipped", fn);
>  		    else
>  			zerr("rewriting %s would change its ownership -- history not saved", fn);
> diff --git a/Src/options.c b/Src/options.c
> index e83dc58..2163bff 100644
> --- a/Src/options.c
> +++ b/Src/options.c
> @@ -165,6 +165,7 @@ static struct optname optns[] = {
>  {{NULL, "ignoreclosebraces",  OPT_EMULATE},		 IGNORECLOSEBRACES},
>  {{NULL, "ignoreeof",	      0},			 IGNOREEOF},
>  {{NULL, "incappendhistory",   0},			 INCAPPENDHISTORY},
> +{{NULL, "incappendhistorytime",   0},			 INCAPPENDHISTORYTIME},
>  {{NULL, "interactive",	      OPT_SPECIAL},		 INTERACTIVE},
>  {{NULL, "interactivecomments",OPT_BOURNE},		 INTERACTIVECOMMENTS},
>  {{NULL, "ksharrays",	      OPT_EMULATE|OPT_BOURNE},	 KSHARRAYS},
> diff --git a/Src/zsh.h b/Src/zsh.h
> index 620883b..05d582c 100644
> --- a/Src/zsh.h
> +++ b/Src/zsh.h
> @@ -2115,6 +2115,7 @@ enum {
>      IGNORECLOSEBRACES,
>      IGNOREEOF,
>      INCAPPENDHISTORY,
> +    INCAPPENDHISTORYTIME,
>      INTERACTIVE,
>      INTERACTIVECOMMENTS,
>      KSHARRAYS,
> 
> -- 
> Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>
> Web page now at http://homepage.ntlworld.com/p.w.stephenson/