From: Stefan Sperling <stsp@stsp.name>
Subject: Re: tog: clear search highlighting when reloading view
To: Christian Weisgerber <naddy@mips.inka.de>
Cc: gameoftrees@openbsd.org
Date: Mon, 20 Dec 2021 22:26:01 +0100

On Mon, Dec 20, 2021 at 10:10:14PM +0100, Christian Weisgerber wrote:
> In tog, highlighting from a prior search persists when you switch
> the content of a diff view ('>', '<') or a blame view ('b', 'p', 'B').
> 
> To reproduce:
> * Start "tog log".
> * Hit enter for a diff view.
> * Search for something that appears in the first few lines.
>   -> The match is highlighted.
> * Move to previous or next commit with '<' or '>'.
>   -> The text position of the match from the previous step is still
>      highlighted.
> 
> The same happens in a blame view when you reload it with a selected/
> parent/previous commit with 'b', 'p', 'B'.
> 
> Those actions should reset the matched_line variable in the view
> state.  For the diff view, input_diff_view() looks like the right
> place.  I'm less certain about the blame view, but run_blame() works
> for me.

run_blame() looks like the correct place to me.

> OK?

Not yet, see below:

> diff 46ea77db89a0945a013f032c6c6a94779126412f /home/naddy/got
> blob - 689aa4b67e0bcae980ac8b0f007f11c145528be1
> file + tog/tog.c
> --- tog/tog.c
> +++ tog/tog.c
> @@ -3793,6 +3793,7 @@ input_diff_view(struct tog_view **new_view, struct tog
>  
>  		s->first_displayed_line = 1;
>  		s->last_displayed_line = view->nlines;
> +		s->matched_line = 0;
>  
>  		diff_view_indicate_progress(view);
>  		err = create_diff(s);
> @@ -3817,6 +3818,7 @@ input_diff_view(struct tog_view **new_view, struct tog
>  
>  		s->first_displayed_line = 1;
>  		s->last_displayed_line = view->nlines;
> +		s->matched_line = 0;

There is a case which you missed for the diff view: Changing the amount
of diff context with '[' or ']' has the same problem.

Instead of having at least three places which reset those 3 variables,
maybe create a function that resets diff view state and call it from
at least those three places?

>  		diff_view_indicate_progress(view);
>  		err = create_diff(s);
> @@ -4369,6 +4371,7 @@ run_blame(struct tog_view *view)
>  		s->last_displayed_line = view->nlines;
>  		s->selected_line = 1;
>  	}
> +	s->matched_line = 0;
>  
>  done:
>  	if (blob)
> -- 
> Christian "naddy" Weisgerber                          naddy@mips.inka.de
> 
>