On Sat, Jul 09, 2022 at 01:52:28AM +1000, Mark Jamsek wrote:
> On 22-07-09 01:21am, Mark Jamsek wrote:
> > On 22-07-08 04:57pm, Stefan Sperling wrote:
> > > On Sat, Jul 09, 2022 at 12:28:58AM +1000, Mark Jamsek wrote:
> > > > This is the log counterpart of the <>/,. key maps to traverse the blamed
> > > > file from the diff view; that is, with a diff view opened from a given
> > > > blamed line, load the previous or next lines with < or >, respectively.
> > > 
> > > I like it.
> > > 
> > > There is a NULL-deref when I move to a line that has not been annotated yet.
> > > It helps to blame a file that takes a while for blame to finish, such as
> > > got/got.c. And using the patience diff algorithm also slows it down.
> > > Can you reproduce this crash?
> > 
> > Ah, yes I can. And it's a boneheaded bug! I'll fix this in a sec :)
> 
> This fixes the NULL deref. Thanks, Stefan :)

There is one remaining issue, fixed with the diff below which applies
on top of yours.

While annotation information is still being built up, moving to another
already annotated line does not update the diff view. This is because
your code assumes that the previously diffed line and the next line
we want to diff are always adjacent.

Fixed below by remembering the last line where we could show a diff.
It now works in both cases, while annotating, and when fully annotated.


diff /home/stsp/src/got
commit - 0d61e0da8c180a8e3b7a3ce8704c90f6e3f61ec4
path + /home/stsp/src/got
blob - 21eb2989e0b91fb066f0e6380febe877bc6f16b7
file + tog/tog.c
--- tog/tog.c
+++ tog/tog.c
@@ -428,6 +428,7 @@ struct tog_blame_view_state {
 	int first_displayed_line;
 	int last_displayed_line;
 	int selected_line;
+	int last_diffed_line;
 	int blame_complete;
 	int eof;
 	int done;
@@ -4601,6 +4602,8 @@ reset_diff_view(struct tog_view *view)
 
 static struct got_object_id *get_selected_commit_id(struct tog_blame_line *,
     int, int, int);
+static struct got_object_id *get_annotation_for_line(struct tog_blame_line *,
+    int, int);
 
 static const struct got_error *
 input_diff_view(struct tog_view **new_view, struct tog_view *view, int ch)
@@ -4770,9 +4773,8 @@ input_diff_view(struct tog_view **new_view, struct tog
 			struct got_object_id *id, *prev_id;
 
 			bs = &s->parent_view->state.blame;
-			prev_id = get_selected_commit_id(bs->blame.lines,
-			    bs->blame.nlines, bs->first_displayed_line,
-			    bs->selected_line);
+			prev_id = get_annotation_for_line(bs->blame.lines,
+			    bs->blame.nlines, bs->last_diffed_line);
 
 			err = input_blame_view(&view, s->parent_view,
 			    up ? KEY_UP : KEY_DOWN);
@@ -5261,6 +5263,22 @@ get_selected_commit_id(struct tog_blame_line *lines, i
 	return line->id;
 }
 
+static struct got_object_id *
+get_annotation_for_line(struct tog_blame_line *lines, int nlines,
+    int lineno)
+{
+	struct tog_blame_line *line;
+
+	if (nlines <= 0 || lineno >= nlines)
+		return NULL;
+
+	line = &lines[lineno - 1];
+	if (!line->annotated)
+		return NULL;
+
+	return line->id;
+}
+
 static const struct got_error *
 stop_blame(struct tog_blame *blame)
 {
@@ -5847,6 +5865,8 @@ input_blame_view(struct tog_view **new_view, struct to
 			view_close(diff_view);
 			break;
 		}
+		s->last_diffed_line = s->first_displayed_line - 1 +
+		    s->selected_line;
 		if (*new_view)
 			break;	/* still open from active diff view */
 		if (view_is_parent_view(view) &&