On 22-07-18 03:18pm, Stefan Sperling wrote:
> On Mon, Jul 18, 2022 at 11:12:27PM +1000, Mark Jamsek wrote:
> > On 22-07-18 02:49pm, Stefan Sperling wrote:
> > > On Mon, Jul 18, 2022 at 10:34:27PM +1000, Mark Jamsek wrote:
> > > > If resizing a log view while on the _last_ loaded commit on the bottom
> > > > edge of the screen, some of the new lines are not populated.
> > > > 
> > > > repro:
> > > > $ tog  # 80x24
> > > > 23j  # move down to the last commit
> > > > *increase terminal height to ~30 lines then reduce back to 80x24*
> > > > ~5j  # move down to the _last_ commit
> > > > *increase terminal height to ~33 lines*
> > > > 
> > > > Most of the new lines are empty.
> > > > 
> > > > We call request_log_commits() from view_resize()--but only when the view
> > > > has a child view. The fix simply moves this check, so that we always
> > > > call request_log_commits() if the terminal height has been increased,
> > > > irrespective of whether a child view exists.
> > > 
> > > I only have a concern about style consistency:
> > > 
> > > Could we add a resize function pointer to struct tog_view and call this
> > > (if not NULL) instead of checking the view type and manipulating diff
> > > view state in view_resize() directly?
> > > 
> > > The code would then move into a new resize_diff_view() function.
> > 
> > Sure! That would be better. Do you mean just the logic for requesting
> > commits in the event the terminal height is increased into
> > a resize_log_view() routine to be called from view_resize()?
> 
> Look at how reset_diff_view and reset_blame_view work.
> Any view-specific resizing code could be called in the same way.
> 
> You would probably end up with something like this in view_resize();
> 
> 	if (view->resize) {
> 		err = view->resize(view, dif);
> 		if (err)
> 			return err;
> 	}

Yeah, np! That's what I was thinking but I was thrown with the
resize_diff_view() typo and wasn't sure if you meant a new
resize_*_view() function pointer for each view type. I was pretty sure
that wasn't what you meant as it'd mean a lot of duplicated code but
thought I'd double check.

The below diff might do it :)

