From: Mark Jamsek <mark@jamsek.com>
Subject: Re: show base commit markers in tog log view
To: Stefan Sperling <stsp@stsp.name>
Cc: gameoftrees@openbsd.org
Date: Sun, 23 Jul 2023 23:17:38 +1000

Stefan Sperling <stsp@stsp.name> wrote:
> On Sun, Jul 23, 2023 at 11:26:33AM +0200, Stefan Sperling wrote:
> > On Sun, Jul 23, 2023 at 12:01:07PM +1000, Mark Jamsek wrote:
> > > Mark Jamsek <mark@jamsek.com> wrote:
> > > > Stefan Sperling <stsp@stsp.name> wrote:
> > > > > On Sun, Jul 23, 2023 at 12:19:31AM +1000, Mark Jamsek wrote:
> > > > > >  tog: show work tree base commit marker in log view
> > > > > 
> > > > > Yes, I like this very much, thank you! ok by me.
> > > > > 
> > > > > Also ok by me for the got branch -l changes + tests.
> > > > 
> > > > Great, thanks!
> > > > 
> > > > > Should the tog documentation mention * and ~ somewhere? Your latest
> > > > > patch doesn't seem to update tog.1; maybe I missed an earlier change?
> > > > 
> > > > I considered it briefly but didn't act on it; however, it makes sense to
> > > > mention this. I've basically lifted the paragraph and list from the
> > > > got.1 branch -l docs and inserted it into tog.1's log entry:
> > > > 
> > > > (diff applies on the previous and also includes your change to the
> > > > got branch -l docs)
> > > 
> > > Actually, I just noticed we're individually passing each member of the
> > > commit_queue_entry struct to draw_commit(), so pass the struct instead.
> > > 
> > > (I'll fold this into the tog base commit marker diff before sending
> > > but thought this would be easier to review)
> > 
> > sure, ok stsp
> 
> Could we make the commit marker use the samec colour as commits (defaults
> to green) instead of authors (defaults to cyan)?

This makes the coloured sections uneven for the selected entry:

with patch (observe the two column gap between "mark" and '*'):

  https://ss.jamsek.net/20230723T225217-ss.png

without patch:

  https://ss.jamsek.net/20230723T225333-ss.png

I think it looks nicer without the patch for the selected entry.

But if we keep the author colour scheme all the way up to the '*' (as
in the below diff), it looks a bit better:

  https://ss.jamsek.net/20230723T230108-ss.png

Although, I still prefer it as is; however, I won't object either way,
I just wanted to point out the difference and express my aesthetic
preference. Also, I think as it is, the contrast between the '*' and the
changing colour of the log message section makes it [the '*' or '~']
stand out more.

Full disclosure: I haven't tested on a dark colour scheme so this could
be entirely the fault of my environment.

diff /home/mark/src/got
commit - 6ed9ffec8b7318ac6f69a138854afefc48bca2c0
path + /home/mark/src/got
blob - 7850a8a50fe8ff23c3f0d68ab713639f97d85090
file + tog/tog.c
--- tog/tog.c
+++ tog/tog.c
@@ -2484,9 +2484,16 @@ draw_commit(struct tog_view *view, struct commit_queue
 	while (col < avail && author_width < author_display_cols + 2) {
 		if (tog_base_commit.id != NULL &&
 		    author_width == marker_column &&
-		    entry->idx == tog_base_commit.idx)
+		    entry->idx == tog_base_commit.idx) {
+			tc = get_color(&s->colors, TOG_COLOR_COMMIT);
+			if (tc)
+				wattr_on(view->window,
+				    COLOR_PAIR(tc->colorpair), NULL);
 			waddch(view->window, tog_base_commit.marker);
-		else
+			if (tc)
+				wattr_off(view->window,
+				    COLOR_PAIR(tc->colorpair), NULL);
+		} else
 			waddch(view->window, ' ');
 		col++;
 		author_width++;


> 
> diff /home/stsp/src/got
> commit - 6ed9ffec8b7318ac6f69a138854afefc48bca2c0
> path + /home/stsp/src/got
> blob - 7850a8a50fe8ff23c3f0d68ab713639f97d85090
> file + tog/tog.c
> --- tog/tog.c
> +++ tog/tog.c
> @@ -2480,20 +2480,27 @@ draw_commit(struct tog_view *view, struct commit_queue
>  		wattr_on(view->window,
>  		    COLOR_PAIR(tc->colorpair), NULL);
>  	waddwstr(view->window, wauthor);
> +	if (tc)
> +		wattr_off(view->window,
> +		    COLOR_PAIR(tc->colorpair), NULL);
>  	col += author_width;
>  	while (col < avail && author_width < author_display_cols + 2) {
>  		if (tog_base_commit.id != NULL &&
>  		    author_width == marker_column &&
> -		    entry->idx == tog_base_commit.idx)
> +		    entry->idx == tog_base_commit.idx) {
> +			tc = get_color(&s->colors, TOG_COLOR_COMMIT);
> +			if (tc)
> +				wattr_on(view->window,
> +				    COLOR_PAIR(tc->colorpair), NULL);
>  			waddch(view->window, tog_base_commit.marker);
> -		else
> +			if (tc)
> +				wattr_off(view->window,
> +				    COLOR_PAIR(tc->colorpair), NULL);
> +		} else
>  			waddch(view->window, ' ');
>  		col++;
>  		author_width++;
>  	}
> -	if (tc)
> -		wattr_off(view->window,
> -		    COLOR_PAIR(tc->colorpair), NULL);
>  	if (col > avail)
>  		goto done;
>  


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