From: Omar Polo <op@omarpolo.com>
Subject: Re: tog: status bar prompt about togglable events
To: Mark Jamsek <mark@jamsek.com>
Cc: Mikhail <mp39590@gmail.com>, gameoftrees@openbsd.org
Date: Sun, 22 Jan 2023 15:56:11 +0100

On 2023/01/22 19:06:24 +1100, Mark Jamsek <mark@jamsek.com> wrote:
> diffstat 6cdf29f9353bcea609dcbab90131634430ac5a18 e5a23851fd9fdf396289d553749ecbc4a566a9a7
>  M  tog/tog.c  |  54+  5-
> 
> 1 file changed, 54 insertions(+), 5 deletions(-)
> 
> diff 6cdf29f9353bcea609dcbab90131634430ac5a18 e5a23851fd9fdf396289d553749ecbc4a566a9a7

I really like it!

ok op@ with or without the few minor nits/suggestions below.

> 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)
> +{

this can't fail now.  having it as a `static void' simplifies the
logic below in view_input.

> +	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);

Why prefixing with ":"?  I've found a bit misleading since for a
second I thought tog accepted vi-style : commands... ^^"

> +	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";

Just a suggestion: i'd say "using Patience/Myers algorithm".

> +		}
>  		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";

similarly here, "show committer"/"show author"

(or "show commit author", but "show commit committer" sounds bad.)

>  		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 :