From: Omar Polo <op@omarpolo.com>
Subject: Re: rework tog log reference labels
To: Stefan Sperling <stsp@stsp.name>
Cc: gameoftrees@openbsd.org
Date: Mon, 29 May 2023 08:31:48 +0200

On 2023/05/28 12:15:17 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> Rework how reference labels are rendered in the tog log view.
> 
> Draw reference labels and log message as separate strings. The previous
> code contained calculations mixing variables which represent an amount
> of wide characters in a string vs the display width of the string. We
> can avoid such nonsense by keeping the strings separate, though we have
> to be a bit careful about keeping horizontal scrolling intact.
> 
> Also fix a bug where we failed to account for reference labels while
> setting view->maxx which made the $ key not scroll far enough.
> 
> ok?

haven't noticed that with long refstrings we were unable to scroll
horizontally enough, nor thought that we were mixing the byte length
with the column width...

ok op@, but with leak below fixed

[...]
> +		limit = avail - col;
> +		if (view->child && !view_is_hsplit_top(view) && limit > 0)
> +			limit--;	/* for the border */

it's a bit unfortunate that we have to keep accounting for the border :/

[...]
> @@ -2888,6 +2901,21 @@ draw_commits(struct tog_view *view)
>  		free(author);
>  		if (err)
>  			goto done;
> +		refs = got_reflist_object_id_map_lookup(tog_refs_idmap,
> +		    entry->id);
> +		err = build_refs_str(&refs_str, refs, entry->id, s->repo);

we're leaking refs_str here

> +		if (err)
> +			goto done;
> +		if (refs_str) {
> +			wchar_t *ws;
> +			err = format_line(&ws, &width, NULL, refs_str,
> +			    0, INT_MAX, date_display_cols + author_cols, 0);
> +			free(ws);
> +			if (err)
> +				goto done;
> +			refstr_cols = width + 3; /* account for [ ] + space */
> +		} else
> +			refstr_cols = 0;

so I'd add a

		free(refs_str);
		refs_str = NULL;

>  		err = got_object_commit_get_logmsg(&msg0, c);
>  		if (err)
>  			goto done;