On Fri, Jul 22, 2022 at 10:30:15PM +1000, Mark Jamsek wrote:
> This refactors new view requests to dedup some code.  I noticed the
> other day when adding the blame -> log view TODO item that we duplicate
> the view initialisation code for each new request. Simplify this
> boilerplate with a couple new routines, which will make future additions
> nicer too.

Fine with me. This patch reduces quite some copy-paste mess which has
accumulated over time and makes it easier for mistakes to creep in,
similar to the forgotten view->offset = 0 you just fixed in your other
patch.

> +static const struct got_error *
> +view_dispatch_request(struct tog_view **new_view, struct tog_view *view,
> +    enum tog_view_type request, int y, int x)
> +{
> +	const struct got_error *err = NULL;
> +
> +	switch (request) {
> +	case TOG_VIEW_DIFF: {
> +		struct tog_log_view_state *s = &view->state.log;

Using a log view state here implies that view->type == TOG_VIEW_LOG.
Could we check for this explicitly, even if it is the only case right now?

Please consider raising errors in any unhandled cases, to make sure this
part of the code gets some attention when a new type of view is added.

> +
> +		err = open_diff_view_for_commit(new_view, y, x,
> +		    s->selected_entry->commit, s->selected_entry->id,
> +		    view, s->repo);
> +		break;
> +	}
> +	case TOG_VIEW_BLAME: {
> +		struct tog_tree_view_state *s = &view->state.tree;
> +
> +		err = blame_tree_entry(new_view, y, x, s->selected_entry,
> +		    &s->parents, s->commit_id, s->repo);
> +		break;
> +	}
> +	case TOG_VIEW_LOG:
> +		if (view->type == TOG_VIEW_BLAME)
> +			err = log_annotated_line(new_view, y, x,
> +			    view->state.blame.repo, view->state.blame.id_to_log);
> +		else if (view->type == TOG_VIEW_TREE)
> +			err = log_selected_tree_entry(new_view, y, x,
> +			    &view->state.tree);
> +		else if (view->type == TOG_VIEW_REF)
> +			err = log_ref_entry(new_view, y, x,
> +			    view->state.ref.selected_entry,
> +			    view->state.ref.repo);
> +		break;
> +	case TOG_VIEW_TREE: {
> +		if (view->type == TOG_VIEW_LOG)
> +			err = browse_commit_tree(new_view, y, x,
> +			    view->state.log.selected_entry,
> +			    view->state.log.in_repo_path,
> +			    view->state.log.head_ref_name,
> +			    view->state.log.repo);
> +		else if (view->type == TOG_VIEW_REF)
> +			err = browse_ref_tree(new_view, y, x,
> +			    view->state.ref.selected_entry,
> +			    view->state.ref.repo);
> +		break;
> +	}
> +	case TOG_VIEW_REF:
> +		*new_view = view_open(0, 0, y, x, TOG_VIEW_REF);
> +		if (*new_view == NULL)
> +			return got_error_from_errno("view_open");
> +		err = open_ref_view(*new_view, view->type == TOG_VIEW_LOG ?
> +		    view->state.log.repo : view->state.tree.repo);
> +		if (err)
> +			view_close(*new_view);
> +		/* FALL THROUGH */
> +	default:
> +		break;
> +	}
> +
> +	return err;
> +}
> +
>  /*
>   * If view was scrolled down to move the selected line into view when opening a
>   * horizontal split, scroll back up when closing the split/toggling fullscreen.
> 
> -- 
> Mark Jamsek <fnc.bsdbox.org>
> GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68