From: Stefan Sperling <stsp@stsp.name>
Subject: Re: tog diff view search
To: Tracey Emery <tracey@traceyemery.net>
Cc: gameoftrees@openbsd.org
Date: Sat, 1 Feb 2020 12:03:36 +0100

On Fri, Jan 31, 2020 at 01:20:09PM -0700, Tracey Emery wrote:
> Removed unneeded var, freed line_offsets, and fixed indent.
> 
> -- 
> 
> Tracey Emery
> 
> diff de6bdba4416e9e727ac8933082ccb83b56fbd3ab /home/basepr1me/Documents/got/got/got
> blob - 461975117c98f1bb74df15d9317223d1940e4c6d
> file + include/got_object.h
> --- include/got_object.h
> +++ include/got_object.h
> @@ -258,6 +258,15 @@ const struct got_error *got_object_blob_dump_to_file(s
>      off_t **, FILE *, struct got_blob_object *);
>  
>  /*
> + * Read the contents of a FILE stream and indicate the amount of bytes
> + * written in the size_t output argument, the number of lines in the
> + * file in the int argument, and line offsets in the off_t argument,
> + * then flush and rewind the file.
> + */
> +const struct got_error *got_object_file_get_info(size_t *, int *, off_t **,
> +    FILE *);

The got_object_ namespace is the wrong place for this. This is a rather
generic utility which doesn't operate on any of the got_object_* types.

I wonder if this new function is really required.
This function essentially performs a post-processing step on the file
when a search is started. Would it be easily possible to gather the same
information directly in got_object_blob_dump_to_file()?
That could save us from performing a second pass over the file content.

> +static const struct got_error *
>  open_diff_view(struct tog_view *view, struct got_object_id *id1,
>      struct got_object_id *id2, struct tog_view *log_view,
>      struct got_reflist_head *refs, struct got_repository *repo)
>  {
>  	const struct got_error *err;
> +	struct tog_diff_view_state *s = &view->state.diff;

Adding this local variable causes a lot of cosmetic churn in the diff.
Could this cosmetic change be separated out?