On 22-09-09 09:23AM, Stefan Sperling wrote:
> On Fri, Sep 09, 2022 at 08:23:48AM +0200, Stefan Sperling wrote:
> > What do you think about not highlighting a cursor when its view is not
> > in focus? I believe that would give a better visual impression.
> 
> Something like this (on top of your diff)?
> 
> It seems the ref and tree views already do this. So my patch only tweaks
> the log and blame views. And the diff view doesn't have a selection cursor.

My main concern with this being applied to the log and blame views is
that we provide the ability to scroll both these views from the child
diff view with the JK|,.|<> key maps. By removing the highlight, we no
longer provide a primary visual reference for which entry is selected in
the parent log/blame view. I think this might be a greater loss than the
benefit gained through minimising confusion as to which view is in
focus. Do you think this is a valid concern?

Further, without this visual cue, it looks a bit strange when the entry
selected in the log/blame view is the last or first entry at the bottom
or top of the view, and the log/blame view starts scrolling as you
select the next entry off-screen with JK from the child diff view.

I'm undecided as to whether there's a net gain from this change when
these factors are taken into account. Perhaps there's another way we can
minimise the confusion and accentuate the focussed view without removing
this visual cue in the log and blame views?

> diff /home/stsp/src/got
> commit - b21ed2d92fa9a52695cee221ef20b75b1a182884
> path + /home/stsp/src/got
> blob - 81a72cd907757863416d7c2e4b5924f8413371a0
> file + tog/tog.c
> --- tog/tog.c
> +++ tog/tog.c
> @@ -2396,11 +2396,11 @@ draw_commits(struct tog_view *view)
>  	while (entry) {
>  		if (ncommits >= limit - 1)
>  			break;
> -		if (ncommits == s->selected)
> +		if (view->focussed && ncommits == s->selected)
>  			wstandout(view->window);
>  		err = draw_commit(view, entry->commit, entry->id,
>  		    date_display_cols, author_cols);
> -		if (ncommits == s->selected)
> +		if (view->focussed && ncommits == s->selected)
>  			wstandend(view->window);
>  		if (err)
>  			goto done;
> @@ -5271,14 +5271,14 @@ draw_blame(struct tog_view *view)
>  		wline = NULL;
>  		view->maxx = MAX(view->maxx, width);
>  
> -		if (nprinted == s->selected_line - 1)
> +		if (view->focussed && nprinted == s->selected_line - 1)
>  			wstandout(view->window);
>  
>  		if (blame->nlines > 0) {
>  			blame_line = &blame->lines[lineno - 1];
>  			if (blame_line->annotated && prev_id &&
>  			    got_object_id_cmp(prev_id, blame_line->id) == 0 &&
> -			    !(nprinted == s->selected_line - 1)) {
> +			    nprinted != s->selected_line - 1) {
>  				waddstr(view->window, "        ");
>  			} else if (blame_line->annotated) {
>  				char *id_str;
> @@ -5307,7 +5307,7 @@ draw_blame(struct tog_view *view)
>  			prev_id = NULL;
>  		}
>  
> -		if (nprinted == s->selected_line - 1)
> +		if (view->focussed && nprinted == s->selected_line - 1)
>  			wstandend(view->window);
>  		waddstr(view->window, " ");
>  
> 

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