From: Tracey Emery <tracey@traceyemery.net>
Subject: Re: tog diff view search
To: gameoftrees@openbsd.org
Date: Sat, 1 Feb 2020 09:07:34 -0700

On Sat, Feb 01, 2020 at 12:03:36PM +0100, Stefan Sperling wrote:
> 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

Yeah, I agree I put this in the wrong namespace.

> 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.
> 

Since we're diffing, there is no call to got_object_blob_dump_to_file.
We either diff objects as blobs, trees, or commits, and write that diff
to the filestream. So, either those three functions could be tasked with
the duties, which seems like a lot of work through the tree, or add the
new function, which is based on got_object_blob_dump_to_file.

If we decide to add the new function, it should probably be moved to
tog.c and called something more specific like, dump_diff_filestream_info,
or something.

> > +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?

I don't know. I was using the blame search style as the template. Blame
contains the same local variable. I'll have to test that.

-- 

Tracey Emery