On 23-01-22 03:56PM, Omar Polo wrote:
> 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.

Thanks, op! I forgot to change the return type when making the change to
ignore the wprintw() result.

I also went with most suggestions, or a very slight variant of the
action statement.

Regarding the ':' prefix, I've kept that. We already display it with our
count modifier in the same way less(1) does. I think it helps demarcate
the report from the surrounding text.

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

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