From: Stefan Sperling <stsp@stsp.name>
Subject: Re: tog: trim log_scroll_*, trigger_log_thread functions
To: Christian Weisgerber <naddy@mips.inka.de>
Cc: gameoftrees@openbsd.org
Date: Tue, 1 Dec 2020 22:18:27 +0100

On Tue, Dec 01, 2020 at 05:39:47PM +0100, Christian Weisgerber wrote:
> trim redundant parameters from log_scroll_* and trigger_log_thread functions
>  
> Also rename scroll_{up,down} to log_scroll_{up,down}; requested by stsp.
> 
> This is a single diff because log_scroll_down() and trigger_log_thread()
> are entangled.
> 
> ok?

A clear improvement. OK, with one small remark below.
 
> diff refs/heads/main refs/heads/wip
> blob - 3d070a1101fd42f89e3b94632d56d43ee705e66b
> blob + 02ae1bde0056ed2df220f62f486d778115cd673f
> --- tog/tog.c
> +++ tog/tog.c
> @@ -1687,42 +1687,40 @@ done:
>  }
>  
>  static void
> -scroll_up(struct tog_view *view,
> -    struct commit_queue_entry **first_displayed_entry, int maxscroll,
> -    struct commit_queue *commits)
> +log_scroll_up(struct tog_view *view, int maxscroll)

Any reason you are not passing a struct tog_log_view_state parameter here
instead of struct tog_view? Is it for consistency with log_scroll_down()?
If so, I don't think there's a problem with these function taking different
types for their first parameters. log_scroll_down() can be special.
But this is not an objection. I'm fine with this change either way.

>  {
> +	struct tog_log_view_state *s = &view->state.log;
>  	struct commit_queue_entry *entry;
>  	int nscrolled = 0;