Re: [RP] [PATCH] Fix: call using_history after initializing history entries

[Jeremie Courreges-Anglas]

On Wed, Aug 02 2017, Antoine Busque <address@hidden> wrote:
> In its current state, history expansion requiring any sort of string
> lookup is not working, always returing "event not found". This is due
> to `history_offset` being set improperly by an early call to
> `using_history`.
>
> A call to `using_history` needs to follow, not precede, the loop
> performing initialization of history entries found in
> `history_expand_line`. This allows for `history_offset` to reflect the
> now non-zero length of the history, restoring the expected behaviour
> of history expansion.

Thanks for figuring this out, Antoine.

I don't think I've ever used (or tried to use) history expansion
support in ratpoison.  I think there are two possible ways to move
forward:
- history expansion has been broken for so long that this code should
  just be removed.
- only recent versions of libhistory break with the current ratpoison
  code.  So people that are used to this history expansion support might
  like your fix.

I have recently disabled libhistory support by default, and asked
whether people would miss it if it was removed for good from ratpoison.
Only two persons answered positively.  Also, looking at the code in your
diff below, history expansion will only be enabled for lines that
contain '!', which is a subset of what libhistory supports.

I am going to merge your fix, so that people can play with history
expansion, and hopefully improve it.  I won't test it myself, as my
day-to-day system doesn't provide libhistory.  Input welcome.

> Tested-by: Martin Hertz <address@hidden>
> Signed-off-by: Antoine Busque <address@hidden>
> ---
>  src/history.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/history.c b/src/history.c
> index d1e9938..1466e89 100644
> --- a/src/history.c
> +++ b/src/history.c
> @@ -314,10 +314,10 @@ history_expand_line (int history_id UNUSED, char 
> *string, char **output)
>  
>    if (strchr (string, '!')) {
>      clear_history ();
> -    using_history ();
>      list_for_each_entry(item, &histories[history_id].head, node) {
>        add_history (item->line);
>      }
> +    using_history ();
>      return history_expand (string, output);
>    }
>  #endif

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

signature.asc
Description: PGP signature