Re: [PATCH] Prune duplicate command history entries

[Patrick Palka <patrick@parcs.ath.cx>]

On Wed, Jun 3, 2015 at 4:20 AM, Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
> * Patrick Palka <patrick@parcs.ath.cx> [2015-06-02 23:22:46 -0400]:
>
>> This patch implements pruning of duplicate command-history entries using
>> a modest amount of lookbehind.  The motivation for this patch is to
>> reduce the prevalence of basic commands such as "up" and "down" in the
>> history file.
>
> Dropping commands such as up / down could be pretty annoying if you
> wanted to figure out where you were in the past. Dropping things like
> bt from the history would be less annoying.
>
> I wonder if we should classify commands into navigation or
> state-changing commands and diagnostic commands.
> I'd be happier see repeated diagnostic commands disappear, and less so
> for commands that change inferior state, or navigate me around the
> stack.

Hmm, but doesn't the empty-command shorthand already make it mostly
impossible to figure out where you were in the past?  If you run "up"
twice by typing "up\n\n" then only one history entry for "up" gets
added to the history file (with or without the patch).  So you won't
really be sure, by looking at your history, whether that
"up"/"down"/"continue"/"step" entry was invoked once or 5 times in a
row. It seems that this existing behavior throws any semblance of
state recovery out the window (unless you carefully avoid using the
shorthand :)).

>
>> The maximum lookbehind is fixed to 50 (an arbitrary number) so that the
>> operation will be guaranteed to not take too long.
>
> I think at the very least you should make this threshold
> configurable.  I'd then argue for off by default due to the loss of
> state changing commands being too annoying (for me).
>
> You should probably have some tests too, we already test C-p in
> readline.exp, so it should be possible to test that this feature
> works.

I did not think to make the threshold configurable (as opposed to
making some boolean variable configurable).  That makes sense.

>
> Thanks,
> Andrew