diff /home/mark/src/git/got-current
commit - d2587c5f95c6edb51ccc8d4abfac838b58f3a463
path + /home/mark/src/git/got-current
blob - 97ac0690a9232815bb10bc4a237246e2ec53e069
file + tog/tog.c
--- tog/tog.c
+++ tog/tog.c
@@ -517,7 +517,7 @@ struct tog_view {
 	int lines, cols; /* copies of LINES and COLS */
 	int nscrolled, offset; /* lines scrolled and hsplit line offset */
 	int ch, count; /* current keymap and count prefix */
-	int resize; /* set when in a resize event */
+	int resized; /* set when in a resize event */
 	int focussed; /* Only set on one parent or child view at a time. */
 	int dying;
 	struct tog_view *parent;
@@ -549,6 +549,7 @@ struct tog_view {
 	const struct got_error *(*input)(struct tog_view **,
 	    struct tog_view *, int);
 	const struct got_error *(*reset)(struct tog_view *);
+	const struct got_error *(*resize)(struct tog_view *, int);
 	const struct got_error *(*close)(struct tog_view *);

 	const struct got_error *(*search_start)(struct tog_view *);
@@ -731,13 +732,13 @@ view_splitscreen(struct tog_view *view)
 {
 	const struct got_error *err = NULL;

-	if (!view->resize && view->mode == TOG_VIEW_SPLIT_HRZN) {
+	if (!view->resized && view->mode == TOG_VIEW_SPLIT_HRZN) {
 		if (view->resized_y && view->resized_y < view->lines)
 			view->begin_y = view->resized_y;
 		else
 			view->begin_y = view_split_begin_y(view->nlines);
 		view->begin_x = 0;
-	} else if (!view->resize) {
+	} else if (!view->resized) {
 		if (view->resized_x && view->resized_x < view->cols - 1 &&
 		    view->cols > 119)
 			view->begin_x = view->resized_x;
@@ -768,8 +769,8 @@ view_fullscreen(struct tog_view *view)
 	const struct got_error *err = NULL;

 	view->begin_x = 0;
-	view->begin_y = view->resize ? view->begin_y : 0;
-	view->nlines = view->resize ? view->nlines : LINES;
+	view->begin_y = view->resized ? view->begin_y : 0;
+	view->nlines = view->resized ? view->nlines : LINES;
 	view->ncols = COLS;
 	view->lines = LINES;
 	view->cols = COLS;
@@ -875,24 +876,6 @@ view_resize(struct tog_view *view)
 			show_panel(view->child->panel);
 		}
 		/*
-		 * Request commits if terminal height was increased in a log
-		 * view so we have enough commits loaded to populate the view.
-		 */
-		if (view->type == TOG_VIEW_LOG && dif > 0) {
-			struct tog_log_view_state *ts = &view->state.log;
-
-			if (ts->commits.ncommits < ts->selected_entry->idx +
-			    view->lines - ts->selected) {
-				view->nscrolled = ts->selected_entry->idx +
-				    view->lines - ts->selected -
-				    ts->commits.ncommits + dif;
-				err = request_log_commits(view);
-				if (err)
-					return err;
-			}
-		}
-
-		/*
 		 * XXX This is ugly and needs to be moved into the above
 		 * logic but "works" for now and my attempts at moving it
 		 * break either 'tab' or 'F' key maps in horizontal splits.
@@ -915,6 +898,12 @@ view_resize(struct tog_view *view)
 	} else if (view->parent == NULL)
 		ncols = COLS;

+	if (view->resize && dif > 0) {
+		err = view->resize(view, dif);
+		if (err)
+			return err;
+	}
+
 	if (wresize(view->window, nlines, ncols) == ERR)
 		return got_error_from_errno("wresize");
 	if (replace_panel(view->panel, view->window) == ERR)
@@ -929,6 +918,25 @@ view_resize(struct tog_view *view)
 	return NULL;
 }

+static const struct got_error *
+resize_log_view(struct tog_view *view, int increase)
+{
+	struct tog_log_view_state *s = &view->state.log;
+	const struct got_error *err = NULL;
+	int n = s->selected_entry->idx + view->lines - s->selected;
+
+	/*
+	 * Request commits to account for the increased
+	 * height so we have enough to populate the view.
+	 */
+	if (s->commits.ncommits < n) {
+		view->nscrolled = n - s->commits.ncommits + increase + 1;
+		err = request_log_commits(view);
+	}
+
+	return err;
+}
+
 static void
 view_adjust_offset(struct tog_view *view, int n)
 {
@@ -962,7 +970,7 @@ view_resize_split(struct tog_view *view, int resize)
 	if (!v->child || !view_is_splitscreen(v->child))
 		return NULL;

-	v->resize = v->child->resize = resize;  /* lock for resize event */
+	v->resized = v->child->resized = resize;  /* lock for resize event */

 	if (view->mode == TOG_VIEW_SPLIT_HRZN) {
 		int y = v->child->begin_y;
@@ -1031,7 +1039,7 @@ view_resize_split(struct tog_view *view, int resize)
 	else if (v->child->nscrolled)
 		err = request_log_commits(v->child);

-	v->resize = v->child->resize = 0;
+	v->resized = v->child->resized = 0;

 	return err;
 }
@@ -2984,6 +2992,7 @@ open_log_view(struct tog_view *view, struct got_object

 	view->show = show_log_view;
 	view->input = input_log_view;
+	view->resize = resize_log_view;
 	view->close = close_log_view;
 	view->search_start = search_start_log_view;
 	view->search_next = search_next_log_view;


-- 
Mark Jamsek <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68