"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Mark Jamsek <mark@jamsek.com>
Subject:
Re: tog: status bar prompt about togglable events
To:
Mikhail <mp39590@gmail.com>
Cc:
gameoftrees@openbsd.org
Date:
Sun, 22 Jan 2023 19:06:24 +1100

Download raw body.

Thread
On 23-01-21 11:21AM, Mikhail wrote:
> On Sat, Jan 21, 2023 at 07:00:24PM +1100, Mark Jamsek wrote:
> > 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.
> > 
> > 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.
> 
> I've tested the patch - it works and still solves original problem,
> thanks!
> 

Thanks for testing!

I've made a slight adjustment in the action_report() function: we don't
want a fatal error if the action string is truncated so we'll just
ignore the wprintw() return code. It's unlikely someone will be running
tog in a term less than 26 columns wide but we shouldn't error out if
they do.

diffstat 6cdf29f9353bcea609dcbab90131634430ac5a18 e5a23851fd9fdf396289d553749ecbc4a566a9a7
 M  tog/tog.c  |  54+  5-

1 file changed, 54 insertions(+), 5 deletions(-)

diff 6cdf29f9353bcea609dcbab90131634430ac5a18 e5a23851fd9fdf396289d553749ecbc4a566a9a7
commit - 6cdf29f9353bcea609dcbab90131634430ac5a18
commit + e5a23851fd9fdf396289d553749ecbc4a566a9a7
blob - 35282ce50aa4dec0745aa6402cd0b50d87b7416b
blob + a0d1688ab7d888740bba23a982066ac78b0b3a48
--- 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,34 @@ 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);
+	wprintw(v->window, ":%s", view->action);
+	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 +1498,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 +1703,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 +1777,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 +3756,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 +5301,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 +8281,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 <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68