Download raw body.
tog: horizontal split
Mark Jamsek <mark@jamsek.com> wrote: > The below diff adds support for horizontal splits in tog. first of all, this is awesome. i usually keep my xterms at 80x24 and so I almost never enjoy the split mode. Having a horizontal split in tog instead is a game changer when using smaller/not wide terminals. > We use a couple new envvars to configure splits: > > TOG_VIEW_SPLIT_MODE=(h|v) > v: force vertical split > h: force horizontal split > default/unset: if COLUMNS >= 120 we open a vsplit, else hsplit > TOG_VIEW_SPLIT_HEIGHT=n such that 1 <= n <= LINES - 3 > default/unset: 60% of LINES This is the only thing I'm not too sure about. While stuff like colors is highly subjective and thus having a mean to customize it it's warranted, these kind of settings are a bit too much IMHO. I'd prefer to have a sane default "hardcoded" behaviour and eventually change it if people complains. But lets hear what other thinks about this too. > So if the user does nothing the only change in existing behaviour is > that we'll open a horizontal split when COLUMNS < 120 (i.e., when we > would normally open child views in fullscreen mode now). > > Horizontal splits are supported in log -> diff, tree -> blame, ref -> > log, and blame -> commit. So all the main views will open their primary > child view in a horizontal split, but we haven't yet got some of the > secondary child views (e.g., 't' or 'r' key maps from log view). > > There are a couple bugs that need squishing: > - resizing with an open hsplit sometimes kills the border and there > seems to be an OB1 that makes the top split misrender a line > - the double-width bug in the log view has returned where we overwrite > the vertical border by one column; strangely enough if you open the > commit in ja.git in a vsplit then press F, tab, tab, F, it renders > correctly part of the issues i think it's that we don't properly resize the parent. when resizing during a horizontal split the child view is resized and re-rendered, the parent view not. > The diff's already quite large though (sorry!) and I think it's ready > for more eyes if not use to see what other bugs I've introduced and if > the interface is friendly enough. One thing in particular that I'm not > sure about is, if the user sets an invalid TOG_VIEW_SPLIT_HEIGHT, we > error out. But perhaps it'd be better to fallback to the default value > instead. This is trivial though and can be refined later. If an environment variables has a wrong format I'd just ignore it. If one runs with TOG_VIEW_SPLIT_HEIGHT=banana it's just their fault and they can't expect it to work. On the other hand, returning a fatal error upon split also isn't nice IMHO. Just a few nits spotted while reading the diff. I still don't know the cause of the bugs, will try to look into this with more attention later :) > diff refs/heads/main refs/heads/dev/hsplit > blob - 728ba5ee9e20edbde20ed7f0c2ff1cbabad89c0b > blob + 4df10bd264fa7c164c9bb9b3b233bdce245a1cc4 > --- tog/tog.1 > +++ tog/tog.1 > @@ -76,8 +76,14 @@ Switch focus between views. > .It Cm F > Toggle fullscreen mode for a split-screen view. > .Nm > -will automatically use split-screen views if the size of the terminal > -window is sufficiently large. > +will automatically use vertical split-screen views if the width of the > +terminal window is sufficiently large, otherwise views will be split > +horizontally. > +This behavior can be customized with > +TOG_VIEW_SPLIT_MODE and TOG_VIEW_SPLIT_HEIGHT values. > +See > +.Sx ENVIRONMENT > +for details. > .El > .Pp > Global options must precede the command name, and are as follows: > @@ -560,6 +566,36 @@ work tree, use the repository path associated with thi > .El > .El > .Sh ENVIRONMENT > +Depending on the COLUMNS environment variable, > +.Nm > +will open child views in horizontal or vertical split-screens. > +By default, a view will be opened in a vertical split if COLUMNS \(>= 120. > +Otherwise, the view will be opened in a horizontal split. > +This behavior can be configured with the following environment variables. > +.Bl -tag -width TOG_VIEW_SPLIT_HEIGHT > +.It Ev TOG_VIEW_SPLIT_MODE > +Open views in a horizontal or vertical split. > +Value can be > +.Qq h > +for horizontal > +or > +.Qq v > +for vertical. > +If not set, > +.Nm > +will default to the above described behavior. > +.It Ev TOG_VIEW_SPLIT_HEIGHT > +Height of the child view opened in a horizontal split. > +Value must be an integer in the range 1 \(<= > +.Sy n > +\(<= LINES - 3. > +.Xr expr 1 > +can be used to set terminal-relative values; for example, 50% of the terminal > +height can be approximated with > +.Li TOG_VIEW_SPLIT_HEIGHT=$(expr $LINES \e* 50 / 100) . > +If not set, > +horizontal splits will default to approximately 60% of LINES. > +.El > .Bl -tag -width TOG_COLORS > .It Ev TOG_COLORS > .Nm > blob - e63f4a7c65678ce2cdbd63fb24abb940c2551358 > blob + 3a75f96aaa9c44818fc58dfc7cb0339c0b8d1e0f > --- tog/tog.c > +++ tog/tog.c > @@ -64,6 +64,8 @@ > #define MAX(_a,_b) ((_a) > (_b) ? (_a) : (_b)) > #endif > > +#define ABS(_n) ((_n) >= 0 ? (_n) : -(_n)) you can also use abs(3) instead of a macro. > + > #define CTRL(x) ((x) & 0x1f) > > #ifndef nitems > @@ -105,6 +107,14 @@ enum tog_view_type { > TOG_VIEW_REF, > }; > > +enum tog_view_mode { > + TOG_VIEW_SPLIT_NONE, > + TOG_VIEW_SPLIT_VERT, > + TOG_VIEW_SPLIT_HRZN > +}; > + > > +#define HSPLIT_SCALE 0.4 /* default horizontal split scale */ I'd set this to 0.3. Highly subjective, but I like a bit better with 0.3 on a 80x24 terminal. > + > #define TOG_EOF_STRING "(END)" > > struct commit_queue_entry { > @@ -499,9 +509,10 @@ struct tog_view { > TAILQ_ENTRY(tog_view) entry; > WINDOW *window; > PANEL *panel; > - int nlines, ncols, begin_y, begin_x; > + int nlines, ncols, begin_y, begin_x; /* based on split height/width */ > int maxx, x; /* max column and current start column */ > 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 focussed; /* Only set on one parent or child view at a time. */ > int dying; > @@ -519,6 +530,7 @@ struct tog_view { > */ > int focus_child; > > + enum tog_view_mode mode; > /* type-specific state */ > enum tog_view_type type; > union { > @@ -670,6 +682,7 @@ view_open(int nlines, int ncols, int begin_y, int begi > > view->ch = 0; > view->count = 0; > + view->nscrolled = 0; not really an issue, but calloc already zeroes the struct. There are others zero assignments there tho. > view->type = type; > view->lines = LINES; > view->cols = COLS; > @@ -701,6 +714,36 @@ view_split_begin_x(int begin_x) > return (COLS - MAX(COLS / 2, 80)); > } > > +/* > + * Find start line for horizontal split. If TOG_VIEW_SPLIT_HEIGHT is set and > + * between 1 and lines - 3 (inclusive), subtract value from lines and assign > + * to *y, else assign lines * HSPLIT_SCALE to *y. > + */ > +static const struct got_error * > +view_split_begin_y(int *y, int lines) > +{ > + const char *errstr; > + char *height = NULL; > + int h = 0; > + > + height = getenv("TOG_VIEW_SPLIT_HEIGHT"); > + > + /* > + * XXX Should we error out here or fallback to default if > + * an invalid TOG_VIEW_SPLIT_HEIGHT value has been set? > + */ if we agree on ignoring a malformed TOG_VIEW_SPLIT_HEIGHT then you can also simplify this furter and make this function just return an int. > + if (height) { > + h = strtonum(height, 1, lines - 3, &errstr); > + if (errstr != NULL) > + return got_error_msg(GOT_ERR_RANGE, > + "invalid TOG_VIEW_SPLIT_HEIGHT"); > + *y = lines - h; > + } else > + *y = lines * HSPLIT_SCALE; > + > + return NULL; > +} > + > static const struct got_error *view_resize(struct tog_view *); > > static const struct got_error * > @@ -708,9 +751,16 @@ view_splitscreen(struct tog_view *view) > { > const struct got_error *err = NULL; > > - view->begin_y = 0; > - view->begin_x = view_split_begin_x(0); > - view->nlines = LINES; > + if (view->mode == TOG_VIEW_SPLIT_HRZN) { > + err = view_split_begin_y(&view->begin_y, view->nlines); > + if (err) > + return err; > + view->begin_x = 0; > + } else { > + view->begin_x = view_split_begin_x(0); > + view->begin_y = 0; > + } > + view->nlines = LINES - view->begin_y; > view->ncols = COLS - view->begin_x; > view->lines = LINES; > view->cols = COLS; > @@ -718,10 +768,13 @@ view_splitscreen(struct tog_view *view) > if (err) > return err; > > + if (view->parent && view->mode == TOG_VIEW_SPLIT_HRZN) > + view->parent->nlines = view->lines - view->nlines - 1; > + > if (mvwin(view->window, view->begin_y, view->begin_x) == ERR) > return got_error_from_errno("mvwin"); > I'd leave this as it was before: > - return NULL; > + return err; > } > > static const struct got_error * > @@ -754,28 +807,57 @@ view_is_parent_view(struct tog_view *view) > static int > view_is_splitscreen(struct tog_view *view) > { > - return view->begin_x > 0; > + return view->begin_x > 0 || view->begin_y > 0; > } > > +static void > +view_border(struct tog_view *view) > +{ > + PANEL *panel; > + const struct tog_view *view_above; > > + if (view->parent) this is a new one for me, didn't know you can `return void_function()'. > + return view_border(view->parent); > + > + panel = panel_above(view->panel); > + if (panel == NULL) > + return; > + > + view_above = panel_userptr(panel); > + if (view->mode == TOG_VIEW_SPLIT_HRZN) > + mvwhline(view->window, view_above->begin_y - 1, > + view->begin_x, got_locale_is_utf8() ? > + ACS_HLINE : '-', view->ncols); > + else > + mvwvline(view->window, view->begin_y, view_above->begin_x - 1, > + got_locale_is_utf8() ? ACS_VLINE : '|', view->nlines); > +} > + > +static const struct got_error *request_log_commits(struct tog_view *); > + > static const struct got_error * > view_resize(struct tog_view *view) > { > - int nlines, ncols; > + const struct got_error *err = NULL; > + int dif, nlines, ncols; > > + dif = LINES - view->lines; /* line difference to request commits */ > + > if (view->lines > LINES) > nlines = view->nlines - (view->lines - LINES); > else > nlines = view->nlines + (LINES - view->lines); > - > if (view->cols > COLS) > ncols = view->ncols - (view->cols - COLS); > else > ncols = view->ncols + (COLS - view->cols); > > - if (view->child && view_is_splitscreen(view->child)) { > + if (view->child) { > + int hs = view->child->begin_y; > + > view->child->begin_x = view_split_begin_x(view->begin_x); > - if (view->child->begin_x == 0) { > + if (view->mode == TOG_VIEW_SPLIT_HRZN || > + view->child->begin_x == 0) { > ncols = COLS; > > view_fullscreen(view->child); > @@ -789,6 +871,40 @@ view_resize(struct tog_view *view) > view_splitscreen(view->child); > 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; > + } we need something like this also when quitting the split and falling back to a single log view. To reproduce try (in a 80x24 term) RET to open a diff in a split, TAB to focus the log, C-l to reload the commits, TAB again to focus the diff, q to quit the diff. > + } > + > + /* > + * 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. > + */ :) > + if (hs) { > + err = view_splitscreen(view->child); > + if (err) > + return err; > + view_border(view->child); > + update_panels(); > + doupdate(); > + show_panel(view->child->panel); > + nlines = view->nlines; > + ncols = view->ncols; > + } > } else if (view->parent == NULL) > ncols = COLS; > > @@ -803,7 +919,7 @@ view_resize(struct tog_view *view) > view->lines = LINES; > view->cols = COLS; > > - return NULL; > + return err; I'd keep `return NULL' here. IMHO it makes clearer that if we reach this point there are no errors, otherwise you have to read more carefully the function to see whether err is set sometime before and not bailed out. (even if it means having one more byte :P) > } > > static const struct got_error * > @@ -819,15 +935,29 @@ view_close_child(struct tog_view *view) > return err; > } > > -static const struct got_error * > +static void > view_set_child(struct tog_view *view, struct tog_view *child) > { > view->child = child; > child->parent = view; > > - return view_resize(view); > + return; > } > > +static int > +view_needs_focus_indication(struct tog_view *view) > +{ > + if (view_is_parent_view(view)) { > + if (view->child == NULL || view->child->focussed) > + return 0; > + if (!view_is_splitscreen(view->child)) > + return 0; > + } else if (!view_is_splitscreen(view)) > + return 0; > + > + return view->focussed; > +} > + > static void > tog_resizeterm(void) > { > @@ -919,6 +1049,9 @@ get_compound_key(struct tog_view *view, int c) > return c; > } > > +static const struct got_error *offset_selection_down(struct tog_view *); > +static void offset_selection_up(struct tog_view *); > + > static const struct got_error * > view_input(struct tog_view **new, int *done, struct tog_view *view, > struct tog_view_list_head *views) > @@ -1001,11 +1134,31 @@ view_input(struct tog_view **new, int *done, struct to > view->focussed = 0; > view->parent->focussed = 1; > view->parent->focus_child = 0; > - if (!view_is_splitscreen(view)) > + if (!view_is_splitscreen(view) && > + view->mode == TOG_VIEW_SPLIT_HRZN) { > + if (view->parent->type == TOG_VIEW_LOG) { > + err = request_log_commits(view->parent); > + if (err) > + return err; > + } > err = view_fullscreen(view->parent); > + if (err) > + return err; > + offset_selection_up(view->parent); > + } > } > break; > case 'q': > + if (view->parent && view->mode == TOG_VIEW_SPLIT_HRZN) { > + if (view->parent->type == TOG_VIEW_LOG) { > + /* might need more commits to fill fullscreen */ to simplify things, can't we run this only once at a higher level? Say, every time a split changes or the windows is resized we call request_log_commits (or only if the old size is different from the current.) it would avoid keeping this logic all around the places. (i'm still not familiar on how the flow in tog at a higher level is and thus not sure if this is currently feasible.) > + err = request_log_commits(view->parent); > + if (err) > + break; > + } > + offset_selection_up(view->parent); > + view->parent->mode = TOG_VIEW_SPLIT_NONE; > + } > err = view->input(new, view, ch); > view->dying = 1; > break; > @@ -1034,13 +1187,24 @@ view_input(struct tog_view **new, int *done, struct to > err = view_fullscreen(view); > } else { > err = view_splitscreen(view); > - if (!err) > + if (!err && view->mode != TOG_VIEW_SPLIT_HRZN) > err = view_resize(view->parent); > } > if (err) > break; > err = view->input(new, view, KEY_RESIZE); > } > + if (err) > + break; > + if (view->type == TOG_VIEW_LOG) { > + err = request_log_commits(view); > + if (err) > + break; > + } > + if (view->parent) > + err = offset_selection_down(view->parent); > + if (!err) > + err = offset_selection_down(view); > break; > case KEY_RESIZE: > break; > @@ -1069,38 +1233,6 @@ view_input(struct tog_view **new, int *done, struct to > return err; > } > > -static void > -view_vborder(struct tog_view *view) > -{ > - PANEL *panel; > - const struct tog_view *view_above; > - > - if (view->parent) > - return view_vborder(view->parent); > - > - panel = panel_above(view->panel); > - if (panel == NULL) > - return; > - > - view_above = panel_userptr(panel); > - mvwvline(view->window, view->begin_y, view_above->begin_x - 1, > - got_locale_is_utf8() ? ACS_VLINE : '|', view->nlines); > -} > - > -static int > -view_needs_focus_indication(struct tog_view *view) > -{ > - if (view_is_parent_view(view)) { > - if (view->child == NULL || view->child->focussed) > - return 0; > - if (!view_is_splitscreen(view->child)) > - return 0; > - } else if (!view_is_splitscreen(view)) > - return 0; > - > - return view->focussed; > -} > - > static const struct got_error * > view_loop(struct tog_view *view) > { > @@ -1143,7 +1275,8 @@ view_loop(struct tog_view *view) > if (view->parent) { > view->parent->child = NULL; > view->parent->focus_child = 0; > - > + /* Restore fullscreen line height. */ > + view->parent->nlines = view->parent->lines; > err = view_resize(view->parent); > if (err) > break; > @@ -1907,7 +2040,8 @@ draw_commits(struct tog_view *view) > s->last_displayed_entry = s->first_displayed_entry; > ncommits = 0; > while (entry) { > - if (ncommits >= limit - 1) > + if (ncommits >= (view->mode == TOG_VIEW_SPLIT_HRZN ? > + view->nlines - 1 : view->lines - 1)) i think that here we should always use nlines. lines should only be a copy of the value of LINES last time we looked at it. > break; > if (ncommits == s->selected) > wstandout(view->window); > @@ -1922,7 +2056,7 @@ draw_commits(struct tog_view *view) > entry = TAILQ_NEXT(entry, entry); > } > > - view_vborder(view); > + view_border(view); > done: > free(id_str); > free(refs_str); > @@ -1997,6 +2131,19 @@ trigger_log_thread(struct tog_view *view, int wait) > } > > static const struct got_error * > +request_log_commits(struct tog_view *view) > +{ > + struct tog_log_view_state *state = &view->state.log; > + const struct got_error *err = NULL; > + > + state->thread_args.commits_needed = view->nscrolled; > + err = trigger_log_thread(view, 1); > + view->nscrolled = 0; > + > + return err; > +} > + > +static const struct got_error * > log_scroll_down(struct tog_view *view, int maxscroll) > { > struct tog_log_view_state *s = &view->state.log; > @@ -2021,10 +2168,11 @@ log_scroll_down(struct tog_view *view, int maxscroll) > > do { > pentry = TAILQ_NEXT(s->last_displayed_entry, entry); > - if (pentry == NULL) > + if (pentry == NULL && view->mode != TOG_VIEW_SPLIT_HRZN) > break; > > - s->last_displayed_entry = pentry; > + s->last_displayed_entry = pentry ? > + pentry : s->last_displayed_entry;; > > pentry = TAILQ_NEXT(s->first_displayed_entry, entry); > if (pentry == NULL) > @@ -2032,11 +2180,16 @@ log_scroll_down(struct tog_view *view, int maxscroll) > s->first_displayed_entry = pentry; > } while (++nscrolled < maxscroll); > > + if (view->mode == TOG_VIEW_SPLIT_HRZN) > + view->nscrolled += nscrolled; > + else > + view->nscrolled = 0; > + > return err; > } > > static const struct got_error * > -open_diff_view_for_commit(struct tog_view **new_view, int begin_x, > +open_diff_view_for_commit(struct tog_view **new_view, int begin_y, int begin_x, > struct got_commit_object *commit, struct got_object_id *commit_id, > struct tog_view *log_view, struct got_repository *repo) > { > @@ -2044,7 +2197,7 @@ open_diff_view_for_commit(struct tog_view **new_view, > struct got_object_qid *parent_id; > struct tog_view *diff_view; > > - diff_view = view_open(0, 0, 0, begin_x, TOG_VIEW_DIFF); > + diff_view = view_open(0, 0, begin_y, begin_x, TOG_VIEW_DIFF); > if (diff_view == NULL) > return got_error_from_errno("view_open"); > > @@ -2609,7 +2762,133 @@ show_log_view(struct tog_view *view) > return draw_commits(view); > } > > +static void > +log_move_cursor_up(struct tog_view *view, int page, int home) > +{ > + struct tog_log_view_state *s = &view->state.log; > + > + if (s->selected_entry->idx == 0) > + view->count = 0; > + if (s->first_displayed_entry == NULL) > + return; > + > + if ((page && TAILQ_FIRST(&s->commits.head) == s->first_displayed_entry) > + || home) > + s->selected = home ? 0 : MAX(0, s->selected - page - 1); > + > + if (!page && !home && s->selected > 0) > + --s->selected; > + else > + log_scroll_up(s, home ? s->commits.ncommits : MAX(page, 1)); > + > + select_commit(s); > + return; > +} > + > static const struct got_error * > +log_move_cursor_down(struct tog_view *view, int page) > +{ > + struct tog_log_view_state *s = &view->state.log; > + struct commit_queue_entry *first; > + const struct got_error *err = NULL; > + > + first = s->first_displayed_entry; > + if (first == NULL) { > + view->count = 0; > + return err; likewise, I'd say "return NULL" since we know there isn't an error. it makes it clearer to the reader. > + } > + > + if (s->thread_args.log_complete && > + s->selected_entry->idx >= s->commits.ncommits - 1) > + return err; > + > + if (!page) { > + if (s->selected < MIN(view->nlines - 2, > + s->commits.ncommits - 1)) > + ++s->selected; > + else > + err = log_scroll_down(view, 1); > + } else if (s->thread_args.log_complete) { > + if (s->last_displayed_entry->idx == s->commits.ncommits - 1) > + s->selected += MIN(s->last_displayed_entry->idx - > + s->selected_entry->idx, page + 1); > + else > + err = log_scroll_down(view, MIN(page, > + s->commits.ncommits - s->selected_entry->idx - 1)); > + s->selected = MIN(view->nlines - 2, s->commits.ncommits - 1); > + } else { > + err = log_scroll_down(view, page); > + if (err) > + return err; > + if (first == s->first_displayed_entry && s->selected < > + MIN(view->nlines - 2, s->commits.ncommits - 1)) { > + s->selected = MIN(s->commits.ncommits - 1, page); > + } > + } > + if (err) > + return err; > + > + /* > + * We might necessarily overshoot in horizontal > + * splits; if so, select the last displayed commit. > + */ > + s->selected = MIN(s->selected, > + s->last_displayed_entry->idx - s->first_displayed_entry->idx); > + > + select_commit(s); > + > + if (s->thread_args.log_complete && > + s->selected_entry->idx == s->commits.ncommits - 1) > + view->count = 0; > + > + return err; > +} > + > +/* > + * Get splitscreen dimensions. If TOG_VIEW_SPLIT_MODE isn't set and COLS > 119, > + * open vertical split, else open horizontal split. If set to 'v' or 'h', > + * assign start column or line of the new split to *x or *y, respectively, and > + * assign view mode to view->mode. > + */ > +static const struct got_error * > +view_get_split(struct tog_view *view, int *y, int *x) > +{ > + const struct got_error *err = NULL; > + char *mode; > + > + mode = getenv("TOG_VIEW_SPLIT_MODE"); > + > + if (!mode || mode[0] != 'h') { > + view->mode = TOG_VIEW_SPLIT_VERT; > + *x = view_split_begin_x(view->begin_x); > + } > + > + if (!*x && (!mode || mode[0] != 'v')) { > + view->mode = TOG_VIEW_SPLIT_HRZN; > + err = view_split_begin_y(y, view->lines); > + } > + > + return err; > +} > + > +/* Split view horizontally at y and offset view->state->selected line. */ > +static const struct got_error * > +view_init_hsplit(struct tog_view *view, int y) > +{ > + const struct got_error *err = NULL; > + > + view->nlines = y; > + err = view_resize(view); > + if (err) > + return err; > + since the border is drawn *on top* of the window and in this case it's would override only a line, you can avoid changing view->nlines here, it doesn't have any issues like drawing on top of a column has. > + view->nlines = y - 1; /* border */ > + err = offset_selection_down(view); > + > + return err; > +} > + > +static const struct got_error * > input_log_view(struct tog_view **new_view, struct tog_view *view, int ch) > { > const struct got_error *err = NULL; > @@ -2617,17 +2896,14 @@ input_log_view(struct tog_view **new_view, struct tog_ > struct tog_view *diff_view = NULL, *tree_view = NULL; > struct tog_view *ref_view = NULL; > struct commit_queue_entry *entry; > - int begin_x = 0, n, nscroll = view->nlines - 1; > + int begin_x = 0, begin_y = 0, n, nscroll = view->nlines - 1; > > if (s->thread_args.load_all) { > if (ch == KEY_BACKSPACE) > s->thread_args.load_all = 0; > else if (s->thread_args.log_complete) { > s->thread_args.load_all = 0; > - log_scroll_down(view, s->commits.ncommits); > - s->selected = MIN(view->nlines - 2, > - s->commits.ncommits - 1); > - select_commit(s); > + err = log_move_cursor_down(view, s->commits.ncommits); for one, I like the semplifications log_move_cursor_down/up brings here. if you're concerned about the size of the diff you could factor this change out and commit it before. At least I would OK such a diff :) > } > return NULL; > } > @@ -2661,21 +2937,11 @@ input_log_view(struct tog_view **new_view, struct tog_ > case '<': > case ',': > case CTRL('p'): > - if (s->selected_entry->idx == 0) > - view->count = 0; > - if (s->first_displayed_entry == NULL) > - break; > - if (s->selected > 0) > - s->selected--; > - else > - log_scroll_up(s, 1); > - select_commit(s); > + log_move_cursor_up(view, 0, 0); > break; > case 'g': > case KEY_HOME: > - s->selected = 0; > - s->first_displayed_entry = TAILQ_FIRST(&s->commits.head); > - select_commit(s); > + log_move_cursor_up(view, 0, 1); > view->count = 0; > break; > case CTRL('u'): > @@ -2685,35 +2951,14 @@ input_log_view(struct tog_view **new_view, struct tog_ > case KEY_PPAGE: > case CTRL('b'): > case 'b': > - if (s->first_displayed_entry == NULL) > - break; > - if (TAILQ_FIRST(&s->commits.head) == s->first_displayed_entry) > - s->selected = MAX(0, s->selected - nscroll - 1); > - else > - log_scroll_up(s, nscroll); > - select_commit(s); > - if (s->selected_entry->idx == 0) > - view->count = 0; > + log_move_cursor_up(view, nscroll, 0); > break; > case 'j': > case KEY_DOWN: > case '>': > case '.': > case CTRL('n'): > - if (s->first_displayed_entry == NULL) > - break; > - if (s->selected < MIN(view->nlines - 2, > - s->commits.ncommits - 1)) > - s->selected++; > - else { > - err = log_scroll_down(view, 1); > - if (err) > - break; > - } > - select_commit(s); > - if (s->thread_args.log_complete && > - s->selected_entry->idx == s->commits.ncommits - 1) > - view->count = 0; > + err = log_move_cursor_down(view, 0); > break; > case 'G': > case KEY_END: { > @@ -2745,29 +2990,9 @@ input_log_view(struct tog_view **new_view, struct tog_ > case KEY_NPAGE: > case CTRL('f'): > case 'f': > - case ' ': { > - struct commit_queue_entry *first; > - first = s->first_displayed_entry; > - if (first == NULL) { > - view->count = 0; > - break; > - } > - err = log_scroll_down(view, nscroll); > - if (err) > - break; > - if (first == s->first_displayed_entry && > - s->selected < MIN(view->nlines - 2, > - s->commits.ncommits - 1)) { > - /* can't scroll further down */ > - s->selected += MIN(s->last_displayed_entry->idx - > - s->selected_entry->idx, nscroll + 1); > - } > - select_commit(s); > - if (s->thread_args.log_complete && > - s->selected_entry->idx == s->commits.ncommits - 1) > - view->count = 0; > + case ' ': > + err = log_move_cursor_down(view, nscroll); > break; > - } > case KEY_RESIZE: > if (s->selected > view->nlines - 2) > s->selected = view->nlines - 2; > @@ -2782,30 +3007,45 @@ input_log_view(struct tog_view **new_view, struct tog_ > } > break; > case KEY_ENTER: > - case '\r': > + case '\r': { > view->count = 0; > if (s->selected_entry == NULL) > break; > - if (view_is_parent_view(view)) > - begin_x = view_split_begin_x(view->begin_x); > - err = open_diff_view_for_commit(&diff_view, begin_x, > + > + /* get dimensions--don't split till initialisation succeeds */ > + if (view_is_parent_view(view)) { > + err = view_get_split(view, &begin_y, &begin_x); > + if (err) > + break; > + } > + > + err = open_diff_view_for_commit(&diff_view, begin_y, begin_x, > s->selected_entry->commit, s->selected_entry->id, > view, s->repo); > if (err) > break; > + > + if (view->mode == TOG_VIEW_SPLIT_HRZN) { /* safe to split */ > + err = view_init_hsplit(view, begin_y); > + if (err) > + break; > + } > + > view->focussed = 0; > diff_view->focussed = 1; > + diff_view->mode = view->mode; > + diff_view->nlines = view->lines - begin_y; > + > if (view_is_parent_view(view)) { > err = view_close_child(view); > if (err) > return err; > - err = view_set_child(view, diff_view); > - if (err) > - return err; > + view_set_child(view, diff_view); > view->focus_child = 1; > } else > *new_view = diff_view; > break; > + } > case 't': > view->count = 0; > if (s->selected_entry == NULL) > @@ -2823,9 +3063,7 @@ input_log_view(struct tog_view **new_view, struct tog_ > err = view_close_child(view); > if (err) > return err; > - err = view_set_child(view, tree_view); > - if (err) > - return err; > + view_set_child(view, tree_view); > view->focus_child = 1; > } else > *new_view = tree_view; > @@ -2913,9 +3151,7 @@ input_log_view(struct tog_view **new_view, struct tog_ > err = view_close_child(view); > if (err) > return err; > - err = view_set_child(view, ref_view); > - if (err) > - return err; > + view_set_child(view, ref_view); > view->focus_child = 1; > } else > *new_view = ref_view; > @@ -3416,7 +3652,7 @@ draw_file(struct tog_view *view, const char *header) > else > s->last_displayed_line = s->first_displayed_line; > > - view_vborder(view); > + view_border(view); > > if (s->eof) { > while (nprinted < view->nlines) { > @@ -4596,7 +4832,7 @@ draw_blame(struct tog_view *view) > free(line); > s->last_displayed_line = lineno; > > - view_vborder(view); > + view_border(view); > > return NULL; > } > @@ -5042,7 +5278,7 @@ show_blame_view(struct tog_view *view) > > err = draw_blame(view); > > - view_vborder(view); > + view_border(view); > return err; > } > > @@ -5052,7 +5288,7 @@ input_blame_view(struct tog_view **new_view, struct to > const struct got_error *err = NULL, *thread_err = NULL; > struct tog_view *diff_view; > struct tog_blame_view_state *s = &view->state.blame; > - int begin_x = 0, nscroll = view->nlines - 2; > + int begin_y = 0, begin_x = 0, nscroll = view->nlines - 2; > > switch (ch) { > case '0': > @@ -5244,11 +5480,14 @@ input_blame_view(struct tog_view **new_view, struct to > err = got_object_open_as_commit(&commit, s->repo, id); > if (err) > break; > - pid = STAILQ_FIRST( > - got_object_commit_get_parent_ids(commit)); > - if (view_is_parent_view(view)) > - begin_x = view_split_begin_x(view->begin_x); > - diff_view = view_open(0, 0, 0, begin_x, TOG_VIEW_DIFF); > + pid = STAILQ_FIRST(got_object_commit_get_parent_ids(commit)); > + if (view_is_parent_view(view)) { > + err = view_get_split(view, &begin_y, &begin_x); > + if (err) > + break; > + } > + > + diff_view = view_open(0, 0, begin_y, begin_x, TOG_VIEW_DIFF); > if (diff_view == NULL) { > got_object_commit_close(commit); > err = got_error_from_errno("view_open"); > @@ -5261,15 +5500,21 @@ input_blame_view(struct tog_view **new_view, struct to > view_close(diff_view); > break; > } > + if (view->mode == TOG_VIEW_SPLIT_HRZN) { > + err = view_init_hsplit(view, begin_y); > + if (err) > + break; > + } > + > view->focussed = 0; > diff_view->focussed = 1; > + diff_view->mode = view->mode; > + diff_view->nlines = view->lines - begin_y; > if (view_is_parent_view(view)) { > err = view_close_child(view); > if (err) > break; > - err = view_set_child(view, diff_view); > - if (err) > - break; > + view_set_child(view, diff_view); > view->focus_child = 1; > } else > *new_view = diff_view; > @@ -5639,9 +5884,10 @@ tree_scroll_up(struct tog_tree_view_state *s, int maxs > } > } > > -static void > -tree_scroll_down(struct tog_tree_view_state *s, int maxscroll) > +static const struct got_error * > +tree_scroll_down(struct tog_view *view, int maxscroll) > { > + struct tog_tree_view_state *s = &view->state.tree; > struct got_tree_entry *next, *last; > int n = 0; > > @@ -5652,13 +5898,16 @@ tree_scroll_down(struct tog_tree_view_state *s, int ma > next = got_object_tree_get_first_entry(s->tree); > > last = s->last_displayed_entry; > - while (next && last && n++ < maxscroll) { > - last = got_tree_entry_get_next(s->tree, last); > - if (last) { > + while (next && n++ < maxscroll) { > + if (last) > + last = got_tree_entry_get_next(s->tree, last); > + if (last || (view->mode == TOG_VIEW_SPLIT_HRZN && next)) { > s->first_displayed_entry = next; > next = got_tree_entry_get_next(s->tree, next); > } > } > + > + return NULL; > } > > static const struct got_error * > @@ -5708,7 +5957,7 @@ done: > } > > static const struct got_error * > -blame_tree_entry(struct tog_view **new_view, int begin_x, > +blame_tree_entry(struct tog_view **new_view, int begin_y, int begin_x, > struct got_tree_entry *te, struct tog_parent_trees *parents, > struct got_object_id *commit_id, struct got_repository *repo) > { > @@ -5722,7 +5971,7 @@ blame_tree_entry(struct tog_view **new_view, int begin > if (err) > return err; > > - blame_view = view_open(0, 0, 0, begin_x, TOG_VIEW_BLAME); > + blame_view = view_open(0, 0, begin_y, begin_x, TOG_VIEW_BLAME); > if (blame_view == NULL) { > err = got_error_from_errno("view_open"); > goto done; > @@ -5987,7 +6236,7 @@ show_tree_view(struct tog_view *view) > err = draw_tree_entries(view, parent_path); > free(parent_path); > > - view_vborder(view); > + view_border(view); > return err; > } > > @@ -6018,9 +6267,7 @@ input_tree_view(struct tog_view **new_view, struct tog > err = view_close_child(view); > if (err) > return err; > - err = view_set_child(view, log_view); > - if (err) > - return err; > + view_set_child(view, log_view); > view->focus_child = 1; > } else > *new_view = log_view; > @@ -6044,9 +6291,7 @@ input_tree_view(struct tog_view **new_view, struct tog > err = view_close_child(view); > if (err) > return err; > - err = view_set_child(view, ref_view); > - if (err) > - return err; > + view_set_child(view, ref_view); > view->focus_child = 1; > } else > *new_view = ref_view; > @@ -6127,7 +6372,7 @@ input_tree_view(struct tog_view **new_view, struct tog > view->count = 0; > break; > } > - tree_scroll_down(s, 1); > + tree_scroll_down(view, 1); > break; > case CTRL('d'): > case 'd': > @@ -6147,7 +6392,7 @@ input_tree_view(struct tog_view **new_view, struct tog > view->count = 0; > break; > } > - tree_scroll_down(s, nscroll); > + tree_scroll_down(view, nscroll); > break; > case KEY_ENTER: > case '\r': > @@ -6169,6 +6414,8 @@ input_tree_view(struct tog_view **new_view, struct tog > s->selected_entry = > parent->selected_entry; > s->selected = parent->selected; > + if (s->selected > view->nlines - 3) > + offset_selection_down(view); > free(parent); > } else if (S_ISDIR(got_tree_entry_get_mode( > s->selected_entry))) { > @@ -6186,24 +6433,36 @@ input_tree_view(struct tog_view **new_view, struct tog > } else if (S_ISREG(got_tree_entry_get_mode( > s->selected_entry))) { > struct tog_view *blame_view; > - int begin_x = view_is_parent_view(view) ? > - view_split_begin_x(view->begin_x) : 0; > + int begin_x = 0, begin_y = 0; > > - err = blame_tree_entry(&blame_view, begin_x, > + if (view_is_parent_view(view)) { > + err = view_get_split(view, &begin_y, &begin_x); > + if (err) > + break; > + } > + > + err = blame_tree_entry(&blame_view, begin_y, begin_x, > s->selected_entry, &s->parents, > s->commit_id, s->repo); > if (err) > break; > + > + if (view->mode == TOG_VIEW_SPLIT_HRZN) { > + err = view_init_hsplit(view, begin_y); > + if (err) > + break; > + } > + > view->count = 0; > view->focussed = 0; > blame_view->focussed = 1; > + blame_view->mode = view->mode; > + blame_view->nlines = view->lines - begin_y; > if (view_is_parent_view(view)) { > err = view_close_child(view); > if (err) > return err; > - err = view_set_child(view, blame_view); > - if (err) > - return err; > + view_set_child(view, blame_view); > view->focus_child = 1; > } else > *new_view = blame_view; > @@ -6544,7 +6803,7 @@ done: > } > > static const struct got_error * > -log_ref_entry(struct tog_view **new_view, int begin_x, > +log_ref_entry(struct tog_view **new_view, int begin_y, int begin_x, > struct tog_reflist_entry *re, struct got_repository *repo) > { > struct tog_view *log_view; > @@ -6561,7 +6820,7 @@ log_ref_entry(struct tog_view **new_view, int begin_x, > return NULL; > } > > - log_view = view_open(0, 0, 0, begin_x, TOG_VIEW_LOG); > + log_view = view_open(0, 0, begin_y, begin_x, TOG_VIEW_LOG); > if (log_view == NULL) { > err = got_error_from_errno("view_open"); > goto done; > @@ -6596,9 +6855,10 @@ ref_scroll_up(struct tog_ref_view_state *s, int maxscr > } > } > > -static void > -ref_scroll_down(struct tog_ref_view_state *s, int maxscroll) > +static const struct got_error * > +ref_scroll_down(struct tog_view *view, int maxscroll) > { > + struct tog_ref_view_state *s = &view->state.ref; > struct tog_reflist_entry *next, *last; > int n = 0; > > @@ -6608,13 +6868,16 @@ ref_scroll_down(struct tog_ref_view_state *s, int maxs > next = TAILQ_FIRST(&s->refs); > > last = s->last_displayed_entry; > - while (next && last && n++ < maxscroll) { > - last = TAILQ_NEXT(last, entry); > - if (last) { > + while (next && n++ < maxscroll) { > + if (last) > + last = TAILQ_NEXT(last, entry); > + if (last || (view->mode == TOG_VIEW_SPLIT_HRZN)) { > s->first_displayed_entry = next; > next = TAILQ_NEXT(next, entry); > } > } > + > + return NULL; maybe this is a leftover from a previous attempt? why making this return error when it unconditionally returns NULL? > } > > static const struct got_error * > @@ -6847,7 +7110,7 @@ show_ref_view(struct tog_view *view) > re = TAILQ_NEXT(re, entry); > } > > - view_vborder(view); > + view_border(view); > return err; > } > > @@ -6893,7 +7156,7 @@ input_ref_view(struct tog_view **new_view, struct tog_ > struct tog_ref_view_state *s = &view->state.ref; > struct tog_view *log_view, *tree_view; > struct tog_reflist_entry *re; > - int begin_x = 0, n, nscroll = view->nlines - 1; > + int begin_y = 0, begin_x = 0, n, nscroll = view->nlines - 1; > > switch (ch) { > case 'i': > @@ -6925,19 +7188,32 @@ input_ref_view(struct tog_view **new_view, struct tog_ > view->count = 0; > if (!s->selected_entry) > break; > - if (view_is_parent_view(view)) > - begin_x = view_split_begin_x(view->begin_x); > - err = log_ref_entry(&log_view, begin_x, s->selected_entry, > - s->repo); > + if (view_is_parent_view(view)) { > + err = view_get_split(view, &begin_y, &begin_x); > + if (err) > + break; > + } > + > + err = log_ref_entry(&log_view, begin_y, begin_x, > + s->selected_entry, s->repo); > + if (err) > + break; > + > + if (view->mode == TOG_VIEW_SPLIT_HRZN) { > + err = view_init_hsplit(view, begin_y); > + if (err) > + break; > + } > + > view->focussed = 0; > log_view->focussed = 1; > + log_view->mode = view->mode; > + log_view->nlines = view->lines - begin_y; > if (view_is_parent_view(view)) { > err = view_close_child(view); > if (err) > return err; > - err = view_set_child(view, log_view); > - if (err) > - return err; > + view_set_child(view, log_view); > view->focus_child = 1; > } else > *new_view = log_view; > @@ -6958,9 +7234,7 @@ input_ref_view(struct tog_view **new_view, struct tog_ > err = view_close_child(view); > if (err) > return err; > - err = view_set_child(view, tree_view); > - if (err) > - return err; > + view_set_child(view, tree_view); > view->focus_child = 1; > } else > *new_view = tree_view; > @@ -7021,7 +7295,7 @@ input_ref_view(struct tog_view **new_view, struct tog_ > view->count = 0; > break; > } > - ref_scroll_down(s, 1); > + ref_scroll_down(view, 1); > break; > case CTRL('d'): > case 'd': > @@ -7041,7 +7315,7 @@ input_ref_view(struct tog_view **new_view, struct tog_ > view->count = 0; > break; > } > - ref_scroll_down(s, nscroll); > + ref_scroll_down(view, nscroll); > break; > case CTRL('l'): > view->count = 0; > @@ -7174,7 +7448,117 @@ done: > return error; > } > > +/* > + * If view was scrolled down to move the selected line into view when opening a > + * horizontal split, scroll back up when closing the split/toggling fullscreen. > + */ > static void > +offset_selection_up(struct tog_view *view) > +{ > + switch (view->type) { > + case TOG_VIEW_BLAME: { > + struct tog_blame_view_state *s = &view->state.blame; > + if (s->first_displayed_line == 1) { > + s->selected_line = MAX(s->selected_line - view->offset, > + 1); > + break; > + } > + if (s->first_displayed_line > view->offset) > + s->first_displayed_line -= view->offset; > + else > + s->first_displayed_line = 1; > + s->selected_line += view->offset; > + break; > + } I'd avoid the braces when we're not introducing variables. > + case TOG_VIEW_LOG: { > + log_scroll_up(&view->state.log, view->offset); > + view->state.log.selected += view->offset; > + break; > + } > + case TOG_VIEW_REF: { > + ref_scroll_up(&view->state.ref, view->offset); > + view->state.ref.selected += view->offset; > + break; > + } > + case TOG_VIEW_TREE: { > + tree_scroll_up(&view->state.tree, view->offset); > + view->state.tree.selected += view->offset; > + break; > + } > + default: > + break; > + } > + > + view->offset = 0; > + return; > +} > + > +/* > + * If the selected line is in the section of screen covered by the bottom split, > + * scroll down offset lines to move it into view and index its new position. > + */ > +static const struct got_error * > +offset_selection_down(struct tog_view *view) > +{ > + const struct got_error *err = NULL; > + const struct got_error *(*scrolld)(struct tog_view *, int); > + int *selected = NULL; > + int header, offset; > + > + switch (view->type) { > + case TOG_VIEW_BLAME: { > + struct tog_blame_view_state *s = &view->state.blame; > + header = 2; > + scrolld = NULL; > + if (s->selected_line > view->nlines - header) { > + offset = ABS(view->nlines - s->selected_line - header); > + s->first_displayed_line += offset; > + s->selected_line -= offset; > + view->offset = offset; > + } > + break; > + } > + case TOG_VIEW_LOG: { > + struct tog_log_view_state *s = &view->state.log; > + scrolld = &log_scroll_down; > + header = 2; > + selected = &s->selected; > + break; > + } > + case TOG_VIEW_REF: { > + struct tog_ref_view_state *s = &view->state.ref; > + scrolld = &ref_scroll_down; > + header = 2; > + selected = &s->selected; > + break; > + } > + case TOG_VIEW_TREE: { > + struct tog_tree_view_state *s = &view->state.tree; > + scrolld = &tree_scroll_down; > + header = 4; > + selected = &s->selected; > + break; > + } > + default: > + selected = NULL; > + scrolld = NULL; > + header = 0; > + break; > + } > + > + if (selected && *selected > view->nlines - header) { > + offset = ABS(view->nlines - *selected - header); > + view->offset = offset; > + if (scrolld && offset) { > + err = scrolld(view, offset); > + *selected -= offset; > + } > + } > + > + return err; > +} > + > +static void > list_commands(FILE *fp) > { > size_t i;
tog: horizontal split