On Sat, Feb 01, 2020 at 11:19:24AM -0700, Tracey Emery wrote:
> On Sat, Feb 01, 2020 at 06:41:43PM +0100, Stefan Sperling wrote:
> > On Sat, Feb 01, 2020 at 09:54:07AM -0700, Tracey Emery wrote:
> > 
> > As mentioned in my other response, I would prefer a more generic
> > name which doesn't tie this functionality to diff.
> > 
> 
> Renamed to get_filestream_info. Is that ok?

Sure. I can't think of a better suggestion.

> > > -	f = got_opentemp();
> > > -	if (f == NULL) {
> > 
> > What if s->f is not NULL here? Can that happen?
> > If so we should close the old file before opening a new one...
> 
> I couldn't see a path that would make it not NULL, but it doesn't hurt
> to check.

It will happen when the diff context size is adjusted because this
requires us to trigger the entire diffing process again from the
beginning.

> > There's a missing error check here, too, in the existing code.
> > This would be a good opportinity to add one :-)
> 
> Added, and added a missing check.

Thanks!

> Ok on this? Just want one more look-through when you have time.

OK.

The search prompt appears in the middle of the screen for
me when the diff window is maximized, rather than in the
lower left corner. But that can be fixed separately.

Thank you! It's great to get your help on this :)