From: Mark Jamsek Subject: Re: tog: status bar prompt about togglable events To: Mikhail Cc: gameoftrees@openbsd.org Date: Sat, 21 Jan 2023 19:00:24 +1100 On 23-01-20 09:25PM, Mikhail wrote: > On Fri, Jan 20, 2023 at 09:09:07PM +0300, Mikhail wrote: > > Inlined patch adds short timed prompts to status bar about things like > > switching between diff algorithms, showing white spaces in patches, > > toggling bin/ascii views for blobs, changing committer and author > > representations, and switch between date/name sorting for ref view. > > > > Originally I wanted it because I needed to see current diff algorithm, > > but decided to expand the patch to show all togglable events, except for > > ref view, where, for example, showing IDs speaks for itself by changing > > the look of the table and doesn't require a prompt. I like the idea of this; however, where the information is currently echoed feels a little incongruous. And, perhaps more importantly, depending on the user's terminal width and current view, the information may not even be visible. I'm also not sure about displaying some of these actions. You're right about the display of reference IDs being obvious. Likewise for toggling the reference's last modified date, and showing tree entry IDs too. The algorithm definitely makes sense as it's not easily, if at all, discernable. Likewise for ignoring whitespace. However, toggling between author and committer can be surmised already; granted, you need to enter the diff view to make this distinction. Similarly, forcing ASCII text is also apparent from the diff; in a similar vein, the code doesn't report adjusting context lines as the diff speaks for itself. Lastly, the reference sort order is already evident, especially if the 'm' keymap is toggled but even if not, lexicographic order is obvious in most cases so it's simple to tell when sorted by name or by date. That said, I really like the general idea, but it might be nice to see such action updates echoed to the bottom left of screen where we display prefixed count mod key input. This way, it will always be visible irrespective of terminal width and view, which is important. It is also more consistent behaviour as this is where we already echo responses to user input, and, imo, won't feel out of place like it does appended to the end of the headline. The below diff demonstrates this change in placement. The view struct char pointer 'prompt' is also renamed to 'action' as this seems more appropriate; prompt implies we're requesting input from the user, but the report describes action taken in response to user input. And the wording of some action reports has been changed to be more descriptive too. There's also a related fix that should probably go in regardless of whether this action report lands: in view_loop() we initialise at a refresh rate of one tenth per second before reducing this to one second updates. However, the reduction was never hit due to a typo in the conditional. The following hunk fixes this: @@ -1739,7 +1778,7 @@ view_loop(struct tog_view *view) while (!TAILQ_EMPTY(&views) && !done && !tog_thread_error && !tog_fatal_signal_received()) { /* Refresh fast during initialization, then become slower. */ - if (fast_refresh && fast_refresh-- == 0) + if (fast_refresh && --fast_refresh == 0) halfdelay(10); /* switch to once per second */ err = view_input(&new_view, &done, view, &views); Complete diff: diffstat /home/mark/src/got M tog/tog.c | 55+ 5- 1 file changed, 55 insertions(+), 5 deletions(-) diff /home/mark/src/got commit - 7713cc5e4f5544e81909670d592e89526ed86c9b path + /home/mark/src/got blob - 35282ce50aa4dec0745aa6402cd0b50d87b7416b file + tog/tog.c --- tog/tog.c +++ tog/tog.c @@ -690,6 +690,7 @@ struct tog_view { #define TOG_SEARCH_HAVE_NONE 3 regex_t regex; regmatch_t regmatch; + const char *action; }; static const struct got_error *open_diff_view(struct tog_view *, @@ -1460,6 +1461,35 @@ view_input(struct tog_view **new, int *done, struct to } static const struct got_error * +action_report(struct tog_view *view) +{ + struct tog_view *v = view; + + if (view_is_hsplit_top(view)) + v = view->child; + else if (view->mode == TOG_VIEW_SPLIT_VERT && view->parent) + v = view->parent; + + wmove(v->window, v->nlines - 1, 0); + wclrtoeol(v->window); + if (mvwprintw(v->window, v->nlines - 1, 0, ":%s", view->action) == ERR) + return got_error_msg(GOT_ERR_RANGE, "mvwprintw"); + wrefresh(v->window); + + /* + * Clear action status report. Only clear in blame view + * once annotating is complete, otherwise it's too fast. + */ + if (view->type == TOG_VIEW_BLAME) { + if (view->state.blame.blame_complete) + view->action = NULL; + } else + view->action = NULL; + + return NULL; +} + +static const struct got_error * view_input(struct tog_view **new, int *done, struct tog_view *view, struct tog_view_list_head *views) { @@ -1469,6 +1499,12 @@ view_input(struct tog_view **new, int *done, struct to *new = NULL; + if (view->action) { + err = action_report(view); + if (err) + return err; + } + /* Clear "no matches" indicator. */ if (view->search_next_done == TOG_SEARCH_NO_MORE || view->search_next_done == TOG_SEARCH_HAVE_NONE) { @@ -1668,10 +1704,13 @@ view_input(struct tog_view **new, int *done, struct to err = view->input(new, view, ch); break; case 'A': - if (tog_diff_algo == GOT_DIFF_ALGORITHM_MYERS) + if (tog_diff_algo == GOT_DIFF_ALGORITHM_MYERS) { tog_diff_algo = GOT_DIFF_ALGORITHM_PATIENCE; - else + view->action = "Patience algorithm enabled"; + } else { tog_diff_algo = GOT_DIFF_ALGORITHM_MYERS; + view->action = "Myers algorithm enabled"; + } TAILQ_FOREACH(v, views, entry) { if (v->reset) { err = v->reset(v); @@ -1739,7 +1778,7 @@ view_loop(struct tog_view *view) while (!TAILQ_EMPTY(&views) && !done && !tog_thread_error && !tog_fatal_signal_received()) { /* Refresh fast during initialization, then become slower. */ - if (fast_refresh && fast_refresh-- == 0) + if (fast_refresh && --fast_refresh == 0) halfdelay(10); /* switch to once per second */ err = view_input(&new_view, &done, view, &views); @@ -3718,6 +3757,8 @@ input_log_view(struct tog_view **new_view, struct tog_ break; case '@': s->use_committer = !s->use_committer; + view->action = s->use_committer ? + "show committer enabled" : "show author enabled"; break; case 'G': case '*': @@ -5261,10 +5302,18 @@ input_diff_view(struct tog_view **new_view, struct tog break; case 'a': case 'w': - if (ch == 'a') + if (ch == 'a') { s->force_text_diff = !s->force_text_diff; - else if (ch == 'w') + view->action = s->force_text_diff ? + "force ASCII text enabled" : + "force ASCII text disabled"; + } + else if (ch == 'w') { s->ignore_whitespace = !s->ignore_whitespace; + view->action = s->ignore_whitespace ? + "ignore whitespace enabled" : + "ignore whitespace disabled"; + } err = reset_diff_view(view); break; case 'g': @@ -8233,6 +8282,7 @@ input_ref_view(struct tog_view **new_view, struct tog_ break; case 'o': s->sort_by_date = !s->sort_by_date; + view->action = s->sort_by_date ? "sort by date" : "sort by name"; view->count = 0; err = got_reflist_sort(&tog_refs, s->sort_by_date ? got_ref_cmp_by_commit_timestamp_descending : -- Mark Jamsek GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